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

Fix incorrect candidate bachelor degree equivalency #9958

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

CatalinVoineag
Copy link
Contributor

@CatalinVoineag CatalinVoineag commented Oct 21, 2024

Context

When a candidate inputs their degrees, they can chose an option called 'Another qualification equivalent to a degree'. This free text field will try to figure out if the value imputed is a bachelor degree or not.

Previously we would just check if the free text search value is in our list of Bachelor degrees by checking if the string is included in the list. We didn't match the strings exactly.

So if you have the correct keywords, like Bachelor of Medicine and Pixie hunting we would classify this degree as a bachelor degree because there is a valid bachelor degree called Bachelor of Medicine.

This caused the form to break as there is no valid degree for Pixie hunting in the reference data gem.

This commit checks the strings exactly to avoid this issue. Fixing this sentry error

Changes proposed in this pull request

The Type of bachelor degree should only be there for valid bachelor degrees

Before After
Screenshot from 2024-10-21 14-45-15 Screenshot from 2024-10-21 15-37-11

Guidance to review

Go on review app and add a Another qualification equivalent to a degree with the keywords of a actual bachelor degree like Bachelor of Medicine

Things to check

  • If the code removes any existing feature flags, a data migration has also been added to delete the entry from the database
  • This code does not rely on migrations in the same Pull Request
  • If this code includes a migration adding or changing columns, it also backfills existing records for consistency
  • If this code adds a column to the DB, decide whether it needs to be in analytics yml file or analytics blocklist
  • API release notes have been updated if necessary
  • If it adds a significant user-facing change, is it documented in the CHANGELOG?
  • Required environment variables have been updated added to the Azure KeyVault
  • Inform data insights team due to database changes
  • Make sure all information from the Trello card is in here
  • Rebased main
  • Cleaned commit history
  • Tested by running locally
  • Add PR link to Trello card

When a candidate inputs their degrees, they can chose an option called
'Another qualification equivalent to a degree'. This free text field
will try to figure out if the value imputed is a bachelor degree or not.

Previously we would just check if the free text search value is in our
list of Bachelor degrees by checking if the string is included in the
list. We didn't match the strings exactly.

So if you have the correct keywords, like Bachelor of Medicine and Pixie
hunting we would classify this degree as a bachelor degree because there
is a valid bachelor degree called `Bachelor of Medicine`.

This caused the form to break as there is no valid degree for Pixie
hunting in the reference data gem.

This commit checks the strings exactly to avoid this issue.
@CatalinVoineag CatalinVoineag self-assigned this Oct 21, 2024
@CatalinVoineag CatalinVoineag added the deploy_v2 Deploy the review app to AKS label Oct 21, 2024
@CatalinVoineag
Copy link
Contributor Author

component = render_inline(described_class.new(application_form:))
expect(component).to summarise(
key: 'Type of bachelor degree',
value: 'Bachelor of Arts in Architecture',
Copy link
Contributor

Choose a reason for hiding this comment

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

Do we want it to match if the user has inputted bachelor of arts in architecture? I'm happy to approve as it is -- we don't want to delay the bug fix thinking about what edge cases should / shouldn't be acceptable.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Do you mean to make the component smart enough to know that it's a bachelor degree even if the capitalisation is not correct?

Copy link
Contributor

@elceebee elceebee left a comment

Choose a reason for hiding this comment

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

I've tested on the review app and things work as expected.

I'm happy to approve as is -- might be worth considering weather we want it to match something that has just been typed lower case (bachelor of arts in architecture), but don't want to delay the bug fix.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
deploy_v2 Deploy the review app to AKS
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants