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

restore error check on custom params inputs #1989

Merged
merged 1 commit into from
Nov 2, 2023

Conversation

EnzoVezzaro
Copy link
Contributor

@EnzoVezzaro EnzoVezzaro commented Oct 31, 2023

Changes proposed in this PR:

  • restore customer parameter error check

HOW TO TEST:

Publish:

  • Go to Publish
  • Select "Algorith" asset type
  • Complete the rest in the form
  • Check this "This asset uses algorithm custom parameters"
  • Add, test, review custom parameters' fields

Edit:

  • Go to an asset with custom parameters you had published
  • Click on "Edit"
  • Add, remove, test, review custom parameters
  • Save changes

Copy link

vercel bot commented Oct 31, 2023

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Comments Updated (UTC)
market ✅ Ready (Inspect) Visit Preview 💬 Add feedback Oct 31, 2023 9:37pm
1 Ignored Deployment
Name Status Preview Comments Updated (UTC)
dubai-challenge ⬜️ Ignored (Inspect) Oct 31, 2023 9:37pm

Copy link

codeclimate bot commented Oct 31, 2023

Code Climate has analyzed commit 8dda0f0 and detected 0 issues on this pull request.

The test coverage on the diff in this pull request is 100.0% (50% is the threshold).

This pull request will bring the total coverage in the repository to 20.2% (0.0% change).

View more on Code Climate.

@EnzoVezzaro EnzoVezzaro changed the title restore error check on input restore error check on custom params inputs Nov 1, 2023
@mariacarmina
Copy link
Member

I can confirm that the issue is fixed right now, I could add the parameters attached to the algorithm. Thank you @EnzoVezzaro! I'll approve.

Copy link
Member

@bogdanfazakas bogdanfazakas left a comment

Choose a reason for hiding this comment

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

lgtm

@EnzoVezzaro EnzoVezzaro merged commit 90b41c7 into main Nov 2, 2023
15 checks passed
@EnzoVezzaro EnzoVezzaro deleted the fix/issue-1979-consumer-params-crashes-app branch November 2, 2023 12:44
@LucaMilanese90
Copy link
Contributor

LucaMilanese90 commented Nov 2, 2023

I noticed this fix broke the custom provider validation (try to remove the default provider and add a new one and you'll see the app crash), you could probably just add !field.name.endsWith('.providerUrl') to the checkError function.

@EnzoVezzaro
Copy link
Contributor Author

EnzoVezzaro commented Nov 2, 2023

I noticed this fix broke the custom provider validation (try to remove the default provider and add a new one and you'll see the app crash), you could probably just add !field.name.endsWith('.providerUrl') to the checkError function.

Great catch 👍🏽 I created a PR to fix this. Your solution @LucaMilanese90 seems to work, thanks for the help.

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.

Consumer parameters fields redirect to blank page
4 participants