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

Add server-side validation on ext field in phone paragraph in Drupal #17863

Open
8 tasks
Tracked by #16073 ...
laflannery opened this issue Apr 16, 2024 · 15 comments
Open
8 tasks
Tracked by #16073 ...

Add server-side validation on ext field in phone paragraph in Drupal #17863

laflannery opened this issue Apr 16, 2024 · 15 comments
Assignees
Labels
Drupal engineering CMS team practice area Facilities Facilities products (VAMC, Vet Center, etc) Facility services (multi-product) A label for issues that affect how services are handled across multiple facility products points-3 sitewide Technical debt

Comments

@laflannery
Copy link
Contributor

laflannery commented Apr 16, 2024

[2025-01-22] Blocked on tasks in this comment
[2025-01-08] We are not allowing extra extensions, so the ACs are accurate as written.
[2024-10-10] We need to make a call on comma separated multiple extensions allowed or not, and factor that into ACs on this ticket (& make follow on tix if so)
[2024-09-18]: Should block on FE work, and Change Management with editors before we do this.

Description

The Extension number field in the phone paragraph type in Drupal should not allow non-numerical characters to be saved. We should add server-side validation that prevents the editor from saving content such as "ext. 123" and only allows "123" for example.

Acceptance Criteria

  • Server-side validation has been added to the Extension field to prevent non-numerical characters from being saved
  • Error message "Enter an extension that contains only numeric characters (0-9)" is displayed when input fails validation
  • Add input mask for numerics only

QA

  • Design review
  • Accessibility review
  • Validate on multiple content types that use the phone paragraph
  • Test multiple non-numerical character types (symbol, alpha, non-us, wildcard and Boolean)
  • Test string length
@laflannery laflannery added Needs refining Issue status Drupal engineering CMS team practice area Facilities Facilities products (VAMC, Vet Center, etc) labels Apr 16, 2024
@davidmpickett
Copy link
Contributor

@laflannery FYI. I removed your question about "Can we add character counters?" and added the specific guidance for updating them in Drupal

@davidmpickett davidmpickett added the Facility services (multi-product) A label for issues that affect how services are handled across multiple facility products label Jun 4, 2024
@laflannery laflannery changed the title Update validation on phone paragraph type field in Drupal Update server-side validation on phone paragraph type field in Drupal Jun 6, 2024
@laflannery laflannery changed the title Update server-side validation on phone paragraph type field in Drupal Update server-side validation on ext field in phone paragraph in Drupal Jun 6, 2024
@laflannery laflannery changed the title Update server-side validation on ext field in phone paragraph in Drupal Add server-side validation on ext field in phone paragraph in Drupal Jun 6, 2024
@Agile6MSkinner
Copy link

@Agile6MSkinner to work with @omahane to refine and point when he gets back

@davidmpickett
Copy link
Contributor

For traceability the two tickets that were splintered off of this ticket are:

@Agile6MSkinner
Copy link

@davidmpickett See AC#2. We need copy for the error message before moving to "Ready."

@davidmpickett
Copy link
Contributor

Given that the field already has help text saying "Enter only numbers" this error message feels redundant, but tried to make it a little more explicit "Enter an extension that contains only numeric characters (0-9)"

@davidmpickett davidmpickett removed their assignment Jun 26, 2024
@Agile6MSkinner
Copy link

@davidmpickett To make it less redundant we could say something like "You have entered non-numeric characters. Please check your entry and try again."

@jilladams
Copy link
Contributor

This ticket is technically ready but flags have been thrown on comments in #17861 that we want to consider order of operations for migration in context for this work. @Agile6MSkinner to close loop.

@jilladams jilladams added the Blocked Issues that are blocked on factors other than blocking issues. label Aug 7, 2024
@Agile6MSkinner Agile6MSkinner removed the Blocked Issues that are blocked on factors other than blocking issues. label Sep 13, 2024
@FranECross
Copy link

@jilladams @Agile6MSkinner I think this is the ticket mentioned in planning today to have me pull part of it out into another ticket, but since this is a Facilities ticket that I'm unfamiliar with, I think it was supposed to target Michael to do this? Or maybe I wrote down the wrong ticket number?

@jilladams
Copy link
Contributor

@FranECross it was #18736. I'll leave a note there re: the change.

@davidmpickett
Copy link
Contributor

@laflannery Should we update this ticket to allow commas in the extension field based on your recent discovery?

@jilladams
Copy link
Contributor

From UX sync:

  • Ticket is currently suggesting to validate numbers only. If we want to take advantage of this multi-extension comma thing, we need to:
    • Update 17863 to allow the extension validation to allow commas.
    • update new phone field helptext to explain the commas
    • update FE templates to handle the commas correctly within the component

There might be a Change Management advantage to doing it now, vs. later. We need to decide before we take on this ticket.

@Agile6MSkinner
Copy link

We're making the assumption that commas are how we want to handle the multiple extensions or phone tree options thing, and that doesn't feel thought through to me. We risk watering down the benefit that this work was meant to deliver, which is hardening, streamlining and standardizing how phone numbers are entered and displayed.

My vote is that we spin off those questions and perhaps combine them with the international/vanity and other various phone number enhancement stories that are out there for future consideration.

@jilladams jilladams changed the title Add server-side validation on ext field in phone paragraph in Drupal Add server-side validation on ext field in phone paragraph in Drupal & tech debt removal Nov 26, 2024
@jilladams jilladams changed the title Add server-side validation on ext field in phone paragraph in Drupal & tech debt removal Add server-side validation on ext field in phone paragraph in Drupal Nov 26, 2024
@jilladams
Copy link
Contributor

@Agile6MSkinner Flagging that from this slack thread, it'd be nice to prioritize adding this validation once CM tasks are done for Phone #s. However: there are open questions about ACs and use of commas, etc here that need to be resolved with a product call in order to make it actionable.

@Agile6MSkinner Agile6MSkinner self-assigned this Dec 9, 2024
@Shamiaalam
Copy link

@Agile6MSkinner I reviewed this issue with @laflannery and I believe this issue will need accessibility review.

@jilladams
Copy link
Contributor

jilladams commented Jan 22, 2025

BLOCKED: We need to:

  1. Audit all existing Ext fields (across all content types where the Phone paragraph is used, not just the 3 content types we handled in this epic)
  2. Identify cases where Editors are using non-numerical characters
  3. Run a change management process with those editors: warning, change management, cleanup.
    4. Need an audit ticket
    5. Need a Change management ticket / timeline
    6. Need ticket to write a script that fixes the identified cases

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Drupal engineering CMS team practice area Facilities Facilities products (VAMC, Vet Center, etc) Facility services (multi-product) A label for issues that affect how services are handled across multiple facility products points-3 sitewide Technical debt
Projects
None yet
Development

No branches or pull requests

6 participants