UITEN-233: Improve "Number of locations" on Campuses page #349

UladzislauKutarkin · 2023-06-21T06:11:32Z

Remember value in Institutions and campuses dropdown (Settings > Tenant > Libraries) when navigating away from page and

Purpose

UITEN-233

Approach

TODOS and Open Questions

Learning

Pre-Merge Checklist

Before merging this PR, please go through the following list and take appropriate actions.

I've added appropriate record to the CHANGELOG.md
Does this PR meet or exceed the expected quality standards?
- Code coverage on new code is 80% or greater
- Duplications on new code is 3% or less
- There are no major code smells or security issues
Does this introduce breaking changes?
- If any API-related changes - okapi interfaces and permissions are reviewed/changed correspondingly
- There are no breaking changes in this PR.

If there are breaking changes, please STOP and consider the following:

What other modules will these changes impact?
Do JIRAs exist to update the impacted modules?
- If not, please create them
- Do they contain the appropriate level of detail? Which endpoints/schemas changed, etc.
- Do they have all they appropriate links to blocked/related issues?
Are the JIRAs under active development?
- If not, contact the project's PO and make sure they're aware of the urgency.
Do PRs exist for these changes?
- If so, have they been approved?

Ideally all of the PRs involved in breaking changes would be merged in the same day to avoid breaking the folio-testing environment. Communication is paramount if that is to be achieved, especially as the number of intermodule and inter-team dependencies increase.

While it's helpful for reviewers to help identify potential problems, ensuring that it's safe to merge is ultimately the responsibility of the PR assignee.

…ings > Tenant > Libraries) when navigating away from page and returning

github-actions · 2023-06-21T06:12:46Z

Jest Unit Test Statistics

  1 files ±  0 24 suites +24 1m 14s ⏱️ + 1m 14s
77 tests +77 73 ✔️ +73 4 💤 +4 0 ❌ ±0
79 runs +79 75 ✔️ +75 4 💤 +4 0 ❌ ±0

Results for commit 6c65ca4. ± Comparison against base commit faac09d.

♻️ This comment has been updated with latest results.

github-actions · 2023-06-21T06:13:02Z

BigTest Unit Test Statistics

0 tests ±0 0 ✔️ ±0 0s ⏱️ ±0s
0 suites ±0 0 💤 ±0
0 files ±0 0 ❌ ±0

Results for commit 6c65ca4. ± Comparison against base commit faac09d.

♻️ This comment has been updated with latest results.

…ings > Tenant > Libraries) when navigating away from page and returning

BogdanDenis · 2023-06-22T08:37:10Z

package.json

@@ -181,6 +184,8 @@
    "@babel/eslint-parser": "^7.19.1",
    "@babel/plugin-proposal-class-properties": "^7.12.1",
    "@babel/plugin-proposal-decorators": "^7.0.0",
+    "@babel/plugin-proposal-private-methods": "^7.18.6",


Some -proposal- plugins were renamed in stripes-webpack to -transform- because they became standard and will were renamed by babel.
folio-org/stripes-webpack#114
Please rename the same plugins here and in babel.config.js

@BogdanDenis done. Thanks for information

vashjs · 2023-06-22T08:59:28Z

src/settings/LocationLibraries.js

