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

VACMS-19512: Drupal UX for phone numbers #19607

Draft
wants to merge 9 commits into
base: main
Choose a base branch
from
Draft

Conversation

davidmpickett
Copy link
Contributor

@davidmpickett davidmpickett commented Oct 24, 2024

Description

Relates to #19512
Also fixing #16150 as a bonus

Testing done

Manual in Tugboat

Screenshots

Entity & state Current UI Updated UI
Staff profile (no phone) Screenshot 2024-10-25 at 12 53 13 PM Screenshot 2024-10-24 at 5 16 25 PM
Staff profile (with phone) Screenshot 2024-10-24 at 5 19 03 PM Screenshot 2024-10-24 at 5 31 56 PM
VAMC Billing & insurance (with phone) Screenshot 2024-10-25 at 12 15 41 PM Screenshot 2024-10-24 at 5 52 26 PM
VAMC Billing & insurance (no phone) Screenshot 2024-10-25 at 12 15 29 PM 379959166-f3f7b956-3fa8-448e-b145-58b646920e8f
VAMC Facility mental health (with phone) Screenshot 2024-10-25 at 12 11 38 PM Screenshot 2024-10-25 at 12 49 46 PM
VAMC Facility mental health (no phone) Screenshot 2024-10-25 at 12 11 19 PM Screenshot 2024-10-25 at 12 49 32 PM
VAMC Facility mental health (FEATURE TOGGLE OFF) Screenshot 2024-10-25 at 2 24 58 PM Screenshot 2024-10-25 at 2 25 52 PM

QA steps

What needs to be checked to prove this works?
What needs to be checked to prove it didn't break any related things?
What variations of circumstances (users, actions, values) need to be checked?

Once per day setup

  1. Log in as an admin
  2. Go to Feature Toggle page
    • Check FEATURE_TELEPHONE_MIGRATION_V1
    • Click Save
  3. Run Telephone migration (optional)
    • Go to Dashboard
    • Run each command below separately in php terminal
    • Total migration time should be 30-40 minutes.
time drush codit-batch-operations:run MigratePhoneFieldToParagraph
time drush codit-batch-operations:run MigrateVamcBillingInsurancePhoneFieldToParagraph
time drush codit-batch-operations:run MigrateVamcFacilityMentalHealthPhoneFieldToParagraph

Check Staff Profile

  1. Log in to tugboat as a VAMC editor or Content Admin
  2. Go to Content > Add Content > Staff Profile
  3. Save a staff profile with no phone number
    • Fill in the required fields (First name, last name, section, related office)
    • In the contact information box, notice that by default, no phone number has been added
    • Add a revision message and save as draft
  4. Save a staff profile with a phone number
    • Edit the staff profile you just created
    • In the contact information box click the Add Phone number button
    • Before filling out any of the phone number sub-fields, add a revision message and attempt save as draft
    • See validation error for not filling out the required phone number sub-field
    • Fill in the required fields
    • Save as draft
  5. Remove a phone number from a staff profile
    • Edit the staff profile again
    • In the contact information box click the Remove button
    • Click confirm removal button (this is technically optional)
    • Add a revision message and save as draft

Check VAMC System Billing & Insurance

  1. Log in to tugboat as a Content Admin
  2. Go to Content > Add Content > VAMC System Billing and Insurance
  3. Save a Billing & Insurance page with a phone number
    • Notice that in the "For inquiries by phone about copay balance" section, a phone number is added by default
    • Before filling out any of the phone number sub-fields, add a revision message and attempt save as draft
    • See validation error for not filling out the required phone number sub-field
    • Fill in other required fields
    • Save as draft
  4. Remove a phone number from a Billing & Insurance page
    • Edit the page you just created
    • In the "For inquiries by phone about copay balance" section click the Remove button
    • Click confirm removal button (this is technically optional)
    • Add a revision message and save as draft
  5. Add a phone number
    • Edit the page you just created
    • In the "For inquiries by phone about copay balance" section click the Add Phone number button
    • Confirm that the correct phone number fields populate
  6. Verify change for 16150
    • Edit the page you just created
    • Notice that there is no longer an empty "Cashier's office" accordion

Check VAMC Facility - Mental Health Phone Number

  1. Log in to tugboat as a Content Admin
  2. Go to Content > Add Content > VAMC Facility
  3. Save a VAMC facility page with a phone number
    • Notice that in the "Mental health phone number" section, a phone number is added by default
    • Before filling out any of the phone number sub-fields, add a revision message and attempt save as draft
    • See validation error for not filling out the required phone number sub-field
    • Fill in other required fields
    • Save as draft
  4. Remove a phone number from a VAMC facility page
    • Edit the page you just created
    • In the "Mental health phone number" section click the Remove button
    • Click confirm removal button (this is technically optional)
    • Add a revision message and save as draft
  5. Add a phone number
    • Edit the page you just created
    • In the "Mental health phone number" section click the Add Phone number button
    • Confirm that the correct phone number fields populate

Check VAMC Facility - Mental Health Phone Number TOGGLE OFF

  1. Log in as an admin
  2. Go to Feature Toggle page
    • Uncheck FEATURE_TELEPHONE_MIGRATION_V1
    • Click Save
  3. Go to Content > Add Content > VAMC Facility
  4. Notice the slightly changed interface compared to prod

Check other phone number paragraph types to make sure we didn't break their styling

  • VAMC System VA Police
  • VBA Facility Service (service location)

Definition of Done

  • Automated tests have passed.
  • Code Quality Tests have passed.
  • Acceptance Criteria in related issue are met.
  • Manual Code Review Approved.

@davidmpickett davidmpickett self-assigned this Oct 24, 2024
@va-cms-bot va-cms-bot temporarily deployed to Tugboat October 24, 2024 22:09 Destroyed
Copy link

Checking composer.lock changes...

@va-cms-bot va-cms-bot temporarily deployed to Tugboat October 25, 2024 09:03 Destroyed
@va-cms-bot va-cms-bot temporarily deployed to Tugboat October 25, 2024 17:25 Destroyed
Copy link

Checking composer.lock changes...

@davidmpickett davidmpickett marked this pull request as ready for review October 25, 2024 21:46
@davidmpickett davidmpickett requested a review from a team as a code owner October 25, 2024 21:46
@jilladams
Copy link
Contributor

jilladams commented Oct 25, 2024

QA notes

Basic behavior

  • Content with no phone number: add phone, save: 🟢
  • Content with a phone number: remove phone number, save: 🟢
  • Field validations work as expected: 🟢

Label field weirdness 🔴 - now ticketed as #19623

On Staff profile, I removed an existing phone number, added a new one and saved with max char length to test. After removing phone, clicking to Add phone, I see a Label field (across all content types). To reproduce:

  • Remove an existing phone #
  • Add phone - new fields appear: Type, Phone Number, Extension number, Label.
    • Refresh page without populating fields:
      • Label field disappears
    • OR, save content in the field.
      • Node view shows the Label field value
      • Node edit hides the Label field

https://pr19607-h5ubpjxulma3zyqeq8hayikh80scnqhu.ci.cms.va.gov/node/73936
Screenshot 2024-10-25 at 3 31 43 PM

https://pr19607-h5ubpjxulma3zyqeq8hayikh80scnqhu.ci.cms.va.gov/node/73936/edit
Screenshot 2024-10-25 at 3 31 37 PM

Test content (FE is building on Fri pm and can be verified on Monday):

This probably blocks the PR, until we prove whether the label prints in the FE or not.

Remove confirmation / truncated text 🟡

Removal confirmation is technically optional per the QA steps.

When I use the "remove" button to remove, I get a confirmation prompt with truncated text. Loom that shows the experience: https://www.loom.com/share/7f203fbc6650489288d51f85a0e3dd09

Screenshot 2024-10-25 at 3 15 32 PM

This doesn't have to block the PR, but is not great UX debt to add while trying to improve the experience, so I do think it needs follow up. If the confirmation is optional, can we remove it? Or do we need to keep it and address the truncated text? (Updating .js .paragraph-type-top .paragraph-type-title to > 25% displays the full text.)

Empty Cashier's office accordion on B&I

Observed that Cashier's office accordion is empty. That's a documented defect: #16150

@jilladams
Copy link
Contributor

I'm a goofy person, and we cannot test what happens in the fe, because the FE isn't using the new phone number structure yet. I think. Not sure what that means for troubleshooting / fixing labels and whether or not this can merge.

@va-cms-bot va-cms-bot temporarily deployed to Tugboat October 26, 2024 08:58 Destroyed
@va-cms-bot va-cms-bot temporarily deployed to Tugboat October 27, 2024 08:54 Destroyed
@va-cms-bot va-cms-bot temporarily deployed to Tugboat October 28, 2024 08:55 Destroyed
@davidmpickett
Copy link
Contributor Author

