department-of-veterans-affairs / va.gov-cms

Editor-centered management for Veteran-centered content.
https://prod.cms.va.gov
GNU General Public License v2.0
96 stars 70 forks source link

Update phone number fields for VAMC System Billing and Insurance in Drupal #17861

Open laflannery opened 4 months ago

laflannery commented 4 months ago

Background

Currently the Phone number field in the VAMC System Billing and Insurance content type is using the Telephone number field type. This is a single field for the editor to enter the phone number information as well as any applicable extension information. However, phone numbers and extensions require specific formatting in order to meet VA guidelines and to display and function properly on the FE.

Description

In order to make it easier for editors to enter their phone number correctly, we want to convert this single Telephone number field to use the Phone paragraph type field, which uses separate fields for the phone and extension.

Additional notes

Example VAMC Billing and Insurance page

Migration: We do have a module that we could use (developed by VA project alum Steve Wirt with some help from me and Dave) to help with help with migration scripting that uses a lot of the VA code patterns for scripts: https://www.drupal.org/project/codit_batch_operations

Current UI uses field_phone_number

351116522-781cf96b-d5cf-49df-a6b3-4a7aa5c86ea2

New UI should use phone_number paragraph type

Screenshot 2024-07-22 at 4 21 42 PM

### Acceptance Criteria
- [ ] Phone number field on VAMC System Billing and Insurance uses the Phone paragraph type
- [ ] Label field is hidden in Drupal UI
### Drupal - Field Migration
- [ ] Script is written to migrate 132 VAMC System Billing and Insurance phone numbers from field to paragraph.  Can use https://www.drupal.org/project/codit_batch_operations.
- [ ] Cardinality for phone_number paragraph type on VAMC System Billing and Insurance is set to 1
- [ ] 'Label' field on phone_number paragraph type is hidden on VAMC System Billing and Insurance Node:Edit
- [ ] Current`field_phone_number` is hidden but not removed on the node:view page
- [ ] Update [the phone number audit view](https://va-gov-cms.ddev.site/admin/content/audit/phone-numbers?title=&type=vamc_system_billing_insurance&moderation_state=All&workbench_access_section__section=All&phone=) to display phone paragraph type field AND the Telephone number type field (in order to compare values before and after migration)
- [ ] Migration is tested and validated
- [ ] Manual Code review
- [ ] CMS PR is created and approved but not merged
davidmpickett commented 1 month ago

Hey team! Please add your planning poker estimate with Zenhub @omahane @jv-agile6 @dsasser

dsasser commented 1 month ago

Engineering Pre-Refinement Notes

There are 132 telephone numbers to migrate:

select count(*) from node__field_phone_number nfpn
join node_field_data nfd ON nfd.nid = nfpn.entity_id
where nfpn.bundle = 'vamc_system_billing_insurance'
and nfd.status = 1;

# result: 132

The source field is a field type of "telephone" (provided by Drupal core).

Source phone numbers are not in a standard format. Some examples:

316-685-2221, ext. 57036, 52737, or 57031 520-792-1450 extension 1-5487 602-277-5551, ext. 2173 or 800-554-7174 (toll free)

The destination paragraph breaks out phone number and extension into separate fields, field_phone_extension, and field_phone_number.

field_phone_number is standardized in the format nnn-nnn-nnnn

field_phone_extension is not standardized. Existing examples from other entities: 6632 or 6801 4 then 2 Select 3

There are going to be GraphQL changes which means there is also a FE component to this work. The timing of the Drupal and FE changes need to be coordinated. In fact, in order to facilitate a clean release, I recommend leaving the existing phone field in Drupal, such that the FE would be able to reach both fields during the cutover, and remove the old Drupal field once the FE is working with the new paragraph fields.

davidmpickett commented 1 month ago

I recommend leaving the existing phone field in Drupal, such that the FE would be able to reach both fields during the cutover, and remove the old Drupal field once the FE is working with the new paragraph fields.

Good call out. This is our standard order of operations for migrating Drupal fields that are part of GraphQL queries. Those of us who have been through these efforts before know this pretty well, but here's what those steps look like

@Agile6MSkinner Based on our historical way of breaking up tickets, this epic currently has 3 Drupal tickets and 3 corresponding FE tickets. This might be a good place to think about combining those into a 3 stories/tickets with separate task lists for Drupal and FE so we're really tracking the whole kit and caboodle.

davidmpickett commented 1 month ago

There are 132 telephone numbers to migrate:

Oof. I hadn't really thought about the the migration side of this work. Might be a good idea to have a separate ticket for writing a migration script (and possibly also creating an audit View) that work out the logic of how the current data format gets parsed.

That's probably the gnarliest piece of this work from a Drupal perspective. Once that is worked out, it would be pretty the same process for each instance of the field that we want to convert. Tagging @omahane since he just went through all this on a much grander scale for Service Locations

dsasser commented 1 month ago

Oof. I hadn't really thought about the the migration side of this work. Might be a good idea to have a separate ticket for writing a migration script (and possibly also creating an audit View) that work out the logic of how the current data format gets parsed.

@davidmpickett I made the assumption that data migration was part of this issue, and pointed it accordingly. I don't see a lot of dev work here outside of the migration, so I'm not sure a separate ticket is desirable, but I'm open to being wrong about that.

jilladams commented 1 month ago

FE & launch ACs are removed and will go in a sep ticket: (3 points for FE)

### FE - Template & Query changes - 3
- [ ] The CMS PR created above will need to be referenced in this PR
- [ ] Update Content Build template & query for VAMC Billing and Insurance to reference new phone number fields
- [ ] Use Phone Number component: https://github.com/department-of-veterans-affairs/content-build/blob/main/src/site/components/phone-number.drupal.liquid
  - [ ] If type Fax is selected, number is not clickable
  - [ ] If type SMS is selected, number uses the sms prop
  - [ ] If type TTY is selected, number shows "TTY"
  - [ ] Exts are displayed and parsed correctly
- [ ] Manual Code review
- [ ] Content Build PR is created and approved but not merged
### Integrated QA & Launch
- [ ] Integrated testing of both PRs in a Tugboat (based on CMS PR) with Content Release based on Content Build PR
- [ ] Accessibility review
- [ ] Design Review
- [ ] Merge CMS PR and verify on prod
- [ ] Run migration on prod
- [ ] Update related KB articles if necessary
- [ ] Awkward ~23 hour gap where editor can see the new version of the field in Drupal, but their changes won't show up on FE
- [ ] Merge Content Build PR and verify on prod
- [ ] New CMS PR to remove hidden `field_phone_number` & update phone number audit view again

@Agile6MSkinner

omahane commented 1 month ago

@Agile6MSkinner @jilladams @dsasser @jv-agile6 I don't think we captured our decision:

No extension transformation

Original field

Extension transformation

Original field

laflannery commented 1 month ago

We should do the transformation, the extension cannot have letters in it in the component and then later on when we do the validation of this field we are not going to allow non-numerical numbers in this field.

So that's my thought at least

davidmpickett commented 1 month ago

@Agile6MSkinner @jilladams @dsasser @jv-agile6 I don't think we captured our decision:

  • Are we going to be transforming the extension data?

Yes*

*the more pertinent question is when are we going to transform it. @dsasser's suggestion was to do migration tickets without transformation and then normalize all of them when server-side implementation ticket is picked up. But that was more of a preference than a requirement.

omahane commented 1 month ago

Yes, I was asking about in this ticket, @davidmpickett . I apologize if that wasn't clear.

jv-agile6 commented 4 weeks ago

Moved task into progress. Created local branch VACMS-17861-UpdatePhoneFields-VAMC-SystemBillingInsurance.

jilladams commented 2 days ago

Jerry's coding work is done, and Jerry's writing QA steps. PR will target phone # integration branch, for QA.