UITEN-276 - Reading Room Access settings page basic layout implementation #389

Terala-Priyanka · 2024-03-25T10:04:41Z

Purpose

https://folio-org.atlassian.net/browse/UITEN-277
https://folio-org.atlassian.net/browse/UITEN-276

Approach

Introduced new page(Reading Room Access) in tenant settings.
Currently this page is read only with mock data.
Gradually, once the APIs development progresses and UI permission sets are created, this settings page will include add, edit and delete actions.

Note: Please note that the target branch is not master. Due to several reasons, our teams intends to merge this PR into the above mentioned base branch. Once all the dependencies are cleared, we will in turn merge this base branch into master via another PR. Any pieces of code intentionally commented out(like stripes.hasPerm) will be addressed in upcoming PRs.

Screenshots

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.

…tion

github-actions · 2024-03-25T10:06:41Z

Jest Unit Test Statistics

  1 files ±0 27 suites +1 2m 32s ⏱️ +8s
95 tests +1 83 ✔️ +1 12 💤 ±0 0 ❌ ±0
97 runs +1 85 ✔️ +1 12 💤 ±0 0 ❌ ±0

Results for commit 861b7d4. ± Comparison against base commit b27d300.

♻️ This comment has been updated with latest results.

github-actions · 2024-03-25T10:07:01Z

BigTest Unit Test Statistics

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

Results for commit 861b7d4. ± Comparison against base commit b27d300.

♻️ This comment has been updated with latest results.

BogdanDenis · 2024-03-26T13:26:28Z

src/settings/ReadingRoomAccess/ReadingRoomAccess.test.js

+import buildStripes from '../../../test/jest/__new_mocks__/stripesCore.mock';
+
+import {
+  renderWithRouter, renderWithReduxForm


Please put each imported item on a new line

BogdanDenis · 2024-03-26T13:27:21Z

src/settings/ReadingRoomAccess/ReadingRoomAccess.test.js

+      expect(screen.getByLabelText('ui-tenant-settings.settings.reading-room-access.label')).toBeInTheDocument();
+    });
+
+    // it('should render text "There are no Reading room access"', () => {


Please remove unneeded commented code, or add a TODO item and mention a ticket number in which code will be uncommented

This will be uncommented in later PRs.
I will add a TODO here. Thank you for the suggestion!

sonarqubecloud · 2024-03-27T05:12:42Z

Quality Gate passed

Issues
1 New issue
0 Accepted issues

Measures
0 Security Hotspots
94.4% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarCloud

zburke

~~Was this really intended for review? The work feels like a POC given the hard-coded sample data. If you need feedback about an implementation, that's fine, but please mark such PRs as "Draft".~~

My bad; I see this was intended for a feature branch, not #master. Sorry for the stern tone.

zburke · 2024-03-27T11:33:29Z

src/settings/ReadingRoomAccess/ReadingRoomAccess.js

+  values: { records: [
+    {
+      'id': 1,
+      'name': 'RR1',
+      'public': true,
+      'servicePoint': [
+        {
+          name: 'Circ Desk 1',
+          id: '3a40852d-49fd-4df2-a1f9-6e2641a6e91f',
+        },
+        {
+          id: 'c4c90014-c8c9-4ade-8f24-b5e313319f4b',
+          name: 'Circ Desk 2'
+        },
+      ],
+      // metadata: {
+      //   'createdDate': '2024-03-21T10:59:25.085+00:00',
+      //   'createdByUserId': 'af5ad81e-6857-5b65-9c0c-60942e56f872',
+      //   'updatedDate': '2024-03-21T10:59:25.085+00:00',
+      //   'updatedByUserId': 'af5ad81e-6857-5b65-9c0c-60942e56f872'
+      // }
+    },
+    {
+      'id': 2,
+      'name': 'RR2',
+      'public': true,
+      'servicePoint': [{
+        name: 'Circ Desk 1',
+        id: '3a40852d-49fd-4df2-a1f9-6e2641a6e91f',
+      }],
+      // metadata: {
+      //   'createdDate': '2024-03-21T10:59:25.085+00:00',
+      //   'createdByUserId': 'af5ad81e-6857-5b65-9c0c-60942e56f872',
+      //   'updatedDate': '2024-03-21T10:59:25.085+00:00',
+      //   'updatedByUserId': 'af5ad81e-6857-5b65-9c0c-60942e56f872'
+      // }
+    },
+  ] },


It is exceptionally exceptionally odd to me that this kind of data would be hard-coded and universal to every single tenant. The fact that metadata is included but commented out suggests this was copied from an API response; was this test data that should have been removed once the API was configured?

zburke · 2024-03-27T11:35:55Z

src/settings/ReadingRoomAccess/ReadingRoomAccess.js

