UITEN-340 Use locale API to fetch and update tenant locale settings.#486
UITEN-340 Use locale API to fetch and update tenant locale settings.#486BogdanDenis merged 13 commits intomasterfrom
Conversation
Jest Unit Test Results 1 files ± 0 45 suites +2 2m 19s ⏱️ ±0s Results for commit 524f478. ± Comparison against base commit 653aa76. This pull request skips 2 tests.♻️ This comment has been updated with latest results. |
| "displayName": "Settings (tenant): Can view language, localization, and currency", | ||
| "subPermissions": [ | ||
| "mod-settings.entries.collection.get", | ||
| "mod-settings.global.read.stripes-core.prefs.manage", |
There was a problem hiding this comment.
I believe mod-settings.global.read.stripes-core.prefs.manage not required anymore
package.json
Outdated
| "mod-settings.entries.collection.get", | ||
| "mod-settings.global.read.stripes-core.prefs.manage", | ||
| "locale.item.get", | ||
| "locale.item.put", |
There was a problem hiding this comment.
view set shouldn't include put
|
@BogdanDenis can we merge this PR, since it has already 2 approvals, we have 5 BE stories already that waiting this PR merged to merge them in the same time |
|
Sorry, @SerhiiNosko, I didn't realize other work was blocked on this. I'll take a look in the next few hours. |
| // for some reason react-query returns isLoading as true even for disabled queries...this is an expected behaviour according to them | ||
| // so we need to also check for a present interface to not have false positives |
There was a problem hiding this comment.
This is most likely because stripes.hasInterface returns undefined instead of false. enabled requires a boolean value.
There was a problem hiding this comment.
I think it's a bug/weird convention within react-query itself. I had tried explicitly converting into a bool value and still the first render gave isLoading: true
| isUsingLocaleApi | ||
| ? renderLocaleApiForm() | ||
| : renderSettingsEntriesForm() |
There was a problem hiding this comment.
Is there a chance that /locale will not be included in Trillium?
There was a problem hiding this comment.
It's a requirement in stripes-core to not have breaking changes and to have a fallback on mod-configurations, and so we need to support it here too.
There was a problem hiding this comment.
I think it was about stripes-core/smart/components, not ui-* modules. cc @zburke
There was a problem hiding this comment.
It was, but I think if stripes-core can potentially fall-back to using mod-configurations, then UI should also do the same so that admins can change tenant locale via UI
There was a problem hiding this comment.
You're both right :) In this release, stripes-core must avoid breaking changes and thus retain the ability to read from mod-configuration, even as we move these values to the new /locale endpoint on mod-settings. Does that mean ui-tenant-settings, likewise, must retain the ability to write to mod-configuration?
Technically no. It's messy. We are caught in the middle here: we pledged not to break the component APIs in @folio/stripes but in many cases UI apps already broke Okapi-interface APIs (e.g. by bumping their minimum interface version from 1.0 to 1.1 so they can take advantage of the new features, or by adding a new required interface, or bumping to a new major version of an existing interface). Probably it's very unlikely a tenant (or an operator) would choose to update their UI out of step with the backend, or update some UI modules without also updating @folio/stripes, but that is what we pledged to enable.
Given we have the fallback implemented already, @BogdanDenis, I'd love to keep it for now and only remove it when we do the corresponding work in stripes-core. But if you want to remove it now, as @Dmytro-Melnyshyn seems to be suggesting, I would not fight you.
src/settings/Locale/index.js
Outdated
| @@ -0,0 +1 @@ | |||
| export { default as Locale } from './Locale'; | |||
There was a problem hiding this comment.
Plugins/index.js and others just do export { default }, allowing settings/index.js to continue with
import Locale from './Locale';
instead of
import { Locale } from './Locale';
This is fine, obviously, but the consistency might be nice. Your call.
|
…& locale settings (#1691) ## Description **Context**: With the Trillium release, Language & locale settings were moved from mod-configuration to the /locale API of mod-settings. These settings are tenant level (a user can also set user preference for language at some other API). - Front-end modules that use these tenant settings need to call the /locale endpoint to get these settings. - Update required interfaces and required permissions from mod-configuration to mod-settings. - If this work is not completed in Trillium then any workflows using locale settings will not work as expected. Also this work is required for mod-configuration deprecation. ## Approach - Fetch tenant locale settings from `/locale` API. - Removed calls to fetch tenant locale settings from `/settings/entries` and `/configurations/entries` - Removed calls to fetch user locale settings from `/configurations/entries` - User locale settings are now fetched from `/settings/entries` ## Issues [STCOR-1027](https://folio-org.atlassian.net/browse/STCOR-1027) ## Related PRs folio-org/ui-tenant-settings#486



Description
Context: With the Trillium release, Language & locale settings were moved from mod-configuration to the /locale API of mod-settings. These settings are tenant level (a user can also set user preference for language at some other API).
Also this work is required for mod-configuration deprecation.
Approach
/localeAPI.<Locale>component to use the new locale api if it's available, and/configurations/entriesas a fall-backIssues
UITEN-340
Related PRs
folio-org/stripes-core#1691