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

VAGAOV-TEAM-97941: Adds Form Builder role and permission #20192

Merged
merged 8 commits into from
Jan 7, 2025

Conversation

ryguyk
Copy link
Contributor

@ryguyk ryguyk commented Jan 1, 2025

Description

Closes department-of-veterans-affairs/va.gov-team#97941

Testing done

  • Unit tests added/updated.
  • Manual tests to ensure only expected routes are updated (these are the Form Builder routes).

Screenshots

Without correct role/permission:

image

With correct role/permission:

image

QA steps

Create two new users:

  • Ensure role Form Builder user is in the list of roles.
  1. Create a user with role Form Builder user.
  2. Create a user with (only) role Authenticated user.

Log in the new Form Builder user.

  1. Navigate to /form-builder/intro.
    • Validate that the user has access to the page.

Log in the new Authenticated user.

  1. Navigate to /form-builder/intro.
    • Validate that the user does NOT have access to the page.
  2. Navigate to a page that should be accessible like /admin/content.
  • Validate that the user DOES have access to the page. This test ensures that the routing mechanism is not affecting routes outside of form-builder/.

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
  • Form Builder

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

@ryguyk ryguyk requested a review from a team as a code owner January 1, 2025 16:52
@va-cms-bot va-cms-bot temporarily deployed to Tugboat January 1, 2025 16:52 Destroyed
@ryguyk ryguyk requested a review from derekhouck January 1, 2025 16:52
Copy link

github-actions bot commented Jan 1, 2025

Checking composer.lock changes...

@github-actions github-actions bot added CMS Team CMS Product team that manages both editor exp and devops DO NOT MERGE Do not merge this PR labels Jan 1, 2025
@va-cms-bot va-cms-bot temporarily deployed to Tugboat January 2, 2025 08:49 Destroyed
@derekhouck
Copy link
Contributor

Note: As a form builder user, if I click on "Continue" on the Name and DOB page of the Form Builder, I'm currently taken to https://va-gov-cms.ddev.site/node/$NODE_ID/edit and given an "Access denied" message because I do not have the necessary permissions to edit a Digital Form. I imagine this will be fixed as more pages are added to the Form Builder UI. Is this the expected behavior at the moment?

derekhouck
derekhouck previously approved these changes Jan 2, 2025
Copy link
Contributor

@derekhouck derekhouck left a comment

Choose a reason for hiding this comment

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

Code all looks good. I had one question about the current Form Builder experience for a non-admin user with the form builder permission, but it seems to be intended behavior, so I'm approving.

@va-cms-bot va-cms-bot temporarily deployed to Tugboat January 3, 2025 08:47 Destroyed
@ryguyk
Copy link
Contributor Author

ryguyk commented Jan 3, 2025

Note: As a form builder user, if I click on "Continue" on the Name and DOB page of the Form Builder, I'm currently taken to https://va-gov-cms.ddev.site/node/$NODE_ID/edit and given an "Access denied" message because I do not have the necessary permissions to edit a Digital Form. I imagine this will be fixed as more pages are added to the Form Builder UI. Is this the expected behavior at the moment?

Navigating to the page in question is a temporary way to demonstrate that the work being done in the Form Builder is having the desired effect. In its final state, there will be no such navigation between the Form Builder and the default Digital Form node-edit screen. The experience you describe actually helps confirm that the intended behavior of this PR is happening - a Form Builder user only has permission access form builder. That user does not have access to view/edit Digital Forms. So the access control is working!

@va-cms-bot va-cms-bot temporarily deployed to Tugboat January 3, 2025 17:05 Destroyed
Copy link

github-actions bot commented Jan 3, 2025

Checking composer.lock changes...

derekhouck
derekhouck previously approved these changes Jan 3, 2025
@va-cms-bot va-cms-bot temporarily deployed to Tugboat January 4, 2025 08:51 Destroyed
@va-cms-bot va-cms-bot temporarily deployed to Tugboat January 5, 2025 08:49 Destroyed
@va-cms-bot va-cms-bot temporarily deployed to Tugboat January 6, 2025 08:48 Destroyed
@ryguyk ryguyk force-pushed the VAGOV-TEAM-97941-form-builder-role-perm branch from a65522d to eb6944c Compare January 6, 2025 21:48
@va-cms-bot va-cms-bot temporarily deployed to Tugboat January 6, 2025 21:49 Destroyed
Copy link

github-actions bot commented Jan 6, 2025

Checking composer.lock changes...

@ryguyk ryguyk removed the DO NOT MERGE Do not merge this PR label Jan 6, 2025
@va-cms-bot va-cms-bot temporarily deployed to Tugboat January 7, 2025 08:46 Destroyed
ryguyk added 2 commits January 7, 2025 07:55
…existing tests effectively test the RouteSubscriber functionality.
…n as a second parameter. We'll set the permission whenever one is not set.
@ryguyk ryguyk force-pushed the VAGOV-TEAM-97941-form-builder-role-perm branch from eb6944c to dcbb5e9 Compare January 7, 2025 13:55
@va-cms-bot va-cms-bot temporarily deployed to Tugboat January 7, 2025 13:55 Destroyed
Copy link

github-actions bot commented Jan 7, 2025

Checking composer.lock changes...

@va-cms-bot
Copy link
Collaborator

Cypress Accessibility Violations

/test-data-voluptas

ID: button-name
Impact: critical
Tags: cat.name-role-value, wcag2a, wcag412, section508, section508.22.a, TTv5, TT6.a, EN-301-549, EN-9.4.1.2, ACT
Description: Ensure 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 does not have an implicit (wrapped) <label>
    Element does not have an explicit <label>
    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 does not have an implicit (wrapped) <label>
    Element does not have an explicit <label>
    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 does not have an implicit (wrapped) <label>
    Element does not have an explicit <label>
    Element's default semantics were not overridden with role="none" or role="presentation"

@ryguyk ryguyk merged commit 0993b35 into main Jan 7, 2025
18 checks passed
@ryguyk ryguyk deleted the VAGOV-TEAM-97941-form-builder-role-perm branch January 7, 2025 15:57
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
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Create "Form Builder" role with correct permissions
4 participants