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-19227: Adds Philippines as a US state for facility purposes #20068

Merged
merged 9 commits into from
Dec 19, 2024

Conversation

omahane
Copy link
Contributor

@omahane omahane commented Dec 9, 2024

Description

Relates to #19227

Testing done

Manually
Cypress (regression)

Screenshots

QA steps

Set up QA Content Publisher user

  • Log in as an admin
  • Assign the following roles to QA Content Publisher
    • Content creator - VAMC
    • Content publisher
  • Assign the following Section to QA Content Publisher
    • VISN 21

Add Philippines as a state to facilities

Create an event

  • Log in as QA Content Publisher
  • Make an event in the VA Manila Clinic Section
  • Add an address for the location that is in the Philippines
  • Save the node
  • Confirm that it saves without error
  • Confirm that the state is identified as "PH"

Set the location for facilities

  • Log in as an admin
  • Go to the Manila VA Clinic
  • Set the state as "Philippines"
  • Save the node
  • Confirm that it saves without error
  • Confirm that the state is identified as "PH"
  • Go to the VA Regional Benefit Office at U.S. Embassy in Manila
  • Set the state as "Philippines"
  • Save the node
  • Confirm that it saves without error
  • Confirm that the state is identified as "PH"

Create a news release

  • Log in as QA Content Publisher
  • Create a new release
  • Confirm that Philippines is available as a State
  • Fill out all required fields
  • Save the release as draft
  • Confirm that the node saves without error
  • Confirm that "PH" is present in the saved release on node:view

Edit a pre-existing news release

  • Go to the list of content in your section.
  • Filter for a news release
  • Edit the release
  • Confirm that the address is still in the US
  • Save the node
  • Confirm that the node saves without error
  • Confirm that the US is present in the saved release on node:view

Regression tests

  • Creating content with a non-Philippines state
    • Cypress
      • Event
      • Press release

Select Team for PR review

  • CMS Team
  • Public websites
  • Facilities
  • User support
  • Accelerated Publishing

@va-cms-bot va-cms-bot temporarily deployed to Tugboat December 9, 2024 22:16 Destroyed
Copy link

github-actions bot commented Dec 9, 2024

Checking composer.lock changes...

@va-cms-bot va-cms-bot temporarily deployed to Tugboat December 10, 2024 08:47 Destroyed
@va-cms-bot va-cms-bot temporarily deployed to Tugboat December 10, 2024 14:54 Destroyed
Copy link

Checking composer.lock changes...

@omahane
Copy link
Contributor Author

omahane commented Dec 10, 2024

@edmund-dunn @dsasser I have a change in this PR that replaces the default states in the US with a list that includes the Philippines, as we have facilities in the Philippines that are mapped to US addresses by way of the Lighthouse API:
Screenshot 2024-12-10 at 9 02 00 AM

Do you think it makes more sense to keep it in the va_gov_manila module because it's only necessary because of the Manila facilities? Or do you think that the scope of the change makes it necessary to add to a module that encompasses more features, such as va_gov_facilities or va_gov_backend?

@github-actions github-actions bot added CMS Team CMS Product team that manages both editor exp and devops Facilities Facilities products (VAMC, Vet Center, etc) labels Dec 10, 2024
@omahane omahane force-pushed the VACMS-19227-manila-ph-added-to-us branch from 5022f22 to 7578442 Compare December 10, 2024 18:07
Copy link

Checking composer.lock changes...

@omahane
Copy link
Contributor Author

omahane commented Dec 10, 2024

Do you think it makes more sense to keep it in the va_gov_manila module because it's only necessary because of the Manila facilities? Or do you think that the scope of the change makes it necessary to add to a module that encompasses more features, such as va_gov_facilities or va_gov_backend?

Based on the conversation on Slack, I chose to create a new module and add the code to that: https://dsva.slack.com/archives/CT4GZBM8F/p1733844761259909

@va-cms-bot va-cms-bot temporarily deployed to Tugboat December 10, 2024 18:14 Destroyed
Copy link

Checking composer.lock changes...

@omahane omahane requested a review from dsasser December 10, 2024 21:55
@va-cms-bot va-cms-bot temporarily deployed to Tugboat December 11, 2024 08:46 Destroyed
@va-cms-bot va-cms-bot temporarily deployed to Tugboat December 11, 2024 13:38 Destroyed
Copy link

Checking composer.lock changes...

@omahane omahane requested a review from edmund-dunn December 11, 2024 13:58
@omahane omahane force-pushed the VACMS-19227-manila-ph-added-to-us branch from 10553e8 to 72df54e Compare December 11, 2024 14:57
@va-cms-bot va-cms-bot temporarily deployed to Tugboat December 11, 2024 14:57 Destroyed
Copy link

Checking composer.lock changes...

@va-cms-bot va-cms-bot temporarily deployed to Tugboat December 11, 2024 18:55 Destroyed
Copy link

