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-16641: Test restricted archive workflow #17160

Merged
merged 3 commits into from
Feb 8, 2024
Merged

VACMS-16641: Test restricted archive workflow #17160

merged 3 commits into from
Feb 8, 2024

Conversation

Becapa
Copy link
Contributor

@Becapa Becapa commented Feb 7, 2024 •
edited by swirtSJW
Loading

Description

Relates to #16641

This adds two more Scenarios to tests/cypress/integration/features/content_type/facilities/vba/vba_facility.feature

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?

Checking that the new tests pass and that they cover the correct scenarios should be sufficient for QAing this.

  1. Content Editors should not be able to archive.
    • The steps in the Scenario labeled "Log in and try to archive a VBA Facility as a VBA editor" make sense as a way to verify that content editors are not able to archive.
    • Validate that the test above passes.
  2. Content admins should be able to archive.
    • The steps in the Scenario labeled "Log in and archive a VBA Facility as an admin" make sense as a way to verify that admins can archive and un-archive VBA Facilites.
    • Validate that the test above passes.

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 February 7, 2024 19:45 Destroyed
@Becapa Becapa requested review from swirtSJW and omahane February 7, 2024 19:51
@github-actions github-actions bot added the Facilities Facilities products (VAMC, Vet Center, etc) label Feb 7, 2024
swirtSJW
swirtSJW reviewed Feb 7, 2024
View reviewed changes
Copy link
Contributor

@swirtSJW swirtSJW 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 looking quite good @Becapa Two small suggestions.... and a bad suggestion that I retracted, but left it there for general philosophising .

tests/cypress/integration/features/content_type/facilities/vba/vba_facility.feature
Comment on lines +23 to +24
And I click the "Unlock" link
And I click the "Confirm break lock" button
Copy link
Contributor

Choose a reason for hiding this comment

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

Bonus points for having the test clean up its messes.

tests/cypress/integration/features/content_type/facilities/vba/vba_facility.feature Outdated
@@ -13,6 +13,37 @@ Feature: CMS User may effectively interact with the VBA Facility form
Then the primary tab "View" should exist
Then the primary tab "Edit" should not exist

Scenario: Log in and try to archive a VBA Facility as a VBA editor
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
Scenario: Log in and try to archive a VBA Facility as a VBA editor
Scenario: Test restricted_archive workflow prevents archiving a VBA Facility as a VBA editor.

Should: It helps our future selves if the scenerio reveals the intent.

tests/cypress/integration/features/content_type/facilities/vba/vba_facility.feature
Scenario: Log in and try to archive a VBA Facility as a VBA editor
When I am logged in as a user with the roles "content_creator_vba, content_publisher"
And my workbench access sections are set to "1065"
When I am at "/node/4063/edit"
Copy link
Contributor

Choose a reason for hiding this comment

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

Consider: Risky referencing a specific node. If someone actually archives that facility (if it closed) the test will break. It is not a huge issue but does create bumps in the road. I am not sure if it worth having this test create a new VBA facility as an admin, then test it, but that would reduce the risk.

edit: Given that the next test also references the node directly, I think it makes sense not sink more time into trying to have both of them create a node. It just slows things down.

tests/cypress/integration/features/content_type/facilities/vba/vba_facility.feature Outdated
And I click the "Unlock" link
And I click the "Confirm break lock" button

Scenario: Log in and archive a VBA Facility as an admin
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
Scenario: Log in and archive a VBA Facility as an admin
Scenario: Test restricted_archive workflow allows archiving a VBA Facility as a content_admin.

should: future self protection.

@Becapa Becapa force-pushed the VACMS-16641-test-restricted-archive-workflow branch from aa331a1 to 30cedf4 Compare February 8, 2024 15:32
@va-cms-bot va-cms-bot temporarily deployed to Tugboat February 8, 2024 15:32 Destroyed
swirtSJW
swirtSJW approved these changes Feb 8, 2024
View reviewed changes
Copy link
Contributor

@swirtSJW swirtSJW left a comment

Choose a reason for hiding this comment

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

Nice work @Becapa

@swirtSJW swirtSJW enabled auto-merge (squash) February 8, 2024 15:34
@swirtSJW swirtSJW force-pushed the VACMS-16641-test-restricted-archive-workflow branch from 30cedf4 to 8265d16 Compare February 8, 2024 16:37
@va-cms-bot va-cms-bot temporarily deployed to Tugboat February 8, 2024 16:37 Destroyed
@swirtSJW swirtSJW disabled auto-merge February 8, 2024 16:37
@swirtSJW swirtSJW merged commit 1e08ba2 into main Feb 8, 2024
13 of 16 checks passed
@swirtSJW swirtSJW deleted the VACMS-16641-test-restricted-archive-workflow branch February 8, 2024 16:37
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@swirtSJW swirtSJW swirtSJW approved these changes

@omahane omahane Awaiting requested review from omahane

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
@Becapa @swirtSJW @va-cms-bot