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

Improve input validation on Enterprise fees page #9960

Closed
wants to merge 4 commits into from
Closed

Improve input validation on Enterprise fees page #9960

wants to merge 4 commits into from

Conversation

brolinr
Copy link

@brolinr brolinr commented Nov 9, 2022

Improved enterprise fee validation of input? ''?

Steps to reproduce as an admin?

  • edit an enterprise fee
  • choose flexible rate calculator
  • click update
  • type in an invalid value on 'max items' (e.g. some text or numbers with comma as separator)
  • click update --> invalid input is highlighted

Release notes

Changelog Category: User facing changes

The title of the pull request will be included in the release notes.

Dependencies

Documentation updates

@brolinr
Copy link
Author

brolinr commented Nov 9, 2022

This pull request is ready for review.

Copy link
Contributor

@jibees jibees left a comment

Choose a reason for hiding this comment

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

Thanks for this one. Small comments:

  • Title of the PR should be edited: we use the title as our Release notes, so they should be clear, concise and reflect what are the changes for the users ✅
  • Commit d867ffe should be deleted
  • Commit 340b41b should be squashed with the previous one (it's probably better for the git history to avoid this kind of commits which doesn't bring changes)

@@ -106,7 +106,7 @@ def check_enterprise_fee_input
:preferred_flat_percent, :preferred_amount,
:preferred_first_item, :preferred_additional_item,
:preferred_minimal_amount, :preferred_normal_amount,
:preferred_discount_amount, :preferred_per_unit
:preferred_discount_amount, :preferred_per_unit, :preferred_max_items, :preffered_currency
Copy link
Contributor

Choose a reason for hiding this comment

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

preffered_currency

I think there is a typo on preffered

Copy link
Author

Choose a reason for hiding this comment

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

Thanks for the review and the comments. I'll work on it 👍

@RachL
Copy link
Contributor

RachL commented Dec 8, 2022

Hi @brolinr Hope this message finds you well. Just checking in: do you still have time to apply the comments from the review?

@brolinr
Copy link
Author

brolinr commented Dec 9, 2022

Hi @RachL . I am well thank you. I haven't yet found the time to apply the changes. I have been busy stydying for exams but I'd really appreciate it if you can help applying the comments from the review.

@sigmundpetersen sigmundpetersen changed the title 9791 Improve validation of input on Enterprise fees page Dec 9, 2022
@sigmundpetersen sigmundpetersen changed the title Improve validation of input on Enterprise fees page Improve input validation on Enterprise fees page Dec 9, 2022
@RachL
Copy link
Contributor

RachL commented Jan 3, 2023

Hi @brolinr thank you for your feedback and don't worry: life happens :) Our team is currently quite busy, so I will close the PR to make the issue available for new contributors. Hopefully one of them will be able to pick-up your PR and apply the comments.

Thank you so much for your work until here!

@RachL RachL closed this Jan 3, 2023
@RachL RachL reopened this Jan 3, 2023
@brolinr brolinr closed this by deleting the head repository Oct 3, 2023
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.

[Admin][Enterprise Fees] Improve validation of input
3 participants