Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

bump ocean.js v0.16.2, ocean.assets.isConsumable implementation to disable buy button and display relevant message #463

Merged
merged 31 commits into from
Jul 6, 2021
Merged

bump ocean.js v0.16.2, ocean.assets.isConsumable implementation to disable buy button and display relevant message #463

merged 31 commits into from
Jul 6, 2021

Conversation

krisliew
Copy link
Contributor

@krisliew krisliew commented Mar 30, 2021 •
edited
Loading

Fixes #452 and #685

Changes proposed in this PR:

  • bump ocean.js to v0.16.2
    • set datePublished as '' in src/utils/metadata.ts for validate-remote
  • Use ocean.assets.isConsumable to check if dataset can be consume. Disable buy button if false is returned with relevant message below buy button.
  • Updated src/components/molecules/FormFields/Credential/index.tsx to convert all address to lowercase for allow and deny list because accountId from useWeb3() have case sensitive issue. (Returning all lowercase on change metamask account)

To save your time on testing this PR, you may use the following DID to test 😄 (Rinkeby network)

  1. Download test isOrderDisabled=true
    https://market-3628a6svt-oceanprotocol.vercel.app/asset/did:op:d1Dae50a32f613B282BFCcb50fE33Bf987514882

  2. Compute test isOrderDisabled=true
    https://market-3628a6svt-oceanprotocol.vercel.app/asset/did:op:940F4d33C1886C050c5601ED6bd7F51d9084Cc16

  3. Compute test isOrderDisabled=false
    https://market-3628a6svt-oceanprotocol.vercel.app/asset/did:op:6A41c8f617686B4Dff27c8A3ee5ECc4f175cb8a4

  4. Compute test allow list:
    https://market-3628a6svt-oceanprotocol.vercel.app/asset/did:op:01530feD6aE9CFbfA602e05Ef51C447e8C145F1C

  5. Algo test allow list:
    https://market-3628a6svt-oceanprotocol.vercel.app/asset/did:op:CFD8E0a3188A0ED6665929e29e3716e3C2B50df8

  6. Algo test deny list:
    https://market-3628a6svt-oceanprotocol.vercel.app/asset/did:op:5DA103145F09FEf6C60d65A9c7033A1b0458B26a

@vercel
Copy link

vercel bot commented Mar 30, 2021 •
edited
Loading

This pull request is being automatically deployed with Vercel (learn more).
To see the status of your deployment, click below or on the icon next to each commit.

🔍 Inspect: https://vercel.com/oceanprotocol/market/Ee78XrvMKfCnrNEJkYAK8BQX7pYn
✅ Preview: https://market-git-feature-issues-452-oceanprotocol.vercel.app

@krisliew krisliew mentioned this pull request Mar 30, 2021
Closed
@krisliew krisliew changed the title Feature/issues 452 Allow data asset owner (Publisher) to enable/disable consume for a data asset - Feature/issues 452 Mar 30, 2021
@krisliew krisliew self-assigned this Mar 30, 2021
@krisliew
Copy link
Contributor Author

@mihaisc @alexcos20 is isConsumeDisable variable name good enough? or should I update the flag name back to isMaintenance?

@mihaisc
Copy link
Contributor

mihaisc commented Apr 14, 2021

i think isMaintenance is better for this purpose

@alexcos20
Copy link
Member

I would go towards: isDisabled & isRetired. I will open a PR on oep

@kremalicious kremalicious mentioned this pull request Apr 14, 2021
Merged
@krisliew krisliew marked this pull request as ready for review April 15, 2021 07:48
@kremalicious kremalicious mentioned this pull request May 7, 2021
Merged
4 tasks
@kremalicious
Copy link
Contributor

what happened here? The PR tries to revert main to some very old state. Needs a rebase or merge against main

@kremalicious kremalicious marked this pull request as draft May 7, 2021 12:44
@kremalicious
Copy link
Contributor

To not change all flows I would go with whatever we have as established pattern right now to keep it consistent which would be disabled button.

Ideally if we have disabled buttons then the label of the disabled button should say why it's disabled (so in this case here we wouldn't need a dedicated "Unavailable" element, the button can just say that) but we don't need to solve this in here

