TGR-50: Remediation update #2155

EdwinGuzman · 2024-07-29T16:07:43Z

NOTE: Tests currently DO NOT run. This might be the tradeoff we deal with now or spend a week or so updating tests to jest/rtl for Hold and SHEP pages (since those are/will be left in the app until the refactor project is complete).

~~This is still a WIP because we are waiting on tech stacks but~~ please test locally. Make sure you have AWS_ACCESS_KEY_ID and AWS_SECRET_ACCESS_KEY set in .env for the Docker image to build correctly.

1.9.0

Remediation Project Update

Adds

Adds AWS_ACCESS_KEY_ID and AWS_SECRET_ACCESS_KEY as example env vars needed in .env-sample. They are required for building Docker images.
Adds Dockerfile and instructions on how to use it locally.
Adds the following npm packages:
sass, webpack-dev-middleware

Updates

Updates .nvmrc to use Node 20.
Updates the logger file.
Updates npm scripts to be more conventional with build and dev.
Updates how the webpack dev server is implemented.
Updates how the react-number-format package is used in the FieldsetDate component.
Updates implementation of the focus-trap-react package.
Updates node's Buffer implementation for KMS package usage.
Updates the following npm packages:
@babel/preset-env, @babel/preset-react, @babel/preset-register, @nypl/nypl-core-objects, @nypl/nypl-data-api-client, @typescript-eslint/eslint-plugin, axios, babel-plugin-module-resolver, body-parser, chai, clean-webpack-plugin, compression, cookie-parser, cross-env, css-loader, doctoc, dotenv, ejs, enzyme-adapter-react-16, eslint, eslint-config-airbnb, eslint-config-prettier, eslint-plugin-import, eslint-plugin-jsx-a11y, eslint-plugin-prettier, eslint-plugin-react, eslint-plugin-react-hooks, express, file-loader, focus-trap-react, jsonwebtoken, mini-css-extract-plugin, mocha, mock-local-storage, nock, react, react-autosuggest, react-dom, react-number-format, react-redux, redux, redux-thunk, prettier, sass-loader, sinon, sinon-chai, style-loader, supertest, typescript, webpack, webpack-cli, winston, underscore, url-parse, validator

Removals

Removes the .ebextensions folder since this repos should no longer deploy to ElasticBeanstalk.
Removes the .travis.yml file to not use Travis CI.
Removes the Procfile since this we don't use Heroku.
Removes the following npm packages:
@babel/core, @babel/polyfill, @nypl/dgx-header-component, @nypl/dgx-react-footer, @typescript-eslint/parser, node-sass, node-sass-glob-importer, react-router-scroll, webpack-bundle-analyzer, webpack-dev-server, webpack-merge

…ges, but the app doesn't run yet (react-router)

.github/workflows/ci.yml

danamansana

Changes look fine but I think there is some commented code that I've marked to remove (unless there is some reason to keep it).

danamansana · 2024-09-06T14:36:08Z

logger.js

 // Supress error handling
-winston.emitErrs = false;
+// winston.emitErrs = false;


commented code?

danamansana · 2024-09-06T14:36:37Z

logger.js

 // const logLevel = (process.env.NODE_ENV === 'production') ? 'info' : 'debug';
