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

[Admin][Enterprise Fees] Improve validation of input #9860

Closed
drummer83 opened this issue Oct 24, 2022 · 8 comments
Closed

[Admin][Enterprise Fees] Improve validation of input #9860

drummer83 opened this issue Oct 24, 2022 · 8 comments
Assignees
Labels
bug-s4 The bug is annoying, but doesn't prevent from using the platform. Not so many users are impacted. good first issue hackathon Issues for upcoming hackathons

Comments

@drummer83
Copy link
Contributor

Description

These are little leftovers from #9791 and exactly the same as #9836 for shipping methods. Maybe the code can be referenced instead of copying it?

On page /admin/enterprise_fees there is still room for improvement.

  • For fee calculator 'Flexible rate' the 'max items' value is not validated as other fields, but it is set to '0' for invalid input while for 'Price sack' the amount values are validated as other fields and the error message is displayed for invalid input. Flexible rate should do the same.
  • After clicking 'update' or 'create' invalid input field(s) should be highlighted (red border of input field) and all other updated information should be kept (including changes on other existing enterprise fees). Currently changes are lost and have to be entered again.

Expected Behavior

  • All input data should be validated.
  • There should be an error message when invalid data is submitted.
  • Invalid data should be highlighted.
  • Updated information should be kept when submission of the form fails.

Actual Behaviour

  • Not all input data is validated.
  • There is no error message when invalid data is submitted in the field 'max items' of the 'Flexible rate' calculator.
  • Invalid data is not highlighted.
  • Updated information is lost when submission of the form fails.

Steps to Reproduce

Creating or changing an enterprise fee with 'Flexible rate':

  1. As enterprise user or super admin user go to /admin/enterprise_fees
  2. Select fee type, enter a name and select a tax category
  3. Select the 'Flexible rate' calculator and click 'update'
  4. Type invalid information into the 'max unit' field (e.g. the text 'invalid')
  5. Click 'update'
  6. See that the invalid value is set to 0

Creating or changing any shipping method:

  1. As enterprise user or super admin user go to /admin/enterprise fees
  2. Fill in required field, select any calculator and click 'update'
  3. Update some fields (e.g. description) - do not click update yet
  4. Type invalid information into any calculator field (e.g. the text 'invalid' in the 'amount' field)
  5. Click 'update'
  6. See that the updated information is lost
  7. See that is no highlighting of the invalid input

Animated Gif/Screenshot

Peek 2022-10-24 22-02

Peek 2022-10-24 22-01

Workaround

Severity

bug-s4: it's annoying, but you can use it

Your Environment

  • Version used:
  • Browser name and version:
  • Operating System and version (desktop or mobile):

Possible Fix

@drummer83 drummer83 added good first issue bug-s4 The bug is annoying, but doesn't prevent from using the platform. Not so many users are impacted. hackathon Issues for upcoming hackathons labels Oct 24, 2022
@brolinr
Copy link

brolinr commented Nov 8, 2022

Can I work on this one

@sigmundpetersen
Copy link
Contributor

Yes, sure thank you @brolinr 🎉
Don't hesitate to ask if you have any questions 👍

@brolinr
Copy link

brolinr commented Nov 9, 2022

I resolved the issue. Here is the pull request https://github.com/openfoodfoundation/openfoodnetwork/pull/9960

@sigmundpetersen sigmundpetersen changed the title Enterprise fees: improve validation of input [Enterprise Fees] Improve validation of input Jan 3, 2023
@sigmundpetersen sigmundpetersen changed the title [Enterprise Fees] Improve validation of input [Admin][Enterprise Fees] Improve validation of input Jan 3, 2023
@mkllnk mkllnk moved this to To triage (By the maintainers) in Welcome New Developers! Jan 20, 2023
@HillaryOkello
Copy link
Contributor

Can I work on the changes in the PR for this?

@drummer83
Copy link
Contributor Author

Hi @HillaryOkello and welcome!

Sure, I will assign you to the issue and the prepared PR.

Thanks for your contribution!

@drummer83
Copy link
Contributor Author

I can't assign you to the PR but please go ahead.

@dacook dacook moved this from To triage (By the maintainers) to In progress in Welcome New Developers! Sep 27, 2023
@HillaryOkello
Copy link
Contributor

Hi @drummer83 I tried to reproduce the issue but it seems to be fixed. We cannot type invalid information into any calculator field because of the number format of the field. Could there be something else that is needed that I haven't understood, please?

@drummer83
Copy link
Contributor Author

Hi @HillaryOkello,
Thanks for getting in touch. I have tried to reproduce the issue as well and it seems that it has been fixed in the meantime already. Great catch!!
Sorry for the confusion! If you like you are very welcome to pick a different issue.
I will close this one here.
Thanks again!

@github-project-automation github-project-automation bot moved this from In progress to Done in Welcome New Developers! Sep 29, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug-s4 The bug is annoying, but doesn't prevent from using the platform. Not so many users are impacted. good first issue hackathon Issues for upcoming hackathons
Projects
Development

Successfully merging a pull request may close this issue.

4 participants