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-18659: Update clp node to be able to use url for field links #2192

Merged
merged 4 commits into from
Jul 22, 2024

Conversation

chriskim2311
Copy link
Contributor

@chriskim2311 chriskim2311 commented Jul 18, 2024

Summary

For this PR we are introducing URL paths when available for CLP field links when referencing internal links. We will defer back to URI if there is no URL available.

Changes for field links are made for the following fields in CLPs:
What you can do
Spotlight
Stories

Related issue(s)

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

Testing done

Tested in tugboat instance

CMS: https://main-jk4uouhulv1tbvo6a4fqxpyyjvjnazmf.ci.cms.va.gov/initiatives/prepare-for-vas-secure-sign-in-changes

Review Instance : https://main-jk4uouhulv1tbvo6a4fqxpyyjvjnazmf.ci.cms.va.gov/initiatives/prepare-for-vas-secure-sign-in-changes?_format=static_html

Screenshots

CMS for Stories Field Link Node Edit_Campaign_Landing_Page_Prepare_for_VA’s_secure_sign-in_changes___VA_gov_CMS
Stories Field Link URL Prepare_For_VA’s_Secure_Sign-In_Changes___Veterans_Affairs
Stories Field Link graphQL Prepare_For_VA’s_Secure_Sign-In_Changes___Veterans_Affairs
CMS for What you can do Field Link Node Edit_Campaign_Landing_Page_Prepare_for_VA’s_secure_sign-in_changes___VA_gov_CMS
What you can do Field Link URL and graphQL Prepare_For_VA’s_Secure_Sign-In_Changes___Veterans_Affairs Prepare_For_VA’s_Secure_Sign-In_Changes___Veterans_Affairs

What areas of the site does it impact?

CLPs

Acceptance criteria

  • Linked node IDs direct to the correct live VA.gov page or trigger a 403

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)

@chriskim2311 chriskim2311 marked this pull request as ready for review July 19, 2024 05:39
@chriskim2311 chriskim2311 requested review from a team as code owners July 19, 2024 05:39
@chriskim2311 chriskim2311 requested a review from laflannery July 19, 2024 05:40
@laflannery
Copy link

@chriskim2311 I'm building this on my tugboat because I want to test something else but for right now I have one question on the What you can do section:

  • The second 2 items have full URLs in the CMS but on the Front end they are going to dev.va URLs, that seems weird. Wouldn't they go to the main prod site because of the URL in Drupal?
    CMS
    image

Front end
image

@laflannery
Copy link

@chriskim2311 My tugboat eventually publishing and I see no other issues. If we can just investigation my previous question and figure out why it's pulling dev instead of the full URL that's the only issue I see

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.

code/queries lgtm

Copy link
Contributor

@randimays randimays left a comment

Choose a reason for hiding this comment

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

LGTM!

@chriskim2311 chriskim2311 merged commit 2b88627 into main Jul 22, 2024
24 checks passed
@chriskim2311 chriskim2311 deleted the 18659-clp-node-links branch July 22, 2024 20:00
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.

5 participants