MODFQMMGR-746 Use mod-settings instead of mod-config for locale settings - #1146
Merged
Conversation
mweaver-ebsco
changed the title
Contributor
There was a problem hiding this comment.
Pull request overview
This PR replaces the legacy SettingsClient/mod-configuration-based locale & currency handling with a new LocaleClient that uses the /locale API, updates all dependent services, cleans up now-unused configuration entity types, and attempts to pin the Docker API version for Testcontainers via Maven Surefire/Failsafe configuration.
Changes:
- Introduced
LocaleClient(backed by/locale) and refactoredResultSetService,EntityTypeService,DataRefreshRepository, and the V4 date migration strategy to consume it instead ofSettingsClientand raw configuration entries. - Removed the
simple_config_dataentity type and related joins/permissions from order-related entity-type JSON definitions that depended on configuration entries. - Adjusted test code to target the new locale flow and added
LocaleClientTest; updatedpom.xmlto set system properties intended for Testcontainers’ Docker API version and simplified test JVM arg handling.
Reviewed changes
Copilot reviewed 18 out of 18 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
src/main/java/org/folio/fqm/client/LocaleClient.java |
New client for the /locale API, encapsulating locale, currency, timezone, and numbering system plus safe defaults and timezone parsing. |
src/main/java/org/folio/fqm/service/ResultSetService.java |
Switched date localization to use LocaleClient for tenant timezone instead of SettingsClient; behavior is preserved while using /locale. |
src/main/java/org/folio/fqm/service/EntityTypeService.java |
Simplified language localization: now pulls the UI locale directly from LocaleClient instead of manually parsing configuration entries. |
src/main/java/org/folio/fqm/repository/DataRefreshRepository.java |
Reworked system-currency resolution to use LocaleClient rather than parsing mod-configuration; still guards unsupported currencies and uses the same exchange-rate fetch flow. |
src/main/java/org/folio/fqm/migration/strategies/impl/V4DateFieldTimezoneAddition.java |
Updated migration to derive the tenant timezone via LocaleClient and cache the resolved ZoneId in the migration state. |
src/main/java/org/folio/fqm/migration/MigrationStrategyRepository.java |
Updated constructor wiring to pass LocaleClient into V4DateFieldTimezoneAddition instead of SettingsClient. |
src/main/java/org/folio/fqm/client/SettingsClientRaw.java |
Removed obsolete Feign client for mod-settings’ entries endpoint. |
src/main/java/org/folio/fqm/client/SettingsClient.java |
Removed wrapper client now superseded by the new LocaleClient. |
src/test/java/org/folio/fqm/client/SettingsClientTest.java |
Deleted tests tied to the removed SettingsClient. |
src/test/java/org/folio/fqm/client/LocaleClientTest.java |
Added tests covering successful locale parsing and default/fallback behavior on Feign errors and invalid JSON. |
src/main/resources/entity-types/configuration/simple_config_data.json5 |
Deleted the simple_config_data entity type, which previously exposed configuration entries as an entity. |
src/main/resources/entity-types/orders/simple_purchase_order.json5 |
Removed joins from bill_to and ship_to columns to the now-deleted simple_config_data entity type; those fields remain as raw UUIDs. |
src/main/resources/entity-types/orders/composite_po_instance.json5 |
Removed configuration.entries.collection.get from requiredPermissions, reflecting the removal of simple_config_data and configuration-based joins. |
src/test/java/org/folio/fqm/service/ResultSetServiceTest.java |
Updated tests to mock LocaleClient and pass timezone identifiers as strings rather than ZoneId, aligning with the new API. |
src/test/java/org/folio/fqm/service/EntityTypeServiceTest.java |
Adjusted language-related tests to use LocaleClient for locale resolution instead of an HTTP JSON payload to mod-configuration. |
src/test/java/org/folio/fqm/repository/DataRefreshRepositoryTest.java |
Reworked exchange-rate refresh tests to rely on LocaleClient for system currency and simplified locale configuration mocking; ensured behavior for supported, unsupported, and default currency paths. |
src/test/java/org/folio/fqm/migration/strategies/impl/V4DateFieldTimezoneAdditionTest.java |
Updated migration tests to mock LocaleClient and verify its use (or non-use) depending on whether date fields are present. |
pom.xml |
Set a global argLine property to enforce UTF-8 and added Surefire/Failsafe systemProperties entries intended to pin the Docker API version for Testcontainers, though the property name/structure currently won’t be recognized by Testcontainers as-is. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
bvsharp
approved these changes
Jan 30, 2026
mweaver-ebsco
force-pushed
the
MODFQMMGR-746-mod-config-ets
branch
9 times, most recently
from
February 3, 2026 17:01
921c444 to
8ea8da9
Compare
mweaver-ebsco
force-pushed
the
MODFQMMGR-746-mod-config-ets
branch
2 times, most recently
from
February 3, 2026 18:08
abd78f1 to
8486453
Compare
mweaver-ebsco
commented
Feb 3, 2026
| * Provides raw access to the /locale API. | ||
| */ | ||
| @FeignClient(name = "locale") | ||
| interface LocaleClientRaw { |
Collaborator
Author
There was a problem hiding this comment.
This is the same basic idea as with SettingsClient and SettingsClientRaw, except I hid the raw version inside this one, to keep things a little simpler
mweaver-ebsco
commented
Feb 3, 2026
Collaborator
Author
There was a problem hiding this comment.
I suppose this should just be
Suggested change
| return oldFieldName; |
mweaver-ebsco
commented
Feb 3, 2026
| id: '4d626ce1-1880-48d2-9d4c-81667fdc5dbb', | ||
| name: 'simple_invoice', | ||
| private: false, | ||
| private: true, |
Collaborator
Author
There was a problem hiding this comment.
The migration basically just moves queries from this ET to the new composite (and adjusts references to the now-deleted bill_to field)
mweaver-ebsco
commented
Feb 3, 2026
Comment on lines
-5
to
-8
| requiredPermissions: [ | ||
| // This inherited from simple_config_data, but is also included here because of the added columns here that also retrieve config data | ||
| 'configuration.entries.collection.get', | ||
| ], |
Collaborator
Author
There was a problem hiding this comment.
Context: There used to be some columns defined in this composite that used data from mod-config. Those are gone now, so we don't need the permission anymore
mweaver-ebsco
force-pushed
the
MODFQMMGR-746-mod-config-ets
branch
from
February 3, 2026 18:50
3daa74d to
ada649d
Compare
|
This migration switches queries from simple_invoice to composite_invoice, and moves references to the old invoice.bill_to field to the new version in the composite. Other ETs that contain simple_invoice will only get field warnings if they reference the bill_to field.
bvsharp
approved these changes
Feb 3, 2026
ncovercash
approved these changes
Feb 3, 2026
| @Override | ||
| public Map<UUID, Map<String, FieldWarningFactory>> getFieldWarnings() { | ||
| FieldWarningFactory billToWarning = RemovedFieldWarning.withoutAlternative(); | ||
| return Map.of( |
Member
There was a problem hiding this comment.
You don't need all of these enumerated anymore! Take a look at the new overridable method in the migration docs; specify the composite entity IDs and source aliases and it'll auto-resolve any composite parent changes
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
No description provided.