+    servicePoint: intl.formatMessage({ id:'ui-tenant-settings.settings.reading-room-access.asp' }),
+  };
+
+  const editable = false; // stripes.hasPerm('ui-users.settings.reading-room-access.all');


Why is this hard-coded? Why does the comment refer to permissions specified by ui-users rather than ui-tenant-settings?

zburke · 2024-03-27T11:43:07Z

src/settings/ReadingRoomAccess/ReadingRoomAccess.js

+  RRAServicePoints: {
+    type: 'okapi',
+    resource: 'service-points',
+    path: 'service-points?limit=200',


200:

Do not use magic numbers; use a usefully named constant. This is CS 101.

200 is almost certainly inadequate

import queryLimit() from @folio/stripes/core, which (eventually) will be implemented to return either stripes-config.maxUnpagedResourceCount (from stripes.config.js and thus configurable per-tenant) or the default.

This manifest is configured but never used. Why? If you don't need the data, don't make the API request :/

Terala-Priyanka · 2024-03-28T05:23:36Z

~~Was this really intended for review? The work feels like a POC given the hard-coded sample data. If you need feedback about an implementation, that's fine, but please mark such PRs as "Draft".~~

My bad; I see this was intended for a feature branch, not #master. Sorry for the stern tone.

No worries. Just to add to what is there in the description, in upcoming PRS(when APIS are ready to consume by UI) everything will be clean.

* UITEN-276 - Reading Room Access settings page basic layout implementation (#389) * UITEN-276 - Reading Room Access settings page basic layout implementation * UITEN-276 - fix review comments * UITEN-278 - create reading room from tenant settings (#393) * UITEN-278 - create reading room from tenant settings * UITEN-278 - update package.json * UITEN-278 - Add prop type validation * UITEN-278 - update jest config * UITEN-278 - add unit test * UITEN-278 - update to final form * UITEN-278 - update uni test * UITEN-278 - refinement * UITEN-278 - add validators, refine code * UITEN-278 - refine * UITEN-278 - disable lint rule * UITEN-278 - refine * UITEN-278 - fix prop types sonar problem * UITEN-278 - update permission check on reading room access * UITEN-278 - fix review comments * UIU-282, UIU-283 - Implementation for Update and Delete of Reading Room Access (#398) * UITEN-278 - create reading room from tenant settings * UITEN-278 - update package.json * UITEN-278 - Add prop type validation * UITEN-278 - update jest config * UITEN-278 - add unit test * UITEN-278 - update to final form * UITEN-278 - update uni test * UITEN-278 - refinement * UITEN-278 - add validators, refine code * UITEN-278 - refine * UITEN-278 - disable lint rule * UITEN-278 - refine * UITEN-278 - fix prop types sonar problem * UITEN-278 - update permission check on reading room access * UITEN-282, UITEN-283 - Implementation for update and delete reading room access.

…ent value (follow up) (#400) * UITEN-276 - Reading Room Access settings page basic layout implementation (#389) * UITEN-276 - Reading Room Access settings page basic layout implementation * UITEN-276 - fix review comments * UITEN-278 - create reading room from tenant settings * UITEN-278 - update package.json * UITEN-278 - Add prop type validation * UITEN-278 - update jest config * UITEN-278 - add unit test * UITEN-278 - update to final form * UITEN-278 - update uni test * UITEN-278 - refinement * UITEN-278 - add validators, refine code * UITEN-278 - refine * UITEN-278 - disable lint rule * UITEN-278 - refine * UITEN-278 - fix prop types sonar problem * UITEN-278 - update permission check on reading room access * UITEN-278 - create reading room from tenant settings (#393) * UITEN-278 - create reading room from tenant settings * UITEN-278 - update package.json * UITEN-278 - Add prop type validation * UITEN-278 - update jest config * UITEN-278 - add unit test * UITEN-278 - update to final form * UITEN-278 - update uni test * UITEN-278 - refinement * UITEN-278 - add validators, refine code * UITEN-278 - refine * UITEN-278 - disable lint rule * UITEN-278 - refine * UITEN-278 - fix prop types sonar problem * UITEN-278 - update permission check on reading room access * UITEN-278 - fix review comments * UITEN-282, UITEN-283 - Implementation for update and delete reading room access. * UITEN-282 - fix the default value of Public checkbox to pick the current value

…ow-up) (#401) * UITEN-276 - Reading Room Access settings page basic layout implementation (#389) * UITEN-276 - Reading Room Access settings page basic layout implementation * UITEN-276 - fix review comments * UITEN-278 - create reading room from tenant settings * UITEN-278 - update package.json * UITEN-278 - Add prop type validation * UITEN-278 - update jest config * UITEN-278 - add unit test * UITEN-278 - update to final form * UITEN-278 - update uni test * UITEN-278 - refinement * UITEN-278 - add validators, refine code * UITEN-278 - refine * UITEN-278 - disable lint rule * UITEN-278 - refine * UITEN-278 - fix prop types sonar problem * UITEN-278 - update permission check on reading room access * UITEN-278 - create reading room from tenant settings (#393) * UITEN-278 - create reading room from tenant settings * UITEN-278 - update package.json * UITEN-278 - Add prop type validation * UITEN-278 - update jest config * UITEN-278 - add unit test * UITEN-278 - update to final form * UITEN-278 - update uni test * UITEN-278 - refinement * UITEN-278 - add validators, refine code * UITEN-278 - refine * UITEN-278 - disable lint rule * UITEN-278 - refine * UITEN-278 - fix prop types sonar problem * UITEN-278 - update permission check on reading room access * UITEN-278 - fix review comments * UITEN-282, UITEN-283 - Implementation for update and delete reading room access. * UITEN-282 - fix the default value of Public checkbox to pick the current value * UITEN-282 - add null checks for isPublic field value and also have a default value in case it is not available. * UITEN-282 - refine code * UITEN-282 - refine the validation for service points * UITEN-282 - refine the validation for service points

UITEN-276 - Reading Room Access settings page basic layout implementa…

107b5da

…tion

Terala-Priyanka marked this pull request as draft March 25, 2024 10:04

Terala-Priyanka self-assigned this Mar 25, 2024

Merge branch 'master' into UITEN-276

7851175

Terala-Priyanka changed the base branch from master to RRA-Settings March 26, 2024 05:14

Terala-Priyanka marked this pull request as ready for review March 26, 2024 11:59

Terala-Priyanka requested review from manvendra-s-rathore and a team March 26, 2024 12:01

BogdanDenis requested changes Mar 26, 2024

View reviewed changes

manvendra-s-rathore approved these changes Mar 27, 2024

View reviewed changes

UITEN-276 - fix review comments

861b7d4

Terala-Priyanka requested a review from BogdanDenis March 27, 2024 05:09

BogdanDenis approved these changes Mar 27, 2024

View reviewed changes

Terala-Priyanka merged commit 4f3c344 into RRA-Settings Mar 27, 2024
6 checks passed

Terala-Priyanka deleted the UITEN-276 branch March 27, 2024 11:01

zburke reviewed Mar 27, 2024

View reviewed changes

Terala-Priyanka mentioned this pull request May 2, 2024

Merge Reading Room Access settings to master branch #399

Merged

11 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

UITEN-276 - Reading Room Access settings page basic layout implementation #389

UITEN-276 - Reading Room Access settings page basic layout implementation #389

Terala-Priyanka commented Mar 25, 2024 •

edited

Loading

github-actions bot commented Mar 25, 2024 •

edited

Loading

github-actions bot commented Mar 25, 2024 •

edited

Loading

BogdanDenis Mar 26, 2024

BogdanDenis Mar 26, 2024

Terala-Priyanka Mar 27, 2024

sonarqubecloud bot commented Mar 27, 2024

zburke left a comment •

edited

Loading

zburke Mar 27, 2024

zburke Mar 27, 2024

zburke Mar 27, 2024

Terala-Priyanka commented Mar 28, 2024

UITEN-276 - Reading Room Access settings page basic layout implementation #389

UITEN-276 - Reading Room Access settings page basic layout implementation #389

Conversation

Terala-Priyanka commented Mar 25, 2024 • edited Loading

Purpose

Approach

Screenshots

Pre-Merge Checklist

github-actions bot commented Mar 25, 2024 • edited Loading

Jest Unit Test Statistics

github-actions bot commented Mar 25, 2024 • edited Loading

BigTest Unit Test Statistics

BogdanDenis Mar 26, 2024

Choose a reason for hiding this comment

BogdanDenis Mar 26, 2024

Choose a reason for hiding this comment

Terala-Priyanka Mar 27, 2024

Choose a reason for hiding this comment

sonarqubecloud bot commented Mar 27, 2024

Quality Gate passed

zburke left a comment • edited Loading

Choose a reason for hiding this comment

zburke Mar 27, 2024

Choose a reason for hiding this comment

zburke Mar 27, 2024

Choose a reason for hiding this comment

zburke Mar 27, 2024

Choose a reason for hiding this comment

Terala-Priyanka commented Mar 28, 2024

Terala-Priyanka commented Mar 25, 2024 •

edited

Loading

github-actions bot commented Mar 25, 2024 •

edited

Loading

github-actions bot commented Mar 25, 2024 •

edited

Loading

zburke left a comment •

edited

Loading