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-15565: reenable breadcrumb modules. #15572

Merged
merged 6 commits into from
Oct 24, 2023

Conversation

timcosgrove
Copy link
Contributor

@timcosgrove timcosgrove commented Oct 5, 2023

Description

Related to #15565

This makes changes to the breadcrumb data model to support breadcrumbs in JSON:API, which is necessary for AP. It is connected to a content-build PR which makes changes in how it references some breadcrumb data.

Testing done & Screenshots

  • Checked broken links and other tests, made sure that build succeeded
  • Checked examples of every content type against their production equivalents to make sure they remained consistent
  • Some links that were unnecessarily dependent on the breadcrumbs data shifted to a more stable mechanism

QA steps

Links to example pages from each affected content type. Unless noted, what should be checked is the breadcrumb.

Note that what we are checking here is Content Build output.

Events

Specifically check that the 'See more events' link works

FAQ

VAMC Facility

Story

Staff Profile

Press Release

Check both breadcrumb and 'See all press releases' link.

Q & A

Step by Step

Resources and Support Detail Page

VA Form

Acceptance criteria

  • All pages listed have identical breadcrumbs and if indicated other links

Definition of done

  • Events are logged appropriately
  • Documentation has been updated, if applicable
  • A link has been provided to the originating GitHub issue (or connected to it via ZenHub)
  • No sensitive information (i.e. PII/credentials/internal URLs/etc.) is captured in logging, hardcoded, or specs

@timcosgrove timcosgrove added the DO NOT MERGE Do not merge this PR label Oct 5, 2023
@va-cms-bot va-cms-bot temporarily deployed to Tugboat October 5, 2023 23:49 Destroyed
@va-cms-bot va-cms-bot temporarily deployed to Tugboat October 10, 2023 18:58 Destroyed
@va-cms-bot va-cms-bot temporarily deployed to Tugboat October 10, 2023 19:01 Destroyed
@va-cms-bot va-cms-bot temporarily deployed to Tugboat October 12, 2023 15:39 Destroyed
@va-cms-bot va-cms-bot temporarily deployed to Tugboat October 12, 2023 18:39 Destroyed
@va-cms-bot va-cms-bot temporarily deployed to Tugboat October 19, 2023 00:01 Destroyed
@va-cms-bot va-cms-bot temporarily deployed to Tugboat October 21, 2023 04:49 Destroyed
@timcosgrove timcosgrove marked this pull request as ready for review October 21, 2023 04:50
@timcosgrove timcosgrove changed the title DNM: VACMS-15565: reenable breadcrumb modules. VACMS-15565: reenable breadcrumb modules. Oct 21, 2023
Copy link
Contributor

@ndouglas ndouglas left a comment

Choose a reason for hiding this comment

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

In FAQ, the reference:

Screenshot 2023-10-23 at 10 37 09 AM

Preview build:

Screenshot 2023-10-23 at 10 37 13 AM

Copy link
Contributor

@ndouglas ndouglas left a comment

Choose a reason for hiding this comment

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

Stories:

Screenshot 2023-10-23 at 10 38 47 AM

Screenshot 2023-10-23 at 10 38 38 AM

@va-cms-bot va-cms-bot temporarily deployed to Tugboat October 23, 2023 16:06 Destroyed
@va-cms-bot va-cms-bot temporarily deployed to Tugboat October 23, 2023 20:42 Destroyed
@tjheffner
Copy link
Contributor

Sorry about that @ndouglas, this env was mistakenly rebuilt with the wrong front end version when you checked it prior. The "default" tag for this environment is 3 weeks old, so it was a little out of date when you peeked. 😅

I re-built it using the latest main (because of this merge to content-build: department-of-veterans-affairs/content-build#1744 ) and the breadcrumbs should look correct again. Whew.

@va-cms-bot va-cms-bot temporarily deployed to Tugboat October 24, 2023 15:28 Destroyed
@tjheffner tjheffner removed the DO NOT MERGE Do not merge this PR label Oct 24, 2023
@va-cms-bot va-cms-bot temporarily deployed to Tugboat October 24, 2023 16:52 Destroyed
@ndouglas
Copy link
Contributor

@tjheffner NP, at least you know I'm awake 😎

@tjheffner
Copy link
Contributor

@ndouglas breadcrumbs should be built against the correct environment to review at your convenience. All CI checks are passing 🎉

Copy link
Contributor

@ndouglas ndouglas left a comment

Choose a reason for hiding this comment

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

All pages' breadcrumbs were identical and the "See more events" and "See more news releases" links worked.

@ndouglas
Copy link
Contributor

Broken links:

==> Broken link report
{
  "generated": 1698173752969,
  "brokenPages": [
    {
      "path": "disability/file-an-appeal/board-of-veterans-appeals",
      "linkErrors": [
        {
          "html": "<a href=\"/disability/file-an-appeal\" id=\"6bc608658fc266d4a1fdbe226d63d86b\">Manage a legacy VA appeal</a>",
          "target": "/disability/file-an-appeal"
        }
      ]
    },
    {
      "path": "my-health/update-benefits-information-form-10-10ezr",
      "linkErrors": [
        {
          "html": "<a href=\"/my-heath/\">\n              My Health\n          </a>",
          "target": "/my-heath/"
        },
        {
          "html": "<a aria-current=\"page\" href=\"/update-benefits-information-form-10-10ezr\">\n              Update your VA health benefits information\n          </a>",
          "target": "/update-benefits-information-form-10-10ezr"
        }
      ]
    }
  ],
  "isHomepageBroken": false,
  "maxBrokenLinks": 5000,
  "brokenLinksCount": 3,
  "summary": "*`disability/file-an-appeal/board-of-veterans-appeals`* : \n```\n<a href=\"/disability/file-an-appeal\" id=\"6bc608658fc266d4a1fdbe226d63d86b\">Manage a legacy VA appeal</a>\n```\n*`my-health/update-benefits-information-form-10-10ezr`* : \n```\n<a href=\"/my-heath/\">\n              My Health\n          </a>\n```\n```\n<a aria-current=\"page\" href=\"/update-benefits-information-form-10-10ezr\">\n              Update your VA health benefits information\n          </a>\n```"
}

This does not appear to be connected to any issue with breadcrumbs AFAICT.

@tjheffner tjheffner merged commit e582059 into main Oct 24, 2023
15 checks passed
@tjheffner tjheffner deleted the VACMS-15565-reenable_breadcrumb_modules branch October 24, 2023 19:12
chri5tia pushed a commit that referenced this pull request Oct 25, 2023
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.

4 participants