@krisliew krisliew changed the title 'Disabled' Status message when isOrderDisabled flag is true - Feature/issues 452 [WIP] 'Disabled' Status message when isOrderDisabled flag is true - Feature/issues 452 Jun 28, 2021
@krisliew
accountId from useWeb3() case sensitive fix
6e90bd4 
* accountId from useWeb3() returning lower case on switching metamask account
@krisliew krisliew linked an issue Jun 28, 2021 that may be closed by this pull request
To show a meaningful UI message for allow/deny list feature #685
Closed
@krisliew krisliew changed the title [WIP] 'Disabled' Status message when isOrderDisabled flag is true - Feature/issues 452 [WIP] ocean.asset.isConsumable implementation to disable buy button and display relevant message Jun 28, 2021
@krisliew krisliew changed the title [WIP] ocean.asset.isConsumable implementation to disable buy button and display relevant message [WIP] ocean.assets.isConsumable implementation to disable buy button and display relevant message Jun 28, 2021
@krisliew krisliew changed the title [WIP] ocean.assets.isConsumable implementation to disable buy button and display relevant message ocean.assets.isConsumable implementation to disable buy button and display relevant message Jun 28, 2021
@krisliew krisliew changed the title ocean.assets.isConsumable implementation to disable buy button and display relevant message bump ocean.js v0.16.2, ocean.assets.isConsumable implementation to disable buy button and display relevant message Jul 1, 2021
@krisliew
Squashed commit of the following:
6cf684b 
commit c6c5021
Merge: 1c4ba14 1ceb0f6
Author: Bogdan Fazakas <bogdan.fazakas@gmail.com>
Date:   Fri Jul 2 13:22:29 2021 +0300

    Merge pull request #517 from oceanprotocol/issue510-match-providers-algo-dataset

    Match dataset providers with private algorithms providers

commit 1c4ba14
Author: Kris Liew <39853992+krisliew@users.noreply.github.com>
Date:   Thu Jul 1 14:52:10 2021 +0800

    Edit Advance Settings (updateMetadata() v3 workaround solution for free pricing) (#692)

    * v3 update metadata workaround

commit 1ceb0f6
Author: Bogdan Fazakas <bogdan.fazakas@gmail.com>
Date:   Tue Jun 29 09:37:12 2021 +0300

    fixed lint errors

commit 127d31d
Author: Bogdan Fazakas <bogdan.fazakas@gmail.com>
Date:   Tue Jun 29 09:20:35 2021 +0300

    fixed issues after merge of main branch

commit b0356f5
Merge: 0d1534d c81a986
Author: Bogdan Fazakas <bogdan.fazakas@gmail.com>
Date:   Tue Jun 29 00:08:45 2021 +0300

    Merge branch 'main' into issue510-match-providers-algo-dataset

commit 0d1534d
Author: Bogdan Fazakas <bogdan.fazakas@gmail.com>
Date:   Thu Apr 15 10:55:58 2021 +0300

    check ddo provider for assets in edit compute

commit 1beb33d
Author: Bogdan Fazakas <bogdan.fazakas@gmail.com>
Date:   Wed Apr 14 20:11:29 2021 +0300

    check ddo provider for algo list in start compute
@codeclimate
Copy link

codeclimate bot commented Jul 5, 2021

Code Climate has analyzed commit 9b86387 and detected 2 issues on this pull request.

Here's the issue category breakdown:

Category Count
Complexity 1
Duplication 1

View more on Code Climate.

@jamiehewitt15
Copy link
Member

It seems to work well to me. As far as I can tell the issues mentioned above seem to have been fixed

@krisliew krisliew merged commit 675da08 into main Jul 6, 2021
@krisliew krisliew deleted the feature/issues-452 branch July 6, 2021 06:51
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@kremalicious kremalicious kremalicious left review comments

@jamiehewitt15 jamiehewitt15 jamiehewitt15 approved these changes

@mihaisc mihaisc Awaiting requested review from mihaisc mihaisc is a code owner

Assignees

@krisliew krisliew

Labels
None yet
Projects
None yet
Milestone
No milestone
6 participants
@krisliew @mihaisc @alexcos20 @kremalicious @jamiehewitt15 @kianyee