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

[VA-16652] Align Main mental health phone number with service contact info #1907

Merged
merged 3 commits into from
Feb 23, 2024

Conversation

maxx1128
Copy link
Contributor

@maxx1128 maxx1128 commented Feb 8, 2024

Summary

If the facility's mental health care chooses "Use the general facility phone number," it will show the facility's mental health care number, not their main phone number. It also updates the phone number partial being used so it includes aria-describedby with the label.

Related issue(s)

Testing done

Visual. Checked all available services on Hot Springs VA Medical Center and Cheyenne VA Medical Center in my tugboat instance.

Screenshots

Note: this screenshots include changes that are no longer included in this pull request. The only ones relevant to this pull request are the main phone numbers.

Before

Screen Shot 2024-02-12 at 1 16 10 PM Screen Shot 2024-02-12 at 1 16 43 PM

After

Screen Shot 2024-02-12 at 1 17 59 PM Screen Shot 2024-02-12 at 1 18 12 PM

What areas of the site does it impact?

VA Health Care Facilities

Acceptance criteria

Quality Assurance & Testing

  • I fixed|updated|added unit tests and integration tests for each feature (if applicable).
  • No sensitive information (i.e. PII/credentials/internal URLs/etc.) is captured in logging, hardcoded, or specs
  • Linting warnings have been addressed
  • Documentation has been updated (link to documentation *if necessary)
  • Screenshot of the developed feature is added
  • Accessibility testing has been performed

Error Handling

  • Browser console contains no warnings or errors.
  • Events are being sent to the appropriate logging solution
  • Feature/bug has a monitor built into Datadog or Grafana (if applicable)

Authentication

  • Did you login to a local build and verify all authenticated routes work as expected with a test user

⚠️ Team Sites (only applies to modifications made to the VA.gov header) ⚠️

  • The header does not contain any web-components
  • I used the proxy-rewrite steps to test the injected header scenario
  • I reached out in the #sitewide-public-websites Slack channel for questions

Requested Feedback

(OPTIONAL) What should the reviewers know in addition to the above. Is there anything specific you wish the reviewer to assist with. Do you have any concerns with this PR, why?

@va-vfs-bot va-vfs-bot temporarily deployed to master/main/va-16652-mental-health-phone-numbers February 8, 2024 22:03 Inactive
@maxx1128 maxx1128 force-pushed the va-16652-mental-health-phone-numbers branch from 9b100b5 to 8e05d22 Compare February 9, 2024 16:30
@va-vfs-bot va-vfs-bot temporarily deployed to master/main/va-16652-mental-health-phone-numbers February 9, 2024 16:50 Inactive
@maxx1128 maxx1128 force-pushed the va-16652-mental-health-phone-numbers branch from 8e05d22 to ab662fa Compare February 9, 2024 18:33
@va-vfs-bot va-vfs-bot temporarily deployed to master/main/va-16652-mental-health-phone-numbers February 9, 2024 18:46 Inactive
@maxx1128 maxx1128 force-pushed the va-16652-mental-health-phone-numbers branch from ab662fa to 992fd59 Compare February 9, 2024 19:27
@va-vfs-bot va-vfs-bot temporarily deployed to master/main/va-16652-mental-health-phone-numbers February 9, 2024 19:31 Inactive
@va-vfs-bot va-vfs-bot temporarily deployed to master/main/va-16652-mental-health-phone-numbers February 12, 2024 17:47 Inactive
@maxx1128 maxx1128 requested a review from laflannery February 12, 2024 21:02
@maxx1128
Copy link
Contributor Author

maxx1128 commented Feb 12, 2024

I'm tagging @laflannery for review here for the aria fix.

@maxx1128 maxx1128 force-pushed the va-16652-mental-health-phone-numbers branch from 224ef77 to 9a79f3d Compare February 15, 2024 20:52
@va-vfs-bot va-vfs-bot temporarily deployed to master/main/va-16652-mental-health-phone-numbers February 15, 2024 21:20 Inactive
@maxx1128 maxx1128 marked this pull request as ready for review February 16, 2024 13:17
@maxx1128 maxx1128 requested review from a team as code owners February 16, 2024 13:17
@maxx1128 maxx1128 force-pushed the va-16652-mental-health-phone-numbers branch from e89e1ed to 6b056db Compare February 16, 2024 14:51
@va-vfs-bot va-vfs-bot temporarily deployed to master/main/va-16652-mental-health-phone-numbers February 16, 2024 15:09 Inactive
Copy link
Contributor

@chriskim2311 chriskim2311 left a comment

Choose a reason for hiding this comment

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

LGTM

Copy link
Contributor

@eselkin eselkin left a comment

Choose a reason for hiding this comment

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

Looks good to me. Some of these were handled in the VAMC and VBA merger of service location and a lot of service location is changing in the integration branch.

I think this should go into that branch VACMS-16144-VAMC-ServiceLocationParagraphs

@maxx1128
Copy link
Contributor Author

@eselkin For merging this pull request into VACMS-16144-VAMC-ServiceLocationParagraphs, I'm concerned about the changes getting indirectly blocked or if changes in that pull request will change the code here. Would it be possible to merge this branch into main and then rebase off the changes in that branch instead?

@eselkin
Copy link
Contributor

eselkin commented Feb 23, 2024

@maxx1128 We can put this in main, but either way work will probably have to be redone somewhere. You might have to help me fix the other branch when I pull in the new changes from main.

@maxx1128 maxx1128 merged commit 24f28b3 into main Feb 23, 2024
22 checks passed
@maxx1128 maxx1128 deleted the va-16652-mental-health-phone-numbers branch February 23, 2024 21:37
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.

6 participants