LG-6708 Add additional columns to in_person_enrollments table#6556
Conversation
x341x
commented
Jul 7, 2022
x341x
commented
- Adds the columns current_address_matches_id and selected_location_details to the in_person_enrollments table
- The ticket specified using add_index and remove_index in the migration for the above changes but add_column was used instead for both tables migration
- Class InPersonEnrollment statuses were modified to include "establishing: 0", and moved previous status down
| establishing: 0, | ||
| pending: 1, | ||
| passed: 2, | ||
| failed: 3, | ||
| expired: 4, | ||
| canceled: 5, |
There was a problem hiding this comment.
In general I think renumbering enums like this is dangerous.... since we haven't launched yet this is probably fine. I think it would preferable to do establishing: 7 in the future
There was a problem hiding this comment.
Due to it not being in production we won't break anything also establishing is the phase that comes before pending so logically that order makes sense
| class AddAddressAndLocationToInPersonEnrollment < ActiveRecord::Migration[6.1] | ||
| def change | ||
| add_column :in_person_enrollments, :current_address_matches_id, :boolean, comment: "True if the user indicates that their current address matches the address on the ID they're bringing to the Post Office." | ||
| add_column :in_person_enrollments, :selected_location_details, :jsonb, comment: "The location details of the Post Office the user selected (including title, address, hours of operation)" |
There was a problem hiding this comment.
Does USPS give us any location identifier we could use instead of storing a blob of the full details? A concern I'd have is that we'd be storing a lot of duplicate data with this.
There was a problem hiding this comment.
Unforunately there's no API to look up locations by ID at this point
There was a problem hiding this comment.
Do we need to be able to do a look-up with this field data? I'd wonder if we could at least create our own identifier as e.g. a fingerprint/hash of the location data blob.
There was a problem hiding this comment.
No, we won't need to do a look-up with this field data, for now.
…mns to in_person_enrollments
tomas-nava
left a comment
tomas-nava
left a comment
There was a problem hiding this comment.
This looks good to me; let's pair to resolve conflicts and merge.
…ew-columns-to-in-person-enrollments
tomas-nava
changed the title
tomas-nava
deleted the
shanice/lg-6708-add-new-columns-to-in-person-enrollments
branch
| failed: 2, | ||
| expired: 3, | ||
| canceled: 4, | ||
| establishing: 0, |
There was a problem hiding this comment.
Reading the associated ticket, if the new establishing status is meant to allow us to create an enrollment record before we've called out to the USPS API, would it also be expected that the Enrollment's profile could now also be optional? I guess I'm imagining a scenario where we create the enrollment early enough in the process that the profile hasn't been created yet, we'd need to save it without a profile, and update it later to assign the profile (when the profile is created at the password submission step).
Am I understanding correctly, @x341x @tomas-nava ?
There was a problem hiding this comment.
Following up on this; at the moment the enrollment is only in establishing for a few moments before it's set to pending, and this all happens when the profile definitely exists. If we end up needing to create the enrollment earlier, we will think about whether the profile should be optional.
4 participants
