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

Resolves #4677 - Require Business Name for Vendors #4704

Open
wants to merge 3 commits into
base: main
Choose a base branch
from

Conversation

cassieemb
Copy link

@cassieemb cassieemb commented Oct 3, 2024

Resolves #4677

Description

This PR makes business_name mandatory when creating or updating a Vendor through the Community | Vendors form and the Purchases | New Vendor form. Previously the validations ensured that at least a contact_name or business_name was entered, but the dropdown to select a vendor looks a little funky without a business_name.

Screenshot 2024-10-03 at 9 40 53 AM

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_name or contact_name was required, we run a migration to backfill the business_name from the contact_name for any vendor instance that is missing one.

I added the validation on business_name to 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_name which doesn't make as much sense now that business_name is always required
b) it shows the error on business_name from the provideable validation in addition to the error on business_name from the vendor validation

Screenshot 2024-10-03 at 9 49 44 AM

I 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.

  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to not work as expected)

How Has This Been Tested?

  • Ran Lint locally
  • Ran the RSpec test suite locally (I had two unrelated system tests failing, but they were failing on main too, so checking to see if they pass with CI to confirm it's a local issue)
  • Tested on Vendor create/update through Community | Vendors
  • Tested on Vendor create through Purchases | New Purchase
  • I created a number of vendors with blank business names on main, and then swapped to my branch and ran the migration to see the business names backfilled from the contact name

Screenshots

Screenshot 2024-10-02 at 3 37 52 PM

Screenshot 2024-10-02 at 3 39 09 PM

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
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.

Business name should be mandatory for vendors
1 participant