-const timestamp = () => new Date().toISOString();
-const formatter = (options) => {
+// const timestamp = () => new Date().toISOString();


commented code?

danamansana · 2024-09-06T14:37:29Z

src/app/components/ItemFilters/ItemFilter.jsx

@@ -1,7 +1,7 @@
 import React, { useState } from 'react';
 import PropTypes from 'prop-types';
 import { Button, Checkbox, Icon } from '@nypl/design-system-react-components';
-import FocusTrap from 'focus-trap-react';
+// import FocusTrap from 'focus-trap-react';


commented code?

danamansana · 2024-09-06T14:37:40Z

src/app/components/ItemFilters/ItemFilter.jsx

+    // <FocusTrap
+    //   focusTrapOptions={{
+    //     clickOutsideDeactivates: true,
+    //     onDeactivate: () => {
+    //       if (!mobile) manageFilterDisplay('none');
+    //     },
+    //     returnFocusOnDeactivate: false,
+    //   }}
+    //   active={isOpen}
+    //   className="item-filter"
+    // >


commented code?

danamansana · 2024-09-06T14:38:16Z

src/app/components/LoadingLayer/LoadingLayer.jsx

+    // <FocusTrap
+    //   focusTrapOptions={{
+    //     onDeactivate: () => {
+    //       if (focus) {
+    //         focus();
+    //       }
+    //     },
+    //   }}
+    // >


commented code?

danamansana · 2024-09-06T14:38:25Z

src/app/components/LoadingLayer/LoadingLayer.jsx

@@ -45,7 +45,7 @@ const LoadingLayer = ({ loading, title, focus }) => {
          </div>
        </div>
      </div>
-    </FocusTrap>
+    // {/* </FocusTrap> */}


commented code?

danamansana · 2024-09-06T14:38:34Z

src/client/App.jsx

@@ -8,7 +8,7 @@ import ReactDOM from 'react-dom';
 import { Router, browserHistory, applyRouterMiddleware } from 'react-router';
 import a11y from 'react-a11y';
 import { Provider } from 'react-redux';
-import useScroll from 'react-router-scroll/lib/useScroll';
+// import useScroll from 'react-router-scroll/lib/useScroll';


commented code?

EdwinGuzman · 2024-09-06T16:16:28Z

Good point, thanks, I'll remove the commented code.

charmingduchess · 2024-09-10T15:48:25Z

@EdwinGuzman are there specs for this work? the ticket seems to be more documentation of the work itself than a directive describing what needs to happen.

.github/workflows/ci.yml

charmingduchess · 2024-09-10T15:59:10Z

CHANGELOG.md

@@ -1,5 +1,36 @@
 ## CHANGE LOG

+### 1.9.0


Does this warrant a new major version?

I was debating that. I don't think so since the update is for the codebase and not any features. Do you think it should be a major bump?

charmingduchess · 2024-09-10T16:10:07Z

src/app/components/ItemFilters/ItemFilter.jsx

-      active={isOpen}
-      className="item-filter"
-    >
+    <div className="item-filter">


does this have any accessibility implications? ie is focus going to be affected?

Yes, but this page will soon be replaced so I'm not too concerned.

charmingduchess · 2024-09-10T16:13:48Z

webpack-dev.config.js

+const path = require('path');
+const webpack = require('webpack');
+
+// Sets appEnv so the the header component will point to the search app on either Dev or Prod


what is the search app?

Hmm just copy/paste issue.

charmingduchess · 2024-09-10T16:36:21Z

per our discussion, please update the readme to indicate the --force flag for npm i invocation

EdwinGuzman · 2024-09-19T20:21:44Z

This update is on the QA, Train, and Production live envs. Merging into development with the known issue of tests failing.

…es-hit-reverse-proxy SCC-4324 - Make bib link on hold pages hit reverse proxy

…-hostname

Add fix to fix use of discovery.nypl.org in redirect_uri

EdwinGuzman added 5 commits July 26, 2024 17:12

first commit of updating to node 20 and successfully installing packa…

ee246db

…ges, but the app doesn't run yet (react-router)

minor update to fix logger

4fdc166

working app

7d0dac0

Updating dockerfile, adding GA, and removing travis

85c850a

changelog update and removing EB from repo

6b96740

EdwinGuzman requested a review from danamansana August 12, 2024 20:59

EdwinGuzman changed the title ~~Tgr 50/remediation update~~ TGR-50: Remediation update Aug 12, 2024

EdwinGuzman marked this pull request as ready for review August 12, 2024 21:00

EdwinGuzman commented Aug 13, 2024

View reviewed changes

.github/workflows/ci.yml Outdated Show resolved Hide resolved

EdwinGuzman added 9 commits August 13, 2024 14:47

Updating ref to ECR for docker image

4d89487

forgot force for npm i

14122ed

Ignore tests for now since they're broken

71d9f13

no local .env copy

8e1b621

hmm deps?

a8c028a

all in deps but will organize later

f69fd31

new qa ecs names

f2faa89

fixing visually hidden styling issue onhold request page

461624b

Updating ecs cluster and service names

748dcdd

danamansana approved these changes Sep 6, 2024

View reviewed changes

Updating based on feedback

87d4d1d

EdwinGuzman requested review from nonword, dgcohen and charmingduchess September 9, 2024 15:03

EdwinGuzman added 3 commits September 9, 2024 12:03

Some env var defaults

59d8889

separating train into it's own ci

d139073

typo in service name for discovery-ui-edd

6bba52d

charmingduchess reviewed Sep 10, 2024

View reviewed changes

.github/workflows/ci.yml Show resolved Hide resolved

charmingduchess reviewed Sep 10, 2024

View reviewed changes

EdwinGuzman added 11 commits September 10, 2024 14:00

Updates based on feedback

19eb44e

readme git branches update

b65b21a

Testing deploy branch name

83e90d9

fixing prod name, pushing to prod

2d3aa17

typo

8da7d20

Hard coding rp enabled links for now

c9a5060

forgot to update qa

07275b6

Updating SHEP links

a9f85d9

Updating SHEP links

9f79eea

fixing dslink issue

3af2d60

cleaning up npm scripts and updating branch targets for GHA deployment

b937ee8

EdwinGuzman and others added 7 commits September 20, 2024 10:05

Debugging and setting more env vars

b2c9395

Not reading from redux store was the issue

b8fc88e

simple log

3e1d27c

Replace react link with dslink on hold request page

f7ec4f3

Remove link import

9cb6286

Fix failing test

a0c4bc5

Merge pull request #2163 from NYPL/SCC-4324/make-bib-link-on-hold-pag…

88eb06c

…es-hit-reverse-proxy SCC-4324 - Make bib link on hold pages hit reverse proxy

dgcohen approved these changes Oct 30, 2024

View reviewed changes

nonword added 4 commits November 6, 2024 11:40

Add fix to fix use of discovery.nypl.org in redirect_uri

72fc70b

Merge branch 'TGR-50/remediation-update' into NOREF-disallow-internal…

bface0c

…-hostname

Change CD to deploy production from namesake branch

c967489

Merge pull request #2165 from NYPL/NOREF-disallow-internal-hostname

b29a083

Add fix to fix use of discovery.nypl.org in redirect_uri

EdwinGuzman merged commit d30b284 into development Nov 7, 2024
3 of 4 checks passed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

TGR-50: Remediation update #2155

TGR-50: Remediation update #2155

EdwinGuzman commented Jul 29, 2024 •

edited

Loading

danamansana left a comment

danamansana Sep 6, 2024

danamansana Sep 6, 2024

danamansana Sep 6, 2024

danamansana Sep 6, 2024

danamansana Sep 6, 2024

danamansana Sep 6, 2024

danamansana Sep 6, 2024

EdwinGuzman commented Sep 6, 2024

charmingduchess commented Sep 10, 2024

charmingduchess Sep 10, 2024

EdwinGuzman Sep 10, 2024

charmingduchess Sep 10, 2024

EdwinGuzman Sep 10, 2024

charmingduchess Sep 10, 2024

EdwinGuzman Sep 10, 2024

charmingduchess commented Sep 10, 2024

EdwinGuzman commented Sep 19, 2024

TGR-50: Remediation update #2155

TGR-50: Remediation update #2155

Conversation

EdwinGuzman commented Jul 29, 2024 • edited Loading

1.9.0

Adds

Updates

Removals

danamansana 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

EdwinGuzman commented Sep 6, 2024

charmingduchess commented Sep 10, 2024

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

charmingduchess commented Sep 10, 2024

EdwinGuzman commented Sep 19, 2024

EdwinGuzman commented Jul 29, 2024 •

edited

Loading