-
Notifications
You must be signed in to change notification settings - Fork 7
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
Conversation
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.
Kudos, SonarCloud Quality Gate passed! |
@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: |
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? |
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. |
OK, @zburke, I'll do a little more investigation before handing it off. |
@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? |
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! |
Perfect, thank you. |
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.