Updates to `AddClaimantModal` #14202

jcq · 2020-05-06T23:18:37Z

Connects #14153

Description

This adds functionality to AddClaimantModal — implementing debounce on searchable dropdown and async fetching from backend API.

Acceptance Criteria

Code compiles correctly
Adds debounce functionality
Pulls select options via async fetch from backend
Saves claimant participant_id
Displays added claimant in radio list
Supports removing claimant from list
Fleshes out feature test

(removes mention of Pulac Cerullo in URL)

added proptypes; refactored into functional component

added story for `AddClaimantModal`; updated `SelectClaimant` to allow opening of `AddClaimantModal` if `attorneyFees` feature toggle is enabled;

… `SelectClaimant`

…Claimant` is true;

added ADD_CLAIMANT_MODAL_TITLE to COPY

…ed on returning participantId

(if appropriate feature toggle is set)

codeclimate · 2020-05-06T23:23:39Z

Code Climate has analyzed commit e6d2fa0 and detected 12 issues on this pull request.

Here's the issue category breakdown:

Category	Count
Duplication	2
Style	10

View more on Code Climate.

(avoids errors with async functions at top level of files)

(default — optionally supports a different async fn as a prop)

jcq · 2020-05-15T22:42:27Z

Note: will need to update story to include local async fn for generating names

va-bot · 2020-05-22T19:31:23Z

	2 Warnings
⚠️	This is a Big PR. Try to break this down if possible. Stacked pull requests encourage more detailed and thorough code reviews
⚠️	This PR modifies React components — consider adding/updating corresponding Storybook file

Generated by 🚫 Danger

jcq · 2020-05-22T22:32:04Z

Updated Screenshots:

leikkisa

Hey JC - This looks really good code-wise! Nice FE test. I also checked out the storybook locally which was fun.

I added a couple of minor comments, non-blocking since the feature generally is still a WIP. We might want to run screenshots by Rutvi, the spacing is a confusing layout for me (esp. for the legacy opt in field). Submitting successfully adds the claimant, but the add_issues page isn't loading because it depends on some claimant formatting stuff, but make sense for another ticket.

client/babel.config.js

leikkisa · 2020-05-28T07:33:16Z

client/app/intake/components/AddClaimantModal.jsx

      <div>
        <ReactMarkdown source={ADD_CLAIMANT_MODAL_DESCRIPTION} />
      </div>
-      <SearchableDropdown label="Claimant's name" options={dropdownOpts} onChange={handleChange} value={claimant} />
+      <SearchableDropdown


I know this may be up in the air but I'm not loving the searchable dropdown with one option in it

@rutvigupta-design Just wanted to flag this for you

I'd love to take a look at the screenshots, if possible!

client/test/app/intake/components/__snapshots__/AddClaimantModal.test.js.snap

client/app/intake/components/SelectClaimant.jsx

spec/feature/intake/intake_spec.rb

nanotone

Hm, while playing around with this and prepping for demo, I might've stumbled on a flaw in how we're using this component. The XHR and the React state contain results, but the <Select> is only showing those with an exact (case-insensitive) substring match.

This makes the dropdown pretty intolerant of typos, and defeats some of the fuzzy matching special sauce 😅

jcq · 2020-05-28T23:14:01Z

Hm, while playing around with this and prepping for demo, I might've stumbled on a flaw in how we're using this component. The XHR and the React state contain results, but the <Select> is only showing those with an exact (case-insensitive) substring match.

This makes the dropdown pretty intolerant of typos, and defeats some of the fuzzy matching special sauce 😅

Oof, good flag. Taking a look.

- loading indicator - filterOption fn - filterOptions fn (it passes relevant props to react-select)

…ons returned from backend

jcq · 2020-05-28T23:41:28Z

@nanotone I think the update I just pushed should cover the issue you raised (it will now return all options that it received from the backend).

nanotone

Thanks for fixing that filter issue so quickly! I think this PR is fine to be merged if you like.

One aspect that might warrant a closer look (maybe a separate PR) is the debouncing. When testing locally, it looks like I was able to fire off these 8 XHRs within a span of about 400 milliseconds. Does that square with how you understand the debouncing strategy?

jcq · 2020-05-29T03:08:27Z

When testing locally, it looks like I was able to fire off these 8 XHRs within a span of about 400 milliseconds. Does that square with how you understand the debouncing strategy?

Nope, looks like there must have been a regression on that. I'll look into it and follow up with another PR. Good catch!

nanotone · 2020-05-29T15:11:08Z

Nope, looks like there must have been a regression on that. I'll look into it and follow up with another PR. Good catch!

Food for thought: this feels like a great opportunity for a frontend test. It's confined to one or a few components, and is the kind of test that benefits from the snappiness of ditching rspec+capybara. I see that Jest comes with some nifty timer mocks, too.

This reverts commit 9b70c85.

This reverts commit 9b70c85. build and bake are failing. Likely a .js dependency in assets precompile step.

This reverts commit 7908ca0.

(Connects #14153) Reverts #14434, which reverted #14202. We reverted #14202 due to issues with builds failing in production. We have since changed things at the devops level, and we want to get this code merged in.

Connects #14430 ### Description Fixes regression from #14202; this brings fully functioning debounce functionality in `AddClaimantModal` along with additional JS tests ### Acceptance Criteria - [ ] Code compiles correctly - [ ] API calls are properly debounced in `AddClaimantModal`

JC Quirin and others added 19 commits May 1, 2020 14:40

Updated copy in label for LIT_SUPPORT_PULAC_CERULLO task action

31ef0e2

Updated the url used for the Notify Lit Support modal

64c9cd9

(removes mention of Pulac Cerullo in URL)

Updated existing SelectClaimant component:

33646a1

added proptypes; refactored into functional component

added frontend support for attorney_fees feature toggle

4d359f6

added AddClaimantModal;

195c3c7

added story for `AddClaimantModal`; updated `SelectClaimant` to allow opening of `AddClaimantModal` if `attorneyFees` feature toggle is enabled;

linting

ea91c10

basics for onCancel and stub for onSubmit for AddClaimantModal from…

a1652fd

… `SelectClaimant`

linting

3fa7d55

SelectClaimant only displays "Add Claimant" button if `veteranIsNot…

ef0af5e

…Claimant` is true;

Merge branch 'master' into jc/14136-add-claimant-modal

3d4aea7

fixed PropTypes on SelectClaimant;

bacaf53

added ADD_CLAIMANT_MODAL_TITLE to COPY

AddClaimantModal now pulls title from COPY;

bf0c125

Merge branch 'master' into jc/14136-add-claimant-modal

cd9288e

wired up missing closeHandler on AddClaimantModal

508a021

Merge branch 'master' into jc/14136-add-claimant-modal

7911600

restructured sample data to make it clear AddClaimantModal is focus…

3763f24

…ed on returning participantId

Added feature test to ensure "Add Claimant" option is available

da7f331

(if appropriate feature toggle is set)

Merge branch 'master' into jc/14136-add-claimant-modal

a522023

implemented debounce & async search to AddClaimantModal

5c99f45

jcq added the Team: Foxtrot 🦊 label May 6, 2020

JC Quirin added 6 commits May 6, 2020 16:33

SelectClaimant updates redux store w/ participant ID of added claimant

0328d07

Merge branch 'master' into jc/14153-update-add-claimant-modal

0784a4f

updated babel config for test environment

ab15ec1

(avoids errors with async functions at top level of files)

Merge branch 'master' into jc/14153-update-add-claimant-modal

9b06090

Merge branch 'master' into jc/14153-update-add-claimant-modal

07dd21a

updated AddClaimantModal to fetch data from '/intake/attorneys' endpoint

31509fe

(default — optionally supports a different async fn as a prop)

JC Quirin added 2 commits May 18, 2020 10:38

Merge branch 'master' into jc/14153-update-add-claimant-modal

538e2eb

built out ability to show and remove added attorney

d76212b

JC Quirin added 2 commits May 22, 2020 12:07

Merge branch 'master' into jc/14153-update-add-claimant-modal

00a9cd8

linting

b463684

Merge branch 'master' into jc/14153-update-add-claimant-modal

dcca175

leikkisa approved these changes May 28, 2020

View reviewed changes

moved "Add Claimant" tests from intake_spec to review_page_spec

8ca5460

nanotone suggested changes May 28, 2020

View reviewed changes

JC Quirin added 2 commits May 28, 2020 16:38

updated SearchableDropdown to support:

37eb858

- loading indicator - filterOption fn - filterOptions fn (it passes relevant props to react-select)

updated AddClaimantModal to ensure claimant dropdown shows all opti…

4ac2108

…ons returned from backend

JC Quirin added 4 commits May 28, 2020 16:58

linting

c63208d

Merge branch 'master' into jc/14153-update-add-claimant-modal

dc979e3

updated snapshot for AddClaimantModal

bfd59b6

Merge branch 'master' into jc/14153-update-add-claimant-modal

300f11f

nanotone approved these changes May 29, 2020

View reviewed changes

jcq added the Ready-to-Merge This PR is ready to be merged and will be picked up by va-bot to automatically merge to master label May 29, 2020

Merge branch 'master' into jc/14153-update-add-claimant-modal

e6d2fa0

va-bot merged commit 9b70c85 into master May 29, 2020

va-bot deleted the jc/14153-update-add-claimant-modal branch May 29, 2020 19:35

pkarman added a commit that referenced this pull request May 29, 2020

Revert "Updates to AddClaimantModal (#14202)"

8746862

This reverts commit 9b70c85.

va-bot pushed a commit that referenced this pull request May 29, 2020

Revert "Updates to AddClaimantModal (#14202)" (#14434)

7908ca0

This reverts commit 9b70c85. build and bake are failing. Likely a .js dependency in assets precompile step.

jcq mentioned this pull request Jun 2, 2020

Fixed Debounce in AddClaimantModal #14454

Merged

2 tasks

jcq added a commit that referenced this pull request Jun 8, 2020

Revert "Revert "Updates to AddClaimantModal (#14202)" (#14434)"

734b590

This reverts commit 7908ca0.

jcq mentioned this pull request Jun 8, 2020

Revert "Revert "Updates to AddClaimantModal (#14202)"" #14484

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Updates to `AddClaimantModal` #14202

Updates to `AddClaimantModal` #14202

jcq commented May 6, 2020 •

edited

Loading

codeclimate bot commented May 6, 2020 •

edited

Loading

jcq commented May 15, 2020

va-bot commented May 22, 2020 •

edited

Loading

jcq commented May 22, 2020

leikkisa left a comment

leikkisa May 28, 2020

jcq May 28, 2020

rutvigupta-design May 28, 2020

nanotone left a comment

jcq commented May 28, 2020

jcq commented May 28, 2020

nanotone left a comment

jcq commented May 29, 2020

nanotone commented May 29, 2020

Updates to AddClaimantModal #14202

Updates to AddClaimantModal #14202

Conversation

jcq commented May 6, 2020 • edited Loading

Description

Acceptance Criteria

codeclimate bot commented May 6, 2020 • edited Loading

jcq commented May 15, 2020

va-bot commented May 22, 2020 • edited Loading

jcq commented May 22, 2020

Updated Screenshots:

leikkisa left a comment

Choose a reason for hiding this comment

leikkisa May 28, 2020

Choose a reason for hiding this comment

jcq May 28, 2020

Choose a reason for hiding this comment

rutvigupta-design May 28, 2020

Choose a reason for hiding this comment

nanotone left a comment

Choose a reason for hiding this comment

jcq commented May 28, 2020

jcq commented May 28, 2020

nanotone left a comment

Choose a reason for hiding this comment

jcq commented May 29, 2020

nanotone commented May 29, 2020

Updates to `AddClaimantModal` #14202

Updates to `AddClaimantModal` #14202

jcq commented May 6, 2020 •

edited

Loading

codeclimate bot commented May 6, 2020 •

edited

Loading

va-bot commented May 22, 2020 •

edited

Loading