@@ -107,10 +110,14 @@ class LocationLibraries extends React.Component {

  onChangeInstitution = (e) => {
    this.setState({ institutionId: e.target.value, campusId: null });
+
+    sessionStorage.setItem('institutionIdLibraries', e.target.value);
  }

  onChangeCampus = (e) => {
    this.setState({ campusId: e.target.value });


e.target.value can be moved to variable

vashjs · 2023-06-22T08:59:41Z

src/settings/LocationLibraries.js

@@ -107,10 +110,14 @@ class LocationLibraries extends React.Component {

  onChangeInstitution = (e) => {
    this.setState({ institutionId: e.target.value, campusId: null });


e.target.value can be moved to variable

Terala-Priyanka

As per the ticket description, The # of libraries in the column should be a link to a pre-filtered view of Settings > Tenant > Locations > Libraries showing only those libraries in the campus.

I tried to pull your branch and bring up local host serving with snapshot. I do not see column with links.
Am I doing something differently? Could you share a screencast?

UladzislauKutarkin · 2023-06-22T11:35:53Z

As per the ticket description, The # of libraries in the column should be a link to a pre-filtered view of Settings > Tenant > Locations > Libraries showing only those libraries in the campus.

I tried to pull your branch and bring up local host serving with snapshot. I do not see column with links. Am I doing something differently? Could you share a screencast?

My bad, I linked the wrong ticket. Now it's clear. Please take a look again

Terala-Priyanka · 2023-06-22T12:01:32Z

As per the ticket description, The # of libraries in the column should be a link to a pre-filtered view of Settings > Tenant > Locations > Libraries showing only those libraries in the campus.
I tried to pull your branch and bring up local host serving with snapshot. I do not see column with links. Am I doing something differently? Could you share a screencast?

My bad, I linked the wrong ticket. Now it's clear. Please take a look again

Thank you for updating the details.
I see different details in User Story and in scenario. However, your PR changes address

As per the ticket description, The # of libraries in the column should be a link to a pre-filtered view of Settings > Tenant > Locations > Libraries showing only those libraries in the campus.
I tried to pull your branch and bring up local host serving with snapshot. I do not see column with links. Am I doing something differently? Could you share a screencast?

My bad, I linked the wrong ticket. Now it's clear. Please take a look again

Thank you for updating the details.
As seen from the ticket, UserStory talks about Settings > Tenant > Locations and scenario talks about Settings > Tenant > Libraries.
Did you get a chance to cross check with your PO about this?
Your changes LGTM against Settings > Tenant > Libraries

UladzislauKutarkin · 2023-06-22T12:20:40Z

Yes, we need to remember value in Libraries.

sonarcloud · 2023-06-23T07:03:03Z

Kudos, SonarCloud Quality Gate passed!

0 Bugs
0 Vulnerabilities
0 Security Hotspots
0 Code Smells

66.7% Coverage
0.0% Duplication

zburke

This PR re-introduced some node config that was deliberately expunged in the past. Please remove it again :)

zburke · 2023-06-26T19:54:40Z

.github/workflows/build-npm-release.yml

-      NODEJS_VERSION: '14'
+      NODEJS_VERSION: '16'


These kinds of change should be handled in a separate PR. If there was a bug in the LocationLibraries.js code and we had to revert it, we'd lose this change too.

zburke · 2023-06-26T20:01:22Z

package.json

+  "engines": {
+    "node": ">=14.0.0"
+  },


engines describes the runtime requirements of this module, but this is a web application, not a NodeJS application. This clause was deliberately removed. Please remove it again.

UITEN-233: Remember value in Institutions and campuses dropdown (Sett…

164fe9c

…ings > Tenant > Libraries) when navigating away from page and returning

UladzislauKutarkin added 6 commits June 21, 2023 10:55

UITEN-233: Remember value in Institutions and campuses dropdown (Sett…

813c206

…ings > Tenant > Libraries) when navigating away from page and returning

UITEN-233: Remember value in Institutions and campuses dropdown (Sett…

b4fff9c

…ings > Tenant > Libraries) when navigating away from page and returning

UITEN-233: update node version github actions

e5f3980

UITEN-233: add @babel/plugin-proposal-private-property-in-object

dbf443e

UITEN-233: add @babel/plugin-proposal-private-methods

126dda9

UITEN-233: add @babel/plugin-proposal-private-methods

3fd4b2f

UladzislauKutarkin requested review from vashjs and a team June 21, 2023 19:24

BogdanDenis requested changes Jun 22, 2023

View reviewed changes

vashjs approved these changes Jun 22, 2023

View reviewed changes

Terala-Priyanka reviewed Jun 22, 2023

View reviewed changes

UladzislauKutarkin added 2 commits June 23, 2023 10:57

UITEN-233: fix comments

e52a9db

UITEN-233: fix comments

6c65ca4

BogdanDenis approved these changes Jun 23, 2023

View reviewed changes

UladzislauKutarkin merged commit 4d0394a into master Jun 23, 2023
6 checks passed

UladzislauKutarkin deleted the UITEN-233 branch June 23, 2023 08:03

zburke reviewed Jun 26, 2023

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

UITEN-233: Improve "Number of locations" on Campuses page #349

UITEN-233: Improve "Number of locations" on Campuses page #349

UladzislauKutarkin commented Jun 21, 2023 •

edited

github-actions bot commented Jun 21, 2023 •

edited

github-actions bot commented Jun 21, 2023 •

edited

BogdanDenis Jun 22, 2023

UladzislauKutarkin Jun 23, 2023

vashjs Jun 22, 2023

UladzislauKutarkin Jun 23, 2023

vashjs Jun 22, 2023

UladzislauKutarkin Jun 23, 2023

Terala-Priyanka left a comment

UladzislauKutarkin commented Jun 22, 2023

Terala-Priyanka commented Jun 22, 2023

UladzislauKutarkin commented Jun 22, 2023

sonarcloud bot commented Jun 23, 2023

zburke left a comment

zburke Jun 26, 2023

zburke Jun 26, 2023

		@@ -107,10 +110,14 @@ class LocationLibraries extends React.Component {

		onChangeInstitution = (e) => {
		this.setState({ institutionId: e.target.value, campusId: null });

UITEN-233: Improve "Number of locations" on Campuses page #349

UITEN-233: Improve "Number of locations" on Campuses page #349

Conversation

UladzislauKutarkin commented Jun 21, 2023 • edited

Purpose

Approach

TODOS and Open Questions

Learning

Pre-Merge Checklist

github-actions bot commented Jun 21, 2023 • edited

Jest Unit Test Statistics

github-actions bot commented Jun 21, 2023 • edited

BigTest Unit Test Statistics

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

Terala-Priyanka left a comment

Choose a reason for hiding this comment

UladzislauKutarkin commented Jun 22, 2023

Terala-Priyanka commented Jun 22, 2023

UladzislauKutarkin commented Jun 22, 2023

sonarcloud bot commented Jun 23, 2023

zburke left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

UladzislauKutarkin commented Jun 21, 2023 •

edited

github-actions bot commented Jun 21, 2023 •

edited

github-actions bot commented Jun 21, 2023 •

edited