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

3558 Add validations for partner requests settings #3647

Merged
merged 1 commit into from
Jun 11, 2023

Conversation

dacur
Copy link
Collaborator

@dacur dacur commented Jun 7, 2023

Resolves #3558

Description

Adds a validation so that a partner cannot entirely remove their own ability to make requests through the partner profile. Also adds tests for this change, and updated a failing spec that was caused by this change.

Other changes include:

  • Bumped Ruby version
  • Added .vscode/ to .gitignore

Type of change

  • Bug fix (non-breaking change which fixes an issue)

How Has This Been Tested?

Added tests and ran the entire test suite.

@dacur dacur force-pushed the 3558-validate-partner-profile-request-settings branch from 8855154 to 98aed5f Compare June 7, 2023 14:08
@dacur dacur self-assigned this Jun 7, 2023
@dacur dacur force-pushed the 3558-validate-partner-profile-request-settings branch from 98aed5f to 11013a3 Compare June 7, 2023 14:26
@dacur dacur changed the title Added validations for partner requests settings, and upgraded to late… 3558 Add validations for partner requests settings Jun 7, 2023
@dacur dacur marked this pull request as ready for review June 7, 2023 14:52
@dacur dacur force-pushed the 3558-validate-partner-profile-request-settings branch from 11013a3 to 8788946 Compare June 7, 2023 14:52
Copy link
Member

@seanmarcia seanmarcia left a comment

Choose a reason for hiding this comment

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

Looks good. Thanks for also getting the Ruby Upgrade!

Copy link
Collaborator

@dorner dorner left a comment

Choose a reason for hiding this comment

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

One minor comment, otherwise looks good!

@@ -129,5 +130,9 @@ def client_share_is_0_or_100
errors.add(:base, "Total client share must be 0 or 100")
end
end

def has_at_least_one_request_setting
errors.add(:base, "At least one request type must be set") unless enable_child_based_requests || enable_individual_requests || enable_quantity_based_requests
Copy link
Collaborator

Choose a reason for hiding this comment

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

I prefer using if ! instead of unless if there's more than one condition - I can never remember how it reads otherwise.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Updated!

@dacur dacur force-pushed the 3558-validate-partner-profile-request-settings branch from 8788946 to 4df17b4 Compare June 9, 2023 19:00
@dacur dacur requested a review from dorner June 9, 2023 19:05
@dorner dorner merged commit 393d5b2 into main Jun 11, 2023
@dorner dorner deleted the 3558-validate-partner-profile-request-settings branch June 11, 2023 14:54
@dorner
Copy link
Collaborator

dorner commented Jun 11, 2023

Thanks for the contribution!

FYI for next time - if there is an active review, please don't force push changes, as it is more difficult to see what changed since the review.

@dacur
Copy link
Collaborator Author

dacur commented Jun 12, 2023

@dorner Gotcha, thanks :)

dacur pushed a commit that referenced this pull request Jun 12, 2023
to be delayed while other work is done, as outlined in 3657.
dorner pushed a commit that referenced this pull request Jun 13, 2023
to be delayed while other work is done, as outlined in 3657.
@cielf
Copy link
Collaborator

cielf commented Jun 27, 2023

@dacur This went into production on Sunday. Thank you for your contribution!

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.

Partner should not be able to entirely remove their own ability to make requests through the partner profile.
4 participants