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

526ez | Pull disability list from Lighthouse API #41302

Closed
10 tasks
Mottie opened this issue May 12, 2022 · 16 comments
Closed
10 tasks

526ez | Pull disability list from Lighthouse API #41302

Mottie opened this issue May 12, 2022 · 16 comments
Assignees
Labels
526ez-Defects bug Something isn't working DBEX-Carbs Disability Benefits Experience - Team Carbs disability-experience frontend Lighthouse needs refinement needs further review and discussion at a refinement session needs-estimate on-hold

Comments

@Mottie
Copy link
Contributor

Mottie commented May 12, 2022

Description

Lighthouse provides a list of disabilities (/disabilities). We need to switch from loading in the static disabilities labels list (27KB) to that endpoint which has the most up-to-date list.

Open questions

  • What feature is this associated with?

    Used by 526ez's add new condition autocomplete input (/disability/file-disability-claim-form-21-526ez/new-disabilities/add)

  • What backend work needs to be done to complete this?
  • Is this a paginated API?

Acceptance Criteria

  • Hardcoded list is removed
  • List from Lighthouse is being pulled
  • List is searchable

Tasks

  • Set up an action to load in the list of disabilities from Lighthouse
  • Remove disabilityLabels.js file
  • Add unit tests
  • Add request spec tests
  • Update e2e tests

Definition of done

  • All tasks complete
  • All tests passing
@Mottie Mottie added frontend 526 v 1 and 2 tech-debt bmt-team-1 Benefits Management Tools Team #1 labels May 12, 2022
@jacobworrell jacobworrell added the benefits-management-tools includes Claim Status Tool, Benefits Letters, Payment History label May 16, 2022
@jerekshoe jerekshoe self-assigned this May 19, 2022
@jerekshoe
Copy link
Contributor

@jacobworrell after talking to @Mottie I'm realizing that there needs to be backend work done before we can start this ticket. vets-website doesn't interface with Lighthouse directly, but uses vets-api as a middle man instead.

@jacobworrell jacobworrell added Decision-Reviews-Team (Formerly squad-2) Label for issues being worked on by Decision Reviews on BMT & DR team and removed benefits-management-tools includes Claim Status Tool, Benefits Letters, Payment History labels Jun 2, 2022
@saderagsdale
Copy link
Contributor

@Mottie do we need to make a second story for the backend work that this depends on?

@saderagsdale saderagsdale added the needs-grooming Use this to designate any issues that need grooming from the team label Jun 3, 2022
@Mottie
Copy link
Contributor Author

Mottie commented Jun 3, 2022

Yes, we would need another ticket for backend work to set up communication with Lighthouse.

@jerekshoe jerekshoe removed their assignment Jun 7, 2022
@saderagsdale saderagsdale changed the title 526: Pull disability list from Lighthouse API 526ez | Pull disability list from Lighthouse API Jun 8, 2022
@saderagsdale saderagsdale added benefits-management-tools includes Claim Status Tool, Benefits Letters, Payment History and removed Decision-Reviews-Team (Formerly squad-2) Label for issues being worked on by Decision Reviews on BMT & DR team labels Jun 8, 2022
@jacobworrell jacobworrell added Decision-Reviews-Team (Formerly squad-2) Label for issues being worked on by Decision Reviews on BMT & DR team and removed benefits-management-tools includes Claim Status Tool, Benefits Letters, Payment History labels Jun 14, 2022
@saderagsdale saderagsdale self-assigned this Jun 16, 2022
@saderagsdale saderagsdale removed their assignment Sep 29, 2022
@saderagsdale saderagsdale removed the needs-grooming Use this to designate any issues that need grooming from the team label Sep 29, 2022
@saderagsdale saderagsdale removed bmt-team-1 Benefits Management Tools Team #1 Decision-Reviews-Team (Formerly squad-2) Label for issues being worked on by Decision Reviews on BMT & DR team labels Jan 18, 2023
@RakshindaAslam RakshindaAslam added bug Something isn't working and removed tech-debt labels May 23, 2023
@RakshindaAslam RakshindaAslam added needs refinement needs further review and discussion at a refinement session needs-estimate 526ez-Defects and removed 526 v 1 and 2 disability-experience labels May 23, 2023
@tblackwe
Copy link
Contributor

@RakshindaAslam This looks like a good candidate for Team Carbs (DBEX Squad 2) to take on.

@RakshindaAslam
Copy link
Contributor

@tblackwe - This could be blocked by the ongoing LH migration work. Good item for 16th minute today.

@RakshindaAslam RakshindaAslam added DBEX-Carbs Disability Benefits Experience - Team Carbs disability-experience labels May 26, 2023
@NB28VT
Copy link
Contributor

NB28VT commented Jun 6, 2023

Noting I removed the blocker on this since the referenced issue has been closed

@NB28VT
Copy link
Contributor

NB28VT commented Jun 6, 2023

Posting some initial findings from the website repo here. I haven't directly investigated the API endpoint as @micahaspyr was going to look into that; I also don't have a sandbox API key yet but will request one.

Findings Summary

There are actually a few places that reference the static disabilities list .

1. The autocomplete form input for adding a new disability, as referenced in the original ticket above.

    // Ideally, this would show the validation on the array itself (or the name
    // field in an array item), but that's not working.
    'ui:validations': [requireDisability],
    items: {
      condition: autosuggest.uiSchema(
        autoSuggestTitle,
        () =>
          Promise.resolve(
            Object.entries(disabilityLabels).map(([key, value]) => ({
              id: key,
              label: value,
            })),
          ),
        {

I validated this by hardcoding my own values in place of the mapping over disabilityLabels:

Screenshot 2023-06-06 at 10.36.52 AM.png

2. A mapping of the disability list values to lower case strings, called LOWERED_DISABILITY_DESCRIPTIONS.

This is referenced in the following validation code, which appears to happen on form submit. As documented in the code, we have a check that prevents a user submitting a disability name longer than 256 characters; if the name is present in our existing list of disabilities, we ignore that restriction

 // We're using a validator for length instead of adding a maxLength schema
  // property because the validator is only applied conditionally - when a user
  // chooses a disability from the list supplied to autosuggest, we don't care
  // about the length - we only care about the length of unique user-entered
  // disability names. We could've done this with `updateSchema` but this seems
  // lighter-touch.
  if (
    !LOWERED_DISABILITY_DESCRIPTIONS.includes(fieldData.toLowerCase()) &&
    fieldData.length > 255
  ) {
    err.addError('Condition names should be less than 256 characters');
  }

3. Another form pre-submittal callback which appears to update the form metadata submitted to the backend to include the code for the disability. I believe this happens anyway if the user selects one of the existing disability names, but presumably we still need this check in some cases

  // new disabilities that match a name on our mapped list need their
  // respective classification code added
  const addClassificationCodeToNewDisabilities = formData => {
    const { newDisabilities } = formData;
    if (!newDisabilities) {
      return formData;
    }

    const flippedDisabilityLabels = {};
    Object.entries(disabilityLabels).forEach(([code, description]) => {
      flippedDisabilityLabels[description?.toLowerCase()] = code;
    });

    const newDisabilitiesWithClassificationCodes = newDisabilities.map(
      disability => {
        const { condition } = disability;
        if (!condition) {
          return disability;
        }
        const loweredDisabilityName = condition?.toLowerCase();
        return flippedDisabilityLabels[loweredDisabilityName]
          ? _.set(
              'classificationCode',
              flippedDisabilityLabels[loweredDisabilityName],
              disability,
            )
          : disability;
      },
    );

    return _.set(
      'newDisabilities',
      newDisabilitiesWithClassificationCodes,
      formData,
    );
  };

4. In the "follow up" dropdown a user can use to select a disability that caused another disability

        causedByDisability: {
          'ui:title':
            'Please choose the disability that caused the new disability you’re claiming here.',
          'ui:required': (formData, index) =>
            formData.newDisabilities[index]?.cause === 'SECONDARY' &&
            getDisabilitiesList(formData, index).length > 0,
          'ui:options': {
            labels: disabilityLabels,
            updateSchema: (formData, primarySchema, primaryUISchema, index) => {
              const disabilitiesList = getDisabilitiesList(formData, index);
              if (!disabilitiesList.length) {
                return {
                  'ui:hidden': true,
                };
              }
              return {
                enum: disabilitiesList,
              };
            },
          },
        },

5. Two validation tests which references the static file directly. Note there are likely other tests/new tests we would write that cover this workflow as well

My Open Questions Remaining

  • Is it within the scope of this ticket to update all references to the static file? The issue description seems to indicate so, although it doesn't appear the original author was aware of the other places the reference that file.
  • What is the preferred mechanism for querying the Lighthouse API (via the vets-api) from within the website repo? The issue mentions setting up an "action". Does this refer to actions in the Redux sense?
  • Is there a way we can load this list from the API once and reference that state in all of the places documented below which now reference the hardcoded file?
  • How many entires are returned in total from the Lighthouse endpoint? It appears to be paginated, so we may need to make multiple calls based on the number of entries/pages/max entries per page (wondering if there is an existing wrapper for that type of data load in the front end somewhere)
  • Many of the disabilities are capitalized in the hardcoded file are they capitalized in the response from the API?
  • Still haven't investigated which existing tests cover this behavior/where we would put new tests, beyond the tests mentioned above which reference the disabilityList file directly.

@Mottie
Copy link
Contributor Author

Mottie commented Jun 6, 2023

  • Last time I looked, all references were pointing to the local disabilityList file. I believe the other places that use this list can be modified to use the form data instead of the list, e.g. we shouldn't need to validate an issue that was selected from the list
  • I don't think there is an established method to query lighthouse. For the service branches, we query and store the short list in memory to limit API requests. There is also a fallback to a local service branches list when the API fails (invalid token?). But in this case, would it be better to actively interact with the API to get recommendations? Would this overload the endpoint?
  • The disability list is very large. Loading it all in at once may not be ideal for mobile or slow Veteran with slow wifi.
  • The lighthouse endpoint pagination is optional, you can get a full list
  • Last time I checked, the list from Lighthouse is exactly the same as the disabilityList
  • Since autosuggest is utilized, the tests for this feature should be in the platform code - I haven't looked in a long time

@micahaspyr
Copy link
Contributor

Yep this list isn't queryable for matching text as the disabilities endpoint just returns everything or the scope of however many you want. I think this doesn't really solve much by hitting the lighthouse endpoint. I'm not sure how often the disabilities list changes, but we could just have an endpoint that returns a selected subset of data from the large json set. Seems the most practical way to me

@Mottie
Copy link
Contributor Author

Mottie commented Jun 6, 2023

The list has changed once in the 3.5 years I've been working on VA products.

@Mottie
Copy link
Contributor Author

Mottie commented Jun 7, 2023

Actually, I forgot that the separation location code loads in the list of locations and uses it in the autocomplete input

@austinjprice
Copy link
Contributor

Backend: ready to move to in progress from initial spike - medium complexity due to unknown PR/Testing requirements, but simple from a code implementation perspective / plus front end and backend deployment

Frontend: need @NB28VT input

@NB28VT
Copy link
Contributor

NB28VT commented Jun 21, 2023

@austinjprice front end insight:

Ready to move forward on the front end, also medium timebox due to unknown PR/Testing requirements. The testing paradigm is the one part I didn't really get to explore.

Overall could either be complex or relatively straightforward I think it will depend on what kind of feedback I can get early.

That being said, from the demo check-in today it sounds like we may be blocked on this pending guidance from other teams.

@austinjprice
Copy link
Contributor

Reviewed in Sprint review with feedback regarding dependencies around other teams working on this problem along with potential UX concerns. @austinjprice to follow up and connect the right parties

@austinjprice
Copy link
Contributor

@austinjprice
Copy link
Contributor

To remove from bug board and move to technical debt, pending discussion with team

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
526ez-Defects bug Something isn't working DBEX-Carbs Disability Benefits Experience - Team Carbs disability-experience frontend Lighthouse needs refinement needs further review and discussion at a refinement session needs-estimate on-hold
Projects
None yet
Development

No branches or pull requests

9 participants