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.

Sign up for GitHub

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

Jump to bottom

VACMS-15715 Create and publish VA police pages. #16277

Merged
merged 3 commits into from
Dec 7, 2023
Merged

VACMS-15715 Create and publish VA police pages. #16277

merged 3 commits into from
Dec 7, 2023

Conversation

swirtSJW
Copy link
Contributor

@swirtSJW swirtSJW commented Dec 2, 2023 •
edited by omahane
Loading

Description

Relates to #15715

Testing done

Screenshots

image

image

image

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?

As an admin

  1. Go to the content View filtered for VA Police
    • Validate that there are 139 VA Police pages.
    • Validate they are all in draft. (this is so the menu items do not show on FE until we want them too)
  2. Click on 5 VA Police pages
    • Validate that in each of them, 'VA police' is nearly above "Policies" ('nearly' because in most cases they will be placed adjacent to but on top of "Policies" However in some cases there may be multiple menu items that appear above policies, all with the exact same weight. This should be rare.)

Definition of Done

  • Documentation has been updated, if applicable.
  • Tests have been added if necessary.
  • Automated tests have passed.
  • Code Quality Tests have passed.
  • Acceptance Criteria in related issue are met.
  • Manual Code Review Approved.
  • If there are field changes, front end output has been thoroughly checked.

Select Team for PR review

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

Is this PR blocked by another PR?

  • DO NOT MERGE

Does this PR need review from a Product Owner

  • Needs PO review

CMS user-facing announcement

Is an announcement needed to let editors know of this change?

  • Yes, and it's written in issue ____ and queued for publication.
    • Merge and ping the UX writer so they are ready to publish after deployment
  • Yes, but it hasn't yet been written
    • Don't merge yet -- ping the UX writer to write and queue content
  • No announcement is needed for this code change.
    • Merge & carry on unburdened by announcements

@va-cms-bot va-cms-bot temporarily deployed to Tugboat December 2, 2023 04:47 Destroyed
@swirtSJW
Copy link
Contributor Author

swirtSJW commented Dec 2, 2023 •
edited
Loading

Page creation is working fine. The menu placement is in the wrong spot though. Continuing to work on that. Tugboat will continue to fail to build until the content type code makes it to prod on Monday.
image

TLDR chicken and egg problem with trying to build content in update BEFORE the content type exists (config import)

@swirtSJW swirtSJW force-pushed the VACMS-15715-generate-va-police-pages branch from 7c03441 to 975a6aa Compare December 6, 2023 01:47
@va-cms-bot va-cms-bot temporarily deployed to Tugboat December 6, 2023 01:47 Destroyed
@swirtSJW swirtSJW force-pushed the VACMS-15715-generate-va-police-pages branch from 975a6aa to 088d576 Compare December 7, 2023 04:30
@va-cms-bot va-cms-bot temporarily deployed to Tugboat December 7, 2023 04:30 Destroyed
@swirtSJW
Copy link
Contributor Author

swirtSJW commented Dec 7, 2023

Log on tugboat for deploy step
image

@swirtSJW
Copy link
Contributor Author

swirtSJW commented Dec 7, 2023

There is a php failure that I think is fleeting, but am waiting to see the log... which is making me sleepy, so calling it a night.

@swirtSJW swirtSJW marked this pull request as ready for review December 7, 2023 04:53
@swirtSJW swirtSJW requested a review from a team as a code owner December 7, 2023 04:53
@swirtSJW swirtSJW requested a review from omahane December 7, 2023 04:53
omahane
omahane previously approved these changes Dec 7, 2023
View reviewed changes
Copy link
Contributor

@omahane omahane left a comment

Choose a reason for hiding this comment

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

This is excellent, so well thought-out and structured. I am just glad you picked this issue up, as that menu placement was very tricky.

docroot/modules/custom/va_gov_vamc/va_gov_vamc.install Outdated
foreach ($sandbox['items_to_process'] as $system_nid => $system_info) {
if (++$i > 25) {
break;
}

$new_tt_page = Node::create([
Copy link
Contributor

@omahane omahane Dec 7, 2023 •
edited
Loading

Choose a reason for hiding this comment

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

Minor quibble: $new_tt_page made sense when this function was about making top task pages. But now that its more generic, it might be better to be $new_page. (I was trying to figure out what "tt" meant until I looked at the old docblock.)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yah that is a good call. I will fix.

@github-actions github-actions bot added the Facilities Facilities products (VAMC, Vet Center, etc) label Dec 7, 2023
@swirtSJW
Copy link
Contributor Author

swirtSJW commented Dec 7, 2023

This is odd. Showing two failing tests but I can't even find record in the terminal that they ran. I'll make Omahane's recommended change and maybe we'll get proper test output this time.

@va-cms-bot va-cms-bot temporarily deployed to Tugboat December 7, 2023 13:19 Destroyed
@va-cms-bot va-cms-bot temporarily deployed to Tugboat December 7, 2023 16:58 Destroyed
@swirtSJW
Copy link
Contributor Author

swirtSJW commented Dec 7, 2023

image

@swirtSJW swirtSJW force-pushed the VACMS-15715-generate-va-police-pages branch from d4187bc to 3aec054 Compare December 7, 2023 18:17
@va-cms-bot va-cms-bot temporarily deployed to Tugboat December 7, 2023 18:17 Destroyed
@swirtSJW swirtSJW merged commit 1e615d9 into main Dec 7, 2023
13 checks passed
@swirtSJW swirtSJW deleted the VACMS-15715-generate-va-police-pages branch December 7, 2023 18:18
ndouglas pushed a commit that referenced this pull request Dec 7, 2023
@swirtSJW @ndouglas
VACMS-15715 Create and publish VA police pages. (#16277)
9d6bba5 
* VACMS-15715 Create and publish VA police pages.

* VACMS-15715 Build VA police menu items.

* VACMS-15715 Make top task variable more generic.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@omahane omahane omahane left review comments

Assignees
No one assigned
Labels
Facilities Facilities products (VAMC, Vet Center, etc)
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@swirtSJW @omahane @va-cms-bot