Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

UITEN-266 No errors when inventory-related interfaces are not present #375

Merged
merged 1 commit into from
Oct 19, 2023

Conversation

MikeTaylor
Copy link
Contributor

@MikeTaylor MikeTaylor commented Oct 19, 2023

We now check for the presence of relevant interfaces at run-time, and no make unsupported settings pages available when the relevant interfaces are not present.

Those interfaces (all to do with inventory) are now marked as optional in the package file.

It will be possible to simplify this code if/when support for the iface element is added to the <Settings> component, but the present fix does not depend on that future change: see folio-org/stripes-smart-components#1401 (comment)

Fixes UITEN-266.

We now check for the presence of relevant interfaces at run-time, and
no make unsupported settings pages available when the relevant
interfaces are not present.

Those interfaces (all to do with inventory) are now marked as optional
in the package file.

It will be possible to simplify this code if/when support for the
`iface` element is added to the `<Settings>` component, but the
present fix does not depend on that future change: see
folio-org/stripes-smart-components#1401 (comment)

Fixes UITEN-266.
@github-actions
Copy link

github-actions bot commented Oct 19, 2023

Jest Unit Test Statistics

  1 files  ±0  26 suites  ±0   1m 46s ⏱️ +18s
94 tests ±0  89 ✔️ ±0  5 💤 ±0  0 ±0 
96 runs  ±0  91 ✔️ ±0  5 💤 ±0  0 ±0 

Results for commit 4ed7088. ± Comparison against base commit 7f46897.

♻️ This comment has been updated with latest results.

@github-actions
Copy link

github-actions bot commented Oct 19, 2023

BigTest Unit Test Statistics

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

Results for commit 4ed7088. ± Comparison against base commit 7f46897.

♻️ This comment has been updated with latest results.

@sonarqubecloud
Copy link

Kudos, SonarCloud Quality Gate passed!    Quality Gate passed

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 0 Code Smells

0.0% 0.0% Coverage
0.0% 0.0% Duplication

@MikeTaylor MikeTaylor merged commit 680c5c4 into master Oct 19, 2023
5 checks passed
@MikeTaylor MikeTaylor deleted the UITEN-266--optional-inventory branch October 19, 2023 18:26
@vashjs
Copy link
Contributor

vashjs commented Oct 31, 2023

@MikeTaylor @zburke moved the discussion of this problem to the original PR.

After this change we are facing bug in tenant-settings - UITEN-268. If we follow a direct link (you must be logged out) to the tenant (for example https://folio-snapshot.dev.folio.org/settings/tenant-settings), then the routes for the locations are not loaded and looks like this:

image

@MikeTaylor
Copy link
Contributor Author

Hmm. That will be because Stripes is rendering this Settings page before it's finished doing its own setup (loading the information about which interfaces Okapi knows about). That seems like a Stripes bug to me. Thoughts, @zburke @mkuklis @skomorokh?

@zburke zburke changed the title No errors when inventory-related interfaces are not present UITEN-266 No errors when inventory-related interfaces are not present Nov 1, 2023
@zburke
Copy link
Member

zburke commented Nov 1, 2023

Gotta be honest, @MikeTaylor: I have no idea what's going on here. It's been a looooong time since I did any work on the components that are in play here. It would be great if you or @vashjs, could investigate this further, but if you're reasonably confident that UITEN-268 surfaced a bug in a stripes component, rather than a bug introduced by this PR, then assign it to Stripes Force and we'll take a look.

@MikeTaylor
Copy link
Contributor Author

OK, @zburke, I'll do a little more investigation before handing it off.

@MikeTaylor
Copy link
Contributor Author

@vashjs I can't reproduce this. Logging into snapshot and choosing Settings --> Tenant takes me to https://folio-snapshot.dev.folio.org/settings/tenant-settings which displays correctly. Reloading displays correctly. Shift-reloading displays correctly.

Can you give me a recipe to reproduce the problem?

@MikeTaylor
Copy link
Contributor Author

Aha! @vashjs, I find I can reproduce this by copying the URL (in the current case http://localhost:3000/settings/tenant-settings) then opening a private window, pasting that URL into the bar, and logging in.

Please file a Jira and assign to me.

@vashjs
Copy link
Contributor

vashjs commented Nov 2, 2023

Aha! @vashjs, I find I can reproduce this by copying the URL (in the current case http://localhost:3000/settings/tenant-settings) then opening a private window, pasting that URL into the bar, and logging in.

Please file a Jira and assign to me.

@MikeTaylor I've assigned UITEN-268. Thanks for help!

@MikeTaylor
Copy link
Contributor Author

Perfect, thank you.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants