refactor(1576): Upgrade react-router packages and refactor related components #1506

dependabot · 2021-12-20T06:21:52Z

Summary of Changes

This dependabot PR was co-opted for react-router upgrades and refactoring.

Pull request closes #1576

Acceptance criteria as stated in the issue

How to Test

List the steps to test the PR
These steps are generic, please adjust as necessary.

cd tdrs-frontend && docker-compose -f docker-compose.yml -f docker-compose.local.yml up -d
cd tdrs-backend && docker-compose -f docker-compose.yml -f docker-compose.local.yml up -d

Open http://localhost:3000/ and sign in.
All functionality should remain unchanged.

Deliverables

More details on how deliverables herein are assessed included here.

Deliverable 1: Accepted Features

Checklist of ACs:

react-router and react-router-dom are ugraded to latest major version
package.json is updated
package-lock.json is updated
All existing behavior and application functionality should remain identical
Testing Checklist has been run and all tests pass
README is updated, if necessary
lfrohlich and/or adpennington confirmed that ACs are met.

Deliverable 2: Tested Code

Are all areas of code introduced in this PR meaningfully tested?
- If this PR introduces backend code changes, are they meaningfully tested?
- If this PR introduces frontend code changes, are they meaningfully tested?
Are code coverage minimums met?
- Frontend coverage: [97.31%] (see CodeCov Report comment in PR)
- Backend coverage: [insert coverage %] (see CodeCov Report comment in PR)

Deliverable 3: Properly Styled Code

Are backend code style checks passing on CircleCI?
Are frontend code style checks passing on CircleCI?
Are code maintainability principles being followed?

Deliverable 4: Accessible

Does this PR complete the epic?
Are links included to any other gov-approved PRs associated with epic?
Does PR include documentation for Raft's a11y review?
Did automated and manual testing with iamjolly and ttran-hub using Accessibility Insights reveal any errors introduced in this PR?

n/a: no user facing changes

Deliverable 5: Deployed

Was the code successfully deployed via automated CircleCI process to development on Cloud.gov?

Deliverable 6: Documented

Does this PR provide background for why coding decisions were made?
If this PR introduces backend code, is that code easy to understand and sufficiently documented, both inline and overall?
If this PR introduces frontend code, is that code easy to understand and sufficiently documented, both inline and overall?
If this PR introduces dependencies, are their licenses documented?
Can reviewer explain and take ownership of these elements presented in this code review?

Deliverable 7: Secure

Does the OWASP Scan pass on CircleCI?
Do manual code review and manual testing detect any new security issues?
If new issues detected, is investigation and/or remediation plan documented?

Deliverable 8: User Research

Research product(s) clearly articulate(s):

the purpose of the research
methods used to conduct the research
who participated in the research
what was tested and how
impact of research on TDP
(if applicable) final design mockups produced for TDP development

jorgegonzalez · 2022-01-25T17:03:47Z

@dependabot rebase

Bumps [react-router-dom](https://github.com/remix-run/react-router/tree/HEAD/packages/react-router-dom) from 5.3.0 to 6.2.1. - [Release notes](https://github.com/remix-run/react-router/releases) - [Commits](https://github.com/remix-run/react-router/commits/v6.2.1/packages/react-router-dom) --- updated-dependencies: - dependency-name: react-router-dom dependency-type: direct:production update-type: version-update:semver-major ... Signed-off-by: dependabot[bot] <support@github.com>

jorgegonzalez · 2022-01-26T14:03:35Z

react-router and react-router-dom need to be upgraded in parallel, and doing so has required some minor refactoring; I'm making a separate issue to track this work.

jorgegonzalez · 2022-01-26T14:06:30Z

Additionally, a package we use connected-react-router does not support these versions so we are migrating to an alternative with a similar api.

supasate/connected-react-router#543

https://github.com/lagunovsky/redux-react-router

…into dependabot/npm_and_yarn/tdrs-frontend/raft-tdp-main/react-router-dom-6.2.1

codecov · 2022-02-02T19:25:54Z

Codecov Report

Merging #1506 (197110a) into raft-tdp-main (c660b81) will decrease coverage by 0.26%.
The diff coverage is 80.00%.

@@                Coverage Diff                @@
##           raft-tdp-main    #1506      +/-   ##
=================================================
- Coverage          97.58%   97.31%   -0.27%     
=================================================
  Files                 80       80              
  Lines               1902     1902              
  Branches             249      249              
=================================================
- Hits                1856     1851       -5     
- Misses                22       24       +2     
- Partials              24       27       +3

Flag	Coverage Δ
dev-backend	`99.10% <ø> (ø)`
dev-frontend	`94.01% <80.00%> (-0.75%)`	⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files	Coverage Δ
...end/src/components/LoginCallback/LoginCallback.jsx	`50.00% <0.00%> (-50.00%)`	⬇️
...rontend/src/components/EditProfile/EditProfile.jsx	`100.00% <100.00%> (ø)`
tdrs-frontend/src/components/Header/Header.jsx	`100.00% <100.00%> (ø)`
...ontend/src/components/PrivateRoute/PrivateRoute.js	`100.00% <100.00%> (ø)`
tdrs-frontend/src/components/Routes/Routes.js	`100.00% <100.00%> (ø)`
...-frontend/src/components/SplashPage/SplashPage.jsx	`100.00% <100.00%> (ø)`

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update c660b81...197110a. Read the comment docs.

andrew-jameson · 2022-02-02T19:32:58Z

tdrs-frontend/src/components/Header/Header.jsx

-    /* istanbul ignore if */
-    if (menuRef.current.classList.contains('is-visible')) {
-      e.preventDefault()
-      console.log('is visible')


Is logging now handled elsewhere? Or was this unnecessary before?

Logging to the console like this is generally unnecessary

andrew-jameson

Found no issues locally, LGTM

jorgegonzalez · 2022-02-02T20:21:29Z

tdrs-frontend/src/components/EditProfile/EditProfile.jsx

@@ -144,7 +144,7 @@ function EditProfile() {
  }

  if (requestedAccess && sttAssigned) {
-    return <Redirect to="/request" />
+    return <Navigate to="/request" />


All instances of Redirect are converted to Navigate

jorgegonzalez · 2022-02-02T20:22:06Z

tdrs-frontend/src/components/EditProfile/EditProfile.test.js

@@ -450,15 +452,39 @@ describe('EditProfile', () => {
        ],
      },
    })
-    const wrapper = mount(
+
+    render(


Note that we are rewriting enzyme tests with react-testing-library everywhere possible when we encounter them

jorgegonzalez · 2022-02-02T20:22:28Z

tdrs-frontend/src/components/EditProfile/EditProfile.test.js

+                </PrivateRoute>
+              }
+            />
+            <Route


Route can only exist as a child of Routes now

jorgegonzalez · 2022-02-02T20:23:22Z

tdrs-frontend/src/components/Header/Header.jsx

+  const keyListenersMap = useMemo(() => {
+    let tabIndex = 0
+    /* istanbul ignore next  */
+    const handleTabKey = (e) => {
+      /* istanbul ignore if */
+      if (menuRef.current.classList.contains('is-visible')) {
+        e.preventDefault()
+        const focusableMenuElements = [
+          ...menuRef.current.querySelectorAll('button'),
+          ...menuRef.current.querySelectorAll('a'),
+        ]

-      const firstElement = focusableMenuElements[0]
-      const lastIndex = focusableMenuElements.length - 1
-      const lastElement = focusableMenuElements[lastIndex]
+        const lastIndex = focusableMenuElements.length - 1

-      if (focusableMenuElements.includes(document.activeElement)) {
-        if (!e.shiftKey && tabIndex >= lastIndex) {
-          tabIndex = 0
-        } else if (e.shiftKey && tabIndex === 0) {
-          tabIndex = lastIndex
-        } else if (e.shiftKey) {
-          tabIndex -= 1
+        if (focusableMenuElements.includes(document.activeElement)) {
+          if (!e.shiftKey && tabIndex >= lastIndex) {
+            tabIndex = 0
+          } else if (e.shiftKey && tabIndex === 0) {
+            tabIndex = lastIndex
+          } else if (e.shiftKey) {
+            tabIndex -= 1
+          } else {
+            tabIndex += 1
+          }
        } else {
-          tabIndex += 1
+          tabIndex = 0
        }
-      } else {
-        tabIndex = 0
+
+        focusableMenuElements[tabIndex].focus()
      }

-      focusableMenuElements[tabIndex].focus()
+      return false
    }

-    return false
-  }
-
-  const keyListenersMap = new Map([[9, handleTabKey]])
+    return new Map([[9, handleTabKey]])
+  }, [menuRef])

  /* istanbul ignore next  */
  useEffect(() => {
    function keyListener(e) {
      const listener = keyListenersMap.get(e.keyCode)
      return listener && listener(e)
    }
+
    document.addEventListener('keydown', keyListener)

    return () => document.removeEventListener('keydown', keyListener)
-  }, [])
+  }, [keyListenersMap])


All of this is just refactored for clarity and to remove unneeded logic and linter warnings

jorgegonzalez · 2022-02-02T20:23:43Z

tdrs-frontend/src/components/LoginCallback/LoginCallback.test.js

@@ -15,39 +18,62 @@ describe('LoginCallback.js', () => {
    const store = mockStore({
      auth: { authenticated: true, user: { email: 'hi@bye.com' } },
    })
-    const wrapper = mount(
+
+    render(


Again, migrate from enzyme's mount to render

jorgegonzalez · 2022-02-02T20:24:16Z

tdrs-frontend/src/components/PrivateRoute/PrivateRoute.js

  ) : null
 }

-PrivateRoute.propTypes = {


We need to move away from PropTypes as this is unhelpful noise.

jorgegonzalez · 2022-02-02T20:24:55Z

tdrs-frontend/src/components/PrivateRoute/PrivateRoute.js

-        <IdleTimer />
-      </PrivateTemplate>
-    </Route>
+    <PrivateTemplate title={title}>


Note we are no longer wrapping this with Route, and it is handled higher up in the tree

jorgegonzalez · 2022-02-02T20:35:11Z

tdrs-frontend/src/components/Routes/Routes.js

-        <NoMatch />
-      </Route>
-    </Switch>
+    <Routes>


Switch is now Routes

jorgegonzalez · 2022-02-02T20:36:12Z

tdrs-frontend/src/index.js

@@ -2,7 +2,7 @@ import React from 'react'
 import ReactDOM from 'react-dom'
 import axios from 'axios'

-import { ConnectedRouter as Router } from 'connected-react-router'
+import { ReduxRouter as Router } from '@lagunovsky/redux-react-router'


The replacement for connected-react-router has a very similar api and only requires a few additions to our root Router and Reducer below

…into dependabot/npm_and_yarn/tdrs-frontend/raft-tdp-main/react-router-dom-6.2.1

jorgegonzalez · 2022-02-04T15:09:52Z

tdrs-frontend/package.json

    "file-type": "^16.5.3",
-    "history": "^4.7.2",
+    "history": "^5.2.0",


This was updated as the older version caused a conflict with the new one needed in react-router-dom

ADPennington

lgtm @jorgegonzalez 👍🏾

…nd/raft-tdp-main/react-router-dom-6.2.1

dependabot bot added dependencies Pull requests that update a dependency file frontend raft review This issue is ready for raft review labels Dec 20, 2021

dependabot bot requested a review from a team December 20, 2021 06:21

dependabot bot mentioned this pull request Dec 20, 2021

Bump react-router-dom from 5.3.0 to 6.1.1 in /tdrs-frontend #1491

Closed

dependabot bot force-pushed the dependabot/npm_and_yarn/tdrs-frontend/raft-tdp-main/react-router-dom-6.2.1 branch 2 times, most recently from c034753 to 9e91149 Compare December 22, 2021 17:08

dependabot bot force-pushed the dependabot/npm_and_yarn/tdrs-frontend/raft-tdp-main/react-router-dom-6.2.1 branch from 9e91149 to db1e594 Compare January 18, 2022 17:16

jorgegonzalez mentioned this pull request Jan 21, 2022

build(854): switch frontend package manager from yarn to npm #1538

Merged

27 tasks

dependabot bot force-pushed the dependabot/npm_and_yarn/tdrs-frontend/raft-tdp-main/react-router-dom-6.2.1 branch 4 times, most recently from 946352e to f7ff522 Compare January 25, 2022 15:11

dependabot bot force-pushed the dependabot/npm_and_yarn/tdrs-frontend/raft-tdp-main/react-router-dom-6.2.1 branch from f7ff522 to fee4a9b Compare January 25, 2022 17:04

jorgegonzalez added 3 commits January 25, 2022 12:09

fix: install latest react-router which should have been done in parallel

1cf5167

fix: upgrade Redirect components to Navigate

2cd0f9a

fix: replace connected-react-router

9591ea8

jorgegonzalez force-pushed the dependabot/npm_and_yarn/tdrs-frontend/raft-tdp-main/react-router-dom-6.2.1 branch from 6413396 to 9591ea8 Compare January 25, 2022 18:38

jorgegonzalez mentioned this pull request Jan 26, 2022

Upgrade react-router packages and refactor related components #1576

Closed

11 tasks

jorgegonzalez linked an issue Jan 26, 2022 that may be closed by this pull request

Upgrade react-router packages and refactor related components #1576

Closed

11 tasks

jorgegonzalez changed the title ~~Bump react-router-dom from 5.3.0 to 6.2.1 in /tdrs-frontend~~ refactor(1576): Upgrade react-router packages and refactor related components Jan 26, 2022

refactor: Header.jsx

2dd8a79

andrew-jameson assigned jorgegonzalez Feb 2, 2022

jorgegonzalez added 2 commits February 2, 2022 12:28

Merge branch 'raft-tdp-main' of https://github.com/raft-tech/TANF-app …

5f53cf3

…into dependabot/npm_and_yarn/tdrs-frontend/raft-tdp-main/react-router-dom-6.2.1

test: fix privateroute test

19ead36

jorgegonzalez force-pushed the dependabot/npm_and_yarn/tdrs-frontend/raft-tdp-main/react-router-dom-6.2.1 branch from 0a947d7 to 19ead36 Compare February 2, 2022 18:07

jorgegonzalez added 4 commits February 2, 2022 13:14

test: fix SplashPage.test.js

347aaf7

test: fix LoginCallback.test.js

5c54937

test: fix EditProfile.test.js

23520fc

Merge branch 'raft-tdp-main' of https://github.com/raft-tech/TANF-app …

1790b40

…into dependabot/npm_and_yarn/tdrs-frontend/raft-tdp-main/react-router-dom-6.2.1

jorgegonzalez requested review from andrew-jameson and riatzukiza February 2, 2022 19:20

andrew-jameson reviewed Feb 2, 2022

View reviewed changes

andrew-jameson approved these changes Feb 2, 2022

View reviewed changes

jorgegonzalez reviewed Feb 2, 2022

View reviewed changes

tdrs-frontend/src/components/Routes/Routes.js

<NoMatch />

</Route>

</Switch>

<Routes>

Copy link

jorgegonzalez Feb 2, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Switch is now Routes

jorgegonzalez reviewed Feb 2, 2022

View reviewed changes

jorgegonzalez requested a review from ADPennington February 2, 2022 20:43

Merge branch 'raft-tdp-main' of https://github.com/raft-tech/TANF-app …

7227325

…into dependabot/npm_and_yarn/tdrs-frontend/raft-tdp-main/react-router-dom-6.2.1

jorgegonzalez added QASP Review and removed raft review This issue is ready for raft review labels Feb 3, 2022

jorgegonzalez reviewed Feb 4, 2022

View reviewed changes

ADPennington approved these changes Feb 4, 2022

View reviewed changes

ADPennington added Ready to Merge and removed QASP Review labels Feb 4, 2022

Merge branch 'raft-tdp-main' into dependabot/npm_and_yarn/tdrs-fronte…

197110a

…nd/raft-tdp-main/react-router-dom-6.2.1

andrew-jameson merged commit 4eef5d2 into raft-tdp-main Feb 4, 2022

andrew-jameson deleted the dependabot/npm_and_yarn/tdrs-frontend/raft-tdp-main/react-router-dom-6.2.1 branch February 4, 2022 17:09

andrew-jameson mentioned this pull request Feb 8, 2022

Multi-Issue PR HHS/TANF-app#371

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

refactor(1576): Upgrade react-router packages and refactor related components #1506

refactor(1576): Upgrade react-router packages and refactor related components #1506

dependabot bot commented on behalf of github Dec 20, 2021 •

edited by ADPennington

jorgegonzalez commented Jan 25, 2022

jorgegonzalez commented Jan 26, 2022

jorgegonzalez commented Jan 26, 2022

codecov bot commented Feb 2, 2022 •

edited

andrew-jameson Feb 2, 2022 •

edited

jorgegonzalez Feb 2, 2022

andrew-jameson left a comment

jorgegonzalez Feb 2, 2022

jorgegonzalez Feb 2, 2022

jorgegonzalez Feb 2, 2022

jorgegonzalez Feb 2, 2022

jorgegonzalez Feb 2, 2022

jorgegonzalez Feb 2, 2022

jorgegonzalez Feb 2, 2022

jorgegonzalez Feb 2, 2022

jorgegonzalez Feb 2, 2022

jorgegonzalez Feb 4, 2022

ADPennington left a comment

refactor(1576): Upgrade react-router packages and refactor related components #1506

refactor(1576): Upgrade react-router packages and refactor related components #1506

Conversation

dependabot bot commented on behalf of github Dec 20, 2021 • edited by ADPennington

Summary of Changes

How to Test

Deliverables

jorgegonzalez commented Jan 25, 2022

jorgegonzalez commented Jan 26, 2022

jorgegonzalez commented Jan 26, 2022

codecov bot commented Feb 2, 2022 • edited

Codecov Report

andrew-jameson Feb 2, 2022 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

andrew-jameson left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ADPennington left a comment

Choose a reason for hiding this comment

dependabot bot commented on behalf of github Dec 20, 2021 •

edited by ADPennington

codecov bot commented Feb 2, 2022 •

edited

andrew-jameson Feb 2, 2022 •

edited