Checking composer.lock changes...

@va-cms-bot va-cms-bot temporarily deployed to Tugboat December 12, 2024 08:42 Destroyed
@va-cms-bot va-cms-bot temporarily deployed to Tugboat December 12, 2024 20:18 Destroyed
Copy link

Checking composer.lock changes...

@va-cms-bot va-cms-bot temporarily deployed to Tugboat December 13, 2024 08:44 Destroyed
@va-cms-bot va-cms-bot temporarily deployed to Tugboat December 14, 2024 08:44 Destroyed
@va-cms-bot va-cms-bot temporarily deployed to Tugboat December 15, 2024 08:46 Destroyed
Copy link
Contributor

@dsasser dsasser left a comment

Choose a reason for hiding this comment

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

Looks like possibly a rebase may have caused some things to get added to this PR that are not expected. See other comments.

Copy link

Checking composer.lock changes...

Copy link

The number of lines changed in composer.lock exceeds the acceptable threshold.

  • Lines changed: 8
  • Threshold: 200

This is a warning only. Please review the changes and ensure that they are acceptable.

@omahane omahane force-pushed the VACMS-19227-manila-ph-added-to-us branch from e249e6a to 659abe4 Compare December 19, 2024 00:48
@va-cms-bot va-cms-bot temporarily deployed to Tugboat December 19, 2024 00:49 Destroyed
Copy link

Checking composer.lock changes...

@va-cms-bot va-cms-bot temporarily deployed to Tugboat December 19, 2024 01:11 Destroyed
Copy link

Checking composer.lock changes...

@omahane omahane requested a review from dsasser December 19, 2024 01:11
Copy link
Contributor

@dsasser dsasser left a comment

Choose a reason for hiding this comment

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

Great work. Code changes are awesome and the QA steps passed. Philippines is now available for all addresses across the CMS!

Screenshot 2024-12-18 at 5 38 11 PM Screenshot 2024-12-18 at 5 29 05 PM

@va-cms-bot va-cms-bot temporarily deployed to Tugboat December 19, 2024 02:44 Destroyed
Copy link

Checking composer.lock changes...

@va-cms-bot
Copy link
Collaborator

Cypress Accessibility Violations

/test-data-facilis

ID: button-name
Impact: critical
Tags: cat.name-role-value, wcag2a, wcag412, section508, section508.22.a, ACT, TTv5, TT6.a
Description: Ensures buttons have discernible text
Help: Buttons must have discernible text
Nodes:

  • HTML: <button class="proofing-element-help" role="tooltip" data-proofing-help-title="About 'Page introduction' field" data-proofing-help="Add an introduction that helps visitors understand if information on the page is relevant to them."> <span aria-hidden="true">i</span> </button>
    Impact: critical
    Target: .field--name-field-intro-text-limited-html > .field__label > .proofing-element-help[role="tooltip"]
    Summary: Fix any of the following:
    Element does not have inner text that is visible to screen readers
    aria-label attribute does not exist or is empty
    aria-labelledby attribute does not exist, references elements that do not exist or references elements that are empty
    Element has no title attribute
    Element's default semantics were not overridden with role="none" or role="presentation"

  • HTML: <button class="proofing-element-help" role="tooltip" data-proofing-help-title="About 'Generate a table of contents from major headings' field" data-proofing-help="By checking this box, all h2's below this point on the page will be linked with with anchor links. This helps users navigate content on very long pages. Do not check this box unless there is at least 2 h2's on the page.">
    Impact: critical
    Target: .field--name-field-table-of-contents-boolean > .field__label > .proofing-element-help[role="tooltip"]
    Summary: Fix any of the following:
    Element does not have inner text that is visible to screen readers
    aria-label attribute does not exist or is empty
    aria-labelledby attribute does not exist, references elements that do not exist or references elements that are empty
    Element has no title attribute
    Element's default semantics were not overridden with role="none" or role="presentation"

  • HTML: <button class="proofing-element-help" role="tooltip" data-proofing-help-title="About 'Main content' field" data-proofing-help="The main body of the page, which appears below the featured content."> <span aria-hidden="true">i</span> </button>
    Impact: critical
    Target: button[data-proofing-help-title="About 'Main content' field"]
    Summary: Fix any of the following:
    Element does not have inner text that is visible to screen readers
    aria-label attribute does not exist or is empty
    aria-labelledby attribute does not exist, references elements that do not exist or references elements that are empty
    Element has no title attribute
    Element's default semantics were not overridden with role="none" or role="presentation"

@omahane omahane merged commit 6d4532b into main Dec 19, 2024
18 checks passed
@omahane omahane deleted the VACMS-19227-manila-ph-added-to-us branch December 19, 2024 04:52
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
CMS Team CMS Product team that manages both editor exp and devops Facilities Facilities products (VAMC, Vet Center, etc)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants