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.

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

Feat: Consumer Parameters #1921

Merged
merged 133 commits into from
Sep 7, 2023

Conversation

moritzkirstein
Copy link
Contributor

@moritzkirstein moritzkirstein commented May 18, 2023

Consumer Parameters for Ocean Market

This PR focuses on bringing consumer parameters as defined in the docs to Ocean Market.
We are targeting the complete flow, considering publish and edit functionalities as well as handling consumption with provided user data. This PR introduces quite a few changes that I will try to summarize below.

Related to oceanprotocol/ocean.js#1730


Proposed Changes in this PR:

1. Publish Form

Add ConsumerParameters Form sections to publish form via a new ConsumerParameters input element

  • Adds a checkbox to algorithm type assets on Step 1 Metadata, that when checked displays a new input component to add consumer parameters
  • Adds a checkbox to Step 2 Access, that when checked displays a new input component to add consumer parameters
    image
    image
  • The InputElement/ConsumerParameter component utilizes a controlled tab component to handle display of multiple parameters
    • New parameters can be added (and deleted), creating new tabs to switch between for respective parameter specification
    • Tabs currently use the parameter name as their title
  • For select type parameters a KeyValueInput component is used to specify the needed parameter.options (similar to Headers in url files)
  • Added reusable form validation for consumer parameters (publish and edit)
  • Added FormConsumerParameter interface that specifically handles display of consumer parameters in the publish and edit form
  • Input for paramter.required is a select field with Required and Optional options, trying to improve UX compared to simple checkbox
  • DefaultInput (input for parameter.default) changes based on the selected parameter type
    • text type parameters, will use a regular text input field
    • number type parameters, will use a number input field
    • boolean type parameters, will use a select input field with true and false as options
    • select type parameters, will use a select input field with the options generated from specified parameter.options
  • Add transformation of consumer parameters in transformPublishFormToDdo utility function

image



2. Edit Form

Add (up to) two ConsumerParameters sections to edit form

  • Extend metadata tab with consumerParameters for algorithms and service.consumerParameters for all assets
  • Uses the same component that is used in publish form, prefilled with data parsed from DDO
  • Add parseConsumerParameters utility function to @utils/ddo.ts to be able to parse DDO consumer parameter data for display in edit form

image



3. Asset Consumption

Add UserCustomparameters to consumption forms

  • New Input section that parses consumerParameters from DDO to display the needed input fields to get consumer input
    • The following parameter configs are supported (based on docs):
      • text type parameters, will use a regular text input field
      • number type parameters, will use a number input field
      • boolean type parameters, will use a select input field with true and false as options
      • select type parameters, will use a select input field with the options parsed from DDO
      • for select inputs that are not required an empty option is added
  • The tab component is used to display up to three possible sections (in compute, one section for download):
    • Dataset Service consumer parameters
    • Algorithm Service consumer parameters
    • Algorithm consumer parameters (algoCustomData.json)
  • Refactor Download.tsx to use <Formik> to be able to collect userdata
  • Dynamically create validation schemes based on consumerParameter definitions from DDO
  • Add collected user data to the provider calls on consumption

image



4. Misc updates / refactorings

  • Add ConsumerParameter, ServiceExtended and MetadataExtended types until they are released in ocean.js (Add consumer parameter types ocean.js#1730)
  • Refactor InputeElement/Headers to InputeElement/KeyValueInput to be reusable for similar purposes, e.g., the select options for consumer parameters in this PR
  • Refactor error handling in FormInput/index.tsx with new utility helper function to get object's properties by given string path (helper function also used for consumer parameter form sections)
  • Add option to make atoms/Tabs component controllable by exposing selectedIndex and onSelect from ReactTabs

moritzkirstein and others added 30 commits March 16, 2023 12:15
@LucaMilanese90
Copy link
Contributor

Hi there 👋🏽,

Hope you're doing well. I'll be reviewing your PR in these days, sorry if I'm not commenting everything at once.

  • We should limit the fields values to avoid this:
Screenshot 2023-06-26 at 15 00 10 * We should display the description of the fields when consuming: Screenshot 2023-06-26 at 15 11 28 * There's a weird flickering when pressing the "Get" / "Retry" button:

custom.parameter.deployment.-.Ocean.Market.-.26.June.2023.mp4
I'll keep adding stuff this week.

but Awesome job!

@EnzoVezzaro I added some chars limit for name and label (min 4, max 50) and description (min 10, max 500), let me know if the numbers are fine or if you want to change them.
As for showing the description for the different fields, is it ok to put them as help text to keep the form compact?

@EnzoVezzaro
Copy link
Contributor

EnzoVezzaro commented Jul 6, 2023

As for showing the description for the different fields, is it ok to put them as help text to keep the form compact?

Hi @LucaMilanese90 absolutely, it's just fine 👍🏽

As for the limit in name I'd add ellipses (...) on the tab headers once it reaches about 15 characters, to avoid having this crazy tab header over here.

Screenshot 2023-07-06 at 08 42 24

the rest seems fine. If @jamiehewitt15 doesn't have anything else to add, after these little changes we should be ready to go.

@EnzoVezzaro EnzoVezzaro self-requested a review July 10, 2023 11:52
content/publish/consumerParameters.json Outdated Show resolved Hide resolved
@LucaMilanese90
Copy link
Contributor

@EnzoVezzaro I updated the placeholders with one of the examples in the docs https://docs.oceanprotocol.com/core-concepts/did-ddo#consumer-parameters
Please, let me know if you prefer to go with something different.

Copy link
Member

@jamiehewitt15 jamiehewitt15 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, this looks good. Thanks for all the hard work and making the changes

@bogdanfazakas
Copy link
Member

@LucaMilanese90 can you do a rebase pls, so we can have the latest fixes from main available in the PR ?

@jamiehewitt15
Copy link
Member

Thanks a lot for your work on this @LucaMilanese90 @moritzkirstein. It would be great if you can also test it in production now that it has been deployed, thank you

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants