Resolves #4677 - Require Business Name for Vendors #4704
Merged
dorner merged 7 commits intoOct 10, 2024
Conversation
add validation for business name on vendor model, skip validation in provideable when the model is a vendor to prevent duplicate errors on invalid form submission, specs for both changes
…me if business name is blank
dorner
requested changes
Oct 6, 2024
…o model instead of skipping when the provideable model is vendor
…assieemb/human-essentials into 4677-require-business-name-for-vendors
Contributor
|
@cassieemb: Your PR |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Resolves #4677
Description
This PR makes
business_namemandatory when creating or updating a Vendor through the Community | Vendors form and the Purchases | New Vendor form. Previously the validations ensured that at least acontact_nameorbusiness_namewas entered, but the dropdown to select a vendor looks a little funky without abusiness_name.This change prevents further vendors from being created with empty business names, but we still need to handle the existing cases of this. Since we know that either a
business_nameorcontact_namewas required, we run a migration to backfill thebusiness_namefrom thecontact_namefor any vendor instance that is missing one.I added the validation on
business_nameto the Vendor model and while that alone works correctly with specs, it didn't look the nicest displayed on the form submission because:a) it shows the error on
contact_namewhich doesn't make as much sense now thatbusiness_nameis always requiredb) it shows the error on
business_namefrom theprovideablevalidation in addition to the error onbusiness_namefrom the vendor validationI originally wanted to avoid modifying Provideable, but it felt like the logic to handle exclusively from Vendor was messier than it would be to just add a check in provideable to skip those validations when the model is vendor so that we can use the vendor specific validations instead.
Type of change
I wasn't 100% sure which of these to choose - I don't think I'd call this a breaking change, but existing functionality would be expected to accept an empty business name when contact is supplied, so this change is different than expectations.
How Has This Been Tested?
Screenshots