department-of-veterans-affairs / va.gov-cms

Editor-centered management for Veteran-centered content.
https://prod.cms.va.gov
GNU General Public License v2.0
99 stars 69 forks source link

[CONSIDER] Extend VA Service synonyms/keywords text fields to >255chars #15685

Open davidmpickett opened 1 year ago

davidmpickett commented 1 year ago

User Story or Problem Statement

As a consumer of Content API I need term lists to able to contain an extensive list of synonyms So that I can better match user input to the VA Service that matches their need

Common Conditions

The Common Conditions fields are lists of keywords that help users know they are looking at the right service. Right now they are a single comma, separated text field, with a 255 char max:

Patient-friendly Name

The Patient-friendly Name fields aren't the exact same use case as Common Conditions. They aren't perfectly formatted as lowercase comma separated lists of words. Some times they are a single synonymic phrase

And when they do take the form of lists of words, they are usually formatted more like a sentence fragment:

They also may include acronyms or initialisms, which is technically a different type of relationship than a synonym and retaining case might be more important.

The inconsistency in formatting means that Patient-friendly Name will be even harder for API consumers to decompose. Consider how a script that treats " and " the same as it treats a comma might break down the examples above:

Steps for implementation

Increase field size via _va_gov_db_change_text_field_max_length() in va_gov_db/va_gov_db.install

Acceptance Criteria

xiongjaneg commented 1 year ago

@xiongjaneg stub out FE ticket to follow this one

jilladams commented 8 months ago

From scrum today, with Christian / Eli: In theory, it may actually be easier to parse the text field. Because then you can check the full field data and flag any match / partial match. Parsing an array may actually be more complex.

KISS files are generated to do a GraphQL query on the taxonomy/terms, or a way to create a view that can be downloaded as a CSV. Will do it the same as the SPIKE results suggested: download all the taxonomy terms including their common names, etc., and is done in content-build. Even if Drupal delivered an array, the front-end would likely have to reassemble all those items into a grouping that can be searched easily. So: more structure in the data isn't necessarily helpful, and may require more nested calls.

Also: this taxonomy is highly structured and has a limited editor base. If we were opening this up to all VAMC editors, we could make an argument for arrays as easier to manage. But: given the editor base, that's not maybe the most valid argument.

@omahane to think harder about this alongside #14993 and we will revisit next week in scrum to finalize decision.

jilladams commented 7 months ago

Reviewed with Michelle, concluded the UX here isn't necessary for Admins editing the taxonomy unless it's a win in terms of data structure for the front end. Since it's not, @mmiddaugh to clarify with DaveC re: if she's missing anything. If not, we'll close as no-op.

jilladams commented 7 months ago

Noting: if we do make this change, data will still need to be squashed back down for FE search. There's a package, Fuse, available to the FE to parse the more structured data if we go this route. We're reluctant to add packages to support single use functionality, but could if we need to.

Christian also flagged text fields ( Common Names, Patient Friendly Names) is capped at 255 chars, which might be a concern.

jilladams commented 7 months ago

We may be able to up the character count on the text fields, if that's the main concern. Maybe. @omahane to verify if that can be done in the schema.

jilladams commented 7 months ago

@omahane Michelle also thought about: we push services to LH, and will need to make sure we don't affect the success of that push if we change this, or will need to account for it.

omahane commented 7 months ago

Regarding the services push to Lighthouse, we don't push the common conditions.

omahane commented 7 months ago

Regarding changing the schema to increase the field size: _va_gov_db_change_text_field_max_length() is in va_gov_db/va_gov_db.install to do just that.

jilladams commented 7 months ago

Final answer from scrum:

davidmpickett commented 7 months ago

Quick analysis of Common Conditions & Patient Friendly Names character counts.

Common Conditions

Patient-Friendly Names

https://dvagov.sharepoint.com/:x:/s/SitewideFacilitiesTeam/ERKcUmY00NlJtCQWzo2TSZEBu1AeJoyRE7rhZJgO9MpqYg?e=fpLSks

jilladams commented 7 months ago

2 things:

  1. I updated ticket body to reflect that this is now an optional backlog ticket for converting to >255chars for these 2 fields. The old ticket body is below, in case we need it bc of question 2.
  2. The Patient friendly name notes in ticket body are something we didn't really discuss when we decided not to move to data arrays. Those are sentence-y-er and include commas in their own right, which makes it harder to explode on commas for the FE. @eselkin could you read through the Patient Friendly Name notes Dave included on that in ticket body and just triple confirm that we are ok leaving things as is? (cc @omahane )

Old ticket definition notes

Ruled out: Data arrays

Facilities team in April 2024 considered converting these fields to become data arrays but ruled out this approach. Notes below are from that proposal, for reference. Data arrays, e.g.

This was ruled out for a few reasons (in comments below):

Potential path for array implementation

Copy what Public Websites did for field_va_benefit_plain_name on the VA Benefit Taxonomy

Screenshot 2023-10-13 201514

Tentacles

Bonus side effect

If we add a new field and migrate existing data into it, we could also clean up some of the legacy naming conventions

If we did this, ACs would've been

jilladams commented 7 months ago

Or maybe we could just separate them on something besides commas?

mmiddaugh commented 7 months ago

Per Dave P's comment, we have no examples of insufficient character counts Sending this to the icebox for now

Agile6MSkinner commented 3 months ago

Going to remove from epic.