Label field weirdness 🔴

On Staff profile, I removed an existing phone number, added a new one and saved with max char length to test. After removing phone, clicking to Add phone, I see a Label field (across all content types). To reproduce:

  • Remove an existing phone #

  • Add phone - new fields appear: Type, Phone Number, Extension number, Label.

    • Refresh page without populating fields:

      • Label field disappears
    • OR, save content in the field.

      • Node view shows the Label field value
      • Node edit hides the Label field

I also noticed this behavior when I was doing QA. However, this behavior is not specific to my PR. I flipped the feature toggle on my demo tugboat and confirmed that the label field shows up if you add, remove, and re-add.

Screenshot 2024-10-28 at 10 02 58 AM

Fixing this should be a separate ticket.

Copy link

Checking composer.lock changes...

@davidmpickett
Copy link
Contributor Author

davidmpickett commented Oct 28, 2024

Remove confirmation / truncated text 🟡

Removal confirmation is technically optional per the QA steps.

When I use the "remove" button to remove, I get a confirmation prompt with truncated text. Loom that shows the experience: https://www.loom.com/share/7f203fbc6650489288d51f85a0e3dd09

Screenshot 2024-10-25 at 3 15 32 PM

This doesn't have to block the PR, but is not great UX debt to add while trying to improve the experience, so I do think it needs follow up. If the confirmation is optional, can we remove it? Or do we need to keep it and address the truncated text? (Updating .js .paragraph-type-top .paragraph-type-title to > 25% displays the full text.)

I just committed a change to increase that to 40%, but then that makes this PR need CMS platform review since that's changing a more fundamental property in the system. I'm also not sure how we would go about verifying that increasing this percentage doesn't create problems elsewhere in the CMS.

Per scrum discussion: out of scope

@va-cms-bot va-cms-bot temporarily deployed to Tugboat October 28, 2024 15:41 Destroyed
Copy link

Checking composer.lock changes...

@va-cms-bot va-cms-bot temporarily deployed to Tugboat October 28, 2024 16:53 Destroyed
Copy link

Checking composer.lock changes...

@davidmpickett davidmpickett removed the request for review from a team October 28, 2024 16:57
@va-cms-bot va-cms-bot temporarily deployed to Tugboat November 1, 2024 08:45 Destroyed
@davidmpickett davidmpickett marked this pull request as draft November 1, 2024 20:19
Copy link

github-actions bot commented Nov 1, 2024

Checking composer.lock changes...

@va-cms-bot va-cms-bot temporarily deployed to Tugboat November 1, 2024 21:03 Destroyed
Copy link

github-actions bot commented Nov 1, 2024

Checking composer.lock changes...

@va-cms-bot va-cms-bot temporarily deployed to Tugboat November 1, 2024 21:40 Destroyed
Copy link

github-actions bot commented Nov 1, 2024

Checking composer.lock changes...

@va-cms-bot va-cms-bot temporarily deployed to Tugboat November 1, 2024 22:12 Destroyed
Copy link

github-actions bot commented Nov 1, 2024

Checking composer.lock changes...

@va-cms-bot va-cms-bot temporarily deployed to Tugboat November 2, 2024 08:45 Destroyed
@va-cms-bot va-cms-bot temporarily deployed to Tugboat November 3, 2024 09:43 Destroyed
@va-cms-bot va-cms-bot temporarily deployed to Tugboat November 4, 2024 08:41 Destroyed
@va-cms-bot va-cms-bot temporarily deployed to Tugboat November 5, 2024 08:40 Destroyed
@va-cms-bot va-cms-bot temporarily deployed to Tugboat November 6, 2024 08:44 Destroyed
@va-cms-bot va-cms-bot temporarily deployed to Tugboat November 7, 2024 08:41 Destroyed
@va-cms-bot va-cms-bot temporarily deployed to Tugboat November 8, 2024 16:52 Destroyed
@va-cms-bot va-cms-bot temporarily deployed to Tugboat November 9, 2024 08:40 Destroyed
@va-cms-bot va-cms-bot temporarily deployed to Tugboat November 10, 2024 08:38 Destroyed
@va-cms-bot va-cms-bot temporarily deployed to Tugboat November 11, 2024 08:37 Destroyed
@va-cms-bot va-cms-bot temporarily deployed to Tugboat November 12, 2024 08:39 Destroyed
@va-cms-bot va-cms-bot temporarily deployed to Tugboat November 13, 2024 08:40 Destroyed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants