Add edit icon and form for approved SIT requests #2004

LeDeep · 2019-04-11T21:25:10Z

Description

Adds edit icon and form for approving sit requests. Also adds the logic to handle the various transitions between the icons and forms in the sit panel.

Reviewer Notes

Might make sense to break up the logic a bit so it's not all in one file. Conversely, we could end up with a bunch of files that are a pain to navigate.

Setup

Add any steps or code to run in this section to help others prepare to run your code:

make db_dev_e2e_populate
make server_run
make client_run

In the Office app, select an HHG record with an APPROVED SIT. In the SIT panel you will see an 'Edit' link. Clicking on this will open up a form that shoes the various SIT information, with an editable authorized notes field. All other fields are informational and not editable.

References

Pivotal story for this change

Screenshots

codecov · 2019-04-11T23:25:19Z

Codecov Report

Merging #2004 into master will increase coverage by 0.17%.
The diff coverage is n/a.

@@            Coverage Diff             @@
##           master    #2004      +/-   ##
==========================================
+ Coverage   60.38%   60.56%   +0.17%     
==========================================
  Files         193      193              
  Lines       12525    12496      -29     
==========================================
+ Hits         7563     7567       +4     
+ Misses       4073     4045      -28     
+ Partials      889      884       -5

kimallen · 2019-04-30T00:15:26Z

src/shared/StorageInTransit/OfficeEditor.test.jsx

+    });
+
+    it('renders without crashing', () => {
+      expect(wrapper.exists('.storage-in-transit-panel-modal')).toBe(true);


Could you start using data-cy rather than class names to identify elements in integration tests as we transition to using css modules?

kimallen · 2019-04-30T00:16:13Z

src/shared/StorageInTransit/OfficeEditor.test.jsx

+    });
+
+    it('buttons are enabled', () => {
+      expect(wrapper.find('button.usa-button-primary').prop('disabled')).toBe(false);


same as above- use data-cy attribute rather than class name

kimallen · 2019-04-30T00:19:50Z

src/shared/StorageInTransit/OfficeEditor.jsx

+            </div>
+            <div className="usa-width-one-half align-right">
+              <button
+                className="button usa-button-primary storage-in-transit-request-form-send-request-button"


Do you use this class name storage-in-transit-request-form-send-request-button anywhere?

kimallen · 2019-04-30T00:25:15Z

src/shared/StorageInTransit/StorageInTransit.css

@@ -97,10 +93,18 @@
  font-weight: normal;
 }

+.storage-in-transit-office-edit-form {
+  font-weight: normal;


All the storage in transit forms have this font-weight: normal applied to them. Might be nice if we create a new class to use for all the forms rather than apply the style to each unique class.

kimallen · 2019-04-30T00:25:35Z

src/shared/StorageInTransit/StorageInTransit.css

 .storage-in-transit-office-approval-form textarea {
  height: 9rem;
 }

+.storage-in-transit-office-edit-form textarea {
+  height: 9rem;


Same as above comment regarding creating a new class (for height: 9rem)

kimallen · 2019-04-30T15:15:29Z

The story for this is an edit form for approved SIT requests, though your demo shows denied SIT requests. Does this cover both approved and denied edit form stories?

kimallen · 2019-05-01T18:05:16Z

src/shared/StorageInTransit/StorageInTransit.css

  height: 9rem;
 }

-.storage-in-transit-office-deny-form {
+.place-in-sit-form {


nitpick: this could also use the .storage-in-transit-form since it has the same styling. Same with .storage-in-transit-office-deny-form below.

I didn't for place-in-sit-form bc that class gets called in the test for PlaceInSitForm.

storage-in-transit-office-deny-form has some additional css for the textarea (line 104), so that's why I kept that class. Otherwise I would have to add storage-in-transit-form and storage-in-transit-office-deny-form in that component. I thought this way was better, because the component reads cleaner.

let me know if you agree/disagree. I can make the update if you think it would be better.

I think trying to refactor the CSS more will be a rabbit-hole- it's fine to leave it as is. As I'm working on the PlaceInSit, I will replace the references to classes in the tests and see if it makes sense to combine.

kimallen · 2019-05-01T18:13:01Z

I ran it, and the form shows up fine, although I get an error:
index.js:1446 Warning: Failed prop type: Invalid prop `value` of type `object` supplied to `PanelField`, expected `string`. in PanelField (at StorageInTransitOfficeEditForm.jsx:42) in StorageInTransitOfficeEditForm (created by Form(StorageInTransitOfficeEditForm)) in Form(StorageInTransitOfficeEditForm) (created by

kimallen · 2019-05-01T18:14:57Z

I'd still like to see use of data-cy in your integration step instead of class names. The syntax is '[data-cy="invoice-detail"]'. Let me know if you have questions how to implement it.

LeDeep · 2019-05-01T18:17:38Z

I ran it, and the form shows up fine, although I get an error:
index.js:1446 Warning: Failed prop type: Invalid prop `value` of type `object` supplied to `PanelField`, expected `string`. in PanelField (at StorageInTransitOfficeEditForm.jsx:42) in StorageInTransitOfficeEditForm (created by Form(StorageInTransitOfficeEditForm)) in Form(StorageInTransitOfficeEditForm) (created by

just pushed up a fix for this.

kimallen

All working for me now!

Add edit icon and form for approved SIT requests

8e88357

LeDeep requested review from reggieriser, lynzt, Ryan-Koch and SirenaBorracha April 11, 2019 21:26

Not equal for tsp edit

1a27232

LeDeep added the A-Team label Apr 12, 2019

show earliest start date in sit office edit form

dbc1794

kimallen reviewed Apr 30, 2019

View reviewed changes

LeDeep added 3 commits April 30, 2019 23:55

Fix issue where authorized start date was not being assigned to sit

e33eb3b

Clean up redundant css

cc2412d

Remove unused className

2f71dd7

kimallen reviewed May 1, 2019

View reviewed changes

Fix to remove console error

0c9446e

Add data-cy for SIT tests

872b1c9

kimallen approved these changes May 1, 2019

View reviewed changes

LeDeep merged commit 4f13eca into master May 1, 2019

LeDeep deleted the sm-164251823-office-sit-edit-form branch May 1, 2019 19:25

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add edit icon and form for approved SIT requests #2004

Add edit icon and form for approved SIT requests #2004

LeDeep commented Apr 11, 2019

codecov bot commented Apr 11, 2019 •

edited

Loading

kimallen Apr 30, 2019

kimallen Apr 30, 2019

kimallen Apr 30, 2019

kimallen Apr 30, 2019

kimallen Apr 30, 2019 •

edited

Loading

kimallen commented Apr 30, 2019

kimallen May 1, 2019

LeDeep May 1, 2019

kimallen May 1, 2019

kimallen commented May 1, 2019

kimallen commented May 1, 2019

LeDeep commented May 1, 2019

kimallen left a comment

Add edit icon and form for approved SIT requests #2004

Add edit icon and form for approved SIT requests #2004

Conversation

LeDeep commented Apr 11, 2019

Description

Reviewer Notes

Setup

References

Screenshots

codecov bot commented Apr 11, 2019 • edited Loading

Codecov Report

kimallen Apr 30, 2019

Choose a reason for hiding this comment

kimallen Apr 30, 2019

Choose a reason for hiding this comment

kimallen Apr 30, 2019

Choose a reason for hiding this comment

kimallen Apr 30, 2019

Choose a reason for hiding this comment

kimallen Apr 30, 2019 • edited Loading

Choose a reason for hiding this comment

kimallen commented Apr 30, 2019

kimallen May 1, 2019

Choose a reason for hiding this comment

LeDeep May 1, 2019

Choose a reason for hiding this comment

kimallen May 1, 2019

Choose a reason for hiding this comment

kimallen commented May 1, 2019

kimallen commented May 1, 2019

LeDeep commented May 1, 2019

kimallen left a comment

Choose a reason for hiding this comment

codecov bot commented Apr 11, 2019 •

edited

Loading

kimallen Apr 30, 2019 •

edited

Loading