refactor: update frontend to be compatible with the new externalsettings structure#2274
Open
refactor: update frontend to be compatible with the new externalsettings structure#2274
Conversation
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Hey - I've found 2 issues, and left some high level feedback:
- There are multiple places where you branch on
conditionSettingsKeyinSharedConditionComponent(e.g. inngOnInit,updateStore,updateConditionField); consider centralizing this mapping (e.g. a lookup object from key → selector/dispatchers) to reduce duplication and the risk of inconsistencies when new condition types are added. - The list of external settings keys (e.g. in
settingsToCheckinsideUserEffects) is currently hard-coded; extracting this into a shared typed constant or enum would make it easier to keep frontend logic aligned with the backendexternalSettingsstructure as it evolves.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- There are multiple places where you branch on `conditionSettingsKey` in `SharedConditionComponent` (e.g. in `ngOnInit`, `updateStore`, `updateConditionField`); consider centralizing this mapping (e.g. a lookup object from key → selector/dispatchers) to reduce duplication and the risk of inconsistencies when new condition types are added.
- The list of external settings keys (e.g. in `settingsToCheck` inside `UserEffects`) is currently hard-coded; extracting this into a shared typed constant or enum would make it easier to keep frontend logic aligned with the backend `externalSettings` structure as it evolves.
## Individual Comments
### Comment 1
<location path="src/app/state-management/effects/user.effects.ts" line_range="405-414" />
<code_context>
+ setConditions$ = createEffect(() => {
</code_context>
<issue_to_address>
**question (bug_risk):** Re-evaluate removing selectColumnAction when restoring conditions from settings
Previously this effect also dispatched `selectColumnAction` for each enabled condition so the related columns became visible when restoring from settings. Now it only dispatches `addScientificConditionAction` and `updateConditionsConfigs`, which may drop that auto-enable behavior. Please confirm whether the UI still depends on this, and if the change is intentional, ensure there’s a clear mechanism to keep columns and conditions in sync.
</issue_to_address>
### Comment 2
<location path="src/app/state-management/effects/user.effects.ts" line_range="332" />
<code_context>
+ ...(userSettings.externalSettings || {}),
+ };
+
+ const settingsToCheck = [
+ "fe_dataset_table_columns",
+ "fe_dataset_table_conditions",
</code_context>
<issue_to_address>
**issue (complexity):** Consider introducing shared helper utilities and a centralised settings configuration map so the new external settings logic is declarative, typed, and reused across effects instead of relying on repeated string operations and inline branching.
You can reduce the new complexity by centralising the stringly-typed logic and the “which conditions to use” decision into small, reusable helpers.
### 1. Replace `startsWith`/`replace` chains with a typed settings config
Define a single source of truth for external setting keys and their scopes:
```ts
const SETTINGS_CONFIG = [
{ key: "fe_dataset_table_columns", scope: "dataset", configKey: "columns" },
{ key: "fe_dataset_table_conditions", scope: "dataset", configKey: "conditions" },
{ key: "fe_dataset_table_filters", scope: "dataset", configKey: "filters" },
{ key: "fe_proposal_table_columns", scope: "proposal", configKey: "columns" },
{ key: "fe_proposal_table_filters", scope: "proposal", configKey: "filters" },
{ key: "fe_sample_table_columns", scope: "sample", configKey: "columns" },
{ key: "fe_sample_table_conditions", scope: "sample", configKey: "conditions" },
{ key: "fe_instrument_table_columns", scope: "instrument", configKey: "columns" },
{ key: "fe_file_table_columns", scope: "file", configKey: "columns" },
] as const;
type SettingsKey = (typeof SETTINGS_CONFIG)[number]["key"];
```
Then extract a normalisation helper used by `fetchUserSettings$`:
```ts
function normalizeExternalSetting(
settingKey: SettingsKey,
externalSettings: Record<string, any>,
config: any,
initialUserState: typeof initialUserState,
) {
let items = externalSettings[settingKey];
if (Array.isArray(items) && items.length > 0) {
return items;
}
const cfg = SETTINGS_CONFIG.find((s) => s.key === settingKey);
if (!cfg) {
return initialUserState.settings[settingKey] || [];
}
const defaultsByScope: Record<string, any> = {
dataset: config.defaultDatasetsListSettings,
proposal: config.defaultProposalsListSettings,
// sample / instrument / file could use their own config section if available
};
const scopeDefaults = defaultsByScope[cfg.scope] ?? {};
return (
scopeDefaults?.[cfg.configKey] ||
initialUserState.settings[settingKey] ||
[]
);
}
```
Usage in `fetchUserSettings$` (no `startsWith`/`replace`):
```ts
map((userSettings: UserSettings) => {
const config = this.configService.getConfig();
const externalSettings = { ...(userSettings.externalSettings || {}) };
for (const { key } of SETTINGS_CONFIG) {
externalSettings[key] = normalizeExternalSetting(
key,
externalSettings,
config,
initialUserState,
);
}
const normalizedUserSettings = {
...userSettings,
externalSettings,
};
return fromActions.fetchUserSettingsCompleteAction({
userSettings: normalizedUserSettings,
});
}),
```
This keeps all behaviour but removes the duplicated `startsWith("fe_dataset_table_")` / `replace(...)` branching.
### 2. Reuse the same config to simplify `updateUserSettings$`
Instead of keeping a separate `settingsToNest` array (which can easily diverge), build it from `SETTINGS_CONFIG`:
```ts
const SETTINGS_KEYS = SETTINGS_CONFIG.map((s) => s.key);
updateUserSettings$ = createEffect(() => {
return this.actions$.pipe(
ofType(fromActions.updateUserSettingsAction),
concatLatestFrom(() => [this.user$]),
takeWhile(([_, user]) => !!user),
switchMap(([{ property }, user]) => {
const propertyKeys = Object.keys(property) as SettingsKey[];
const newProperty: Record<string, any> = {};
const useExternalSettings = propertyKeys.some((k) =>
SETTINGS_KEYS.includes(k),
);
propertyKeys.forEach((key) => {
newProperty[key] = property[key];
});
const apiCall$ = useExternalSettings
? this.usersService.usersControllerPatchExternalSettingsV3(
user?.id,
JSON.stringify(newProperty) as any,
)
: this.usersService.usersControllerPatchSettingsV3(
user?.id,
JSON.stringify(newProperty) as any,
);
return apiCall$.pipe(
map((userSettings: UserSettings) =>
fromActions.updateUserSettingsCompleteAction({ userSettings }),
),
catchError(() => of(fromActions.updateUserSettingsFailedAction())),
);
}),
);
});
```
Now both fetching and updating use the same `SETTINGS_CONFIG`, removing duplication and reducing the chance of missing a key in one place.
### 3. Extract the conditions selection logic from `setConditions$`
The length-based choice between incoming and existing conditions can be hidden behind a pure helper, making the effect easier to scan:
```ts
function resolveConditions(
incoming: any[] | undefined,
existing: any[] | undefined,
) {
if (incoming && incoming.length > 0) {
return incoming;
}
return existing || [];
}
```
Then `setConditions$` becomes:
```ts
setConditions$ = createEffect(() => {
return this.actions$.pipe(
ofType(fromActions.fetchUserSettingsCompleteAction),
concatLatestFrom(() => this.store.select(selectConditions)),
mergeMap(([{ userSettings }, existingConditions]) => {
const incomingConditions =
(userSettings as any).externalSettings?.fe_dataset_table_conditions ||
[];
const conditions = resolveConditions(
incomingConditions,
existingConditions,
);
const actions = [];
conditions
.filter((condition) => condition.enabled)
.forEach((condition) => {
actions.push(
addScientificConditionAction({
condition: condition.condition,
}),
);
});
actions.push(
fromActions.updateConditionsConfigs({
conditionConfigs: conditions,
}),
);
return actions;
}),
);
});
```
This keeps the current behaviour (prefer incoming if any, otherwise fall back) but moves the decision logic into a small, testable function and makes the effect itself more declarative.
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
Comment on lines
405
to
+414
| setConditions$ = createEffect(() => { | ||
| return this.actions$.pipe( | ||
| ofType(fromActions.fetchUserSettingsCompleteAction), | ||
| mergeMap(({ userSettings }) => { | ||
| concatLatestFrom(() => this.store.select(selectConditions)), | ||
| mergeMap(([{ userSettings }, existingConditions]) => { | ||
| const actions = []; | ||
|
|
||
| const incomingConditions = | ||
| (userSettings as any).externalSettings?.fe_dataset_table_conditions || | ||
| []; |
Contributor
There was a problem hiding this comment.
question (bug_risk): Re-evaluate removing selectColumnAction when restoring conditions from settings
Previously this effect also dispatched selectColumnAction for each enabled condition so the related columns became visible when restoring from settings. Now it only dispatches addScientificConditionAction and updateConditionsConfigs, which may drop that auto-enable behavior. Please confirm whether the UI still depends on this, and if the change is intentional, ensure there’s a clear mechanism to keep columns and conditions in sync.
| ...(userSettings.externalSettings || {}), | ||
| }; | ||
|
|
||
| const settingsToCheck = [ |
Contributor
There was a problem hiding this comment.
issue (complexity): Consider introducing shared helper utilities and a centralised settings configuration map so the new external settings logic is declarative, typed, and reused across effects instead of relying on repeated string operations and inline branching.
You can reduce the new complexity by centralising the stringly-typed logic and the “which conditions to use” decision into small, reusable helpers.
1. Replace startsWith/replace chains with a typed settings config
Define a single source of truth for external setting keys and their scopes:
const SETTINGS_CONFIG = [
{ key: "fe_dataset_table_columns", scope: "dataset", configKey: "columns" },
{ key: "fe_dataset_table_conditions", scope: "dataset", configKey: "conditions" },
{ key: "fe_dataset_table_filters", scope: "dataset", configKey: "filters" },
{ key: "fe_proposal_table_columns", scope: "proposal", configKey: "columns" },
{ key: "fe_proposal_table_filters", scope: "proposal", configKey: "filters" },
{ key: "fe_sample_table_columns", scope: "sample", configKey: "columns" },
{ key: "fe_sample_table_conditions", scope: "sample", configKey: "conditions" },
{ key: "fe_instrument_table_columns", scope: "instrument", configKey: "columns" },
{ key: "fe_file_table_columns", scope: "file", configKey: "columns" },
] as const;
type SettingsKey = (typeof SETTINGS_CONFIG)[number]["key"];Then extract a normalisation helper used by fetchUserSettings$:
function normalizeExternalSetting(
settingKey: SettingsKey,
externalSettings: Record<string, any>,
config: any,
initialUserState: typeof initialUserState,
) {
let items = externalSettings[settingKey];
if (Array.isArray(items) && items.length > 0) {
return items;
}
const cfg = SETTINGS_CONFIG.find((s) => s.key === settingKey);
if (!cfg) {
return initialUserState.settings[settingKey] || [];
}
const defaultsByScope: Record<string, any> = {
dataset: config.defaultDatasetsListSettings,
proposal: config.defaultProposalsListSettings,
// sample / instrument / file could use their own config section if available
};
const scopeDefaults = defaultsByScope[cfg.scope] ?? {};
return (
scopeDefaults?.[cfg.configKey] ||
initialUserState.settings[settingKey] ||
[]
);
}Usage in fetchUserSettings$ (no startsWith/replace):
map((userSettings: UserSettings) => {
const config = this.configService.getConfig();
const externalSettings = { ...(userSettings.externalSettings || {}) };
for (const { key } of SETTINGS_CONFIG) {
externalSettings[key] = normalizeExternalSetting(
key,
externalSettings,
config,
initialUserState,
);
}
const normalizedUserSettings = {
...userSettings,
externalSettings,
};
return fromActions.fetchUserSettingsCompleteAction({
userSettings: normalizedUserSettings,
});
}),This keeps all behaviour but removes the duplicated startsWith("fe_dataset_table_") / replace(...) branching.
2. Reuse the same config to simplify updateUserSettings$
Instead of keeping a separate settingsToNest array (which can easily diverge), build it from SETTINGS_CONFIG:
const SETTINGS_KEYS = SETTINGS_CONFIG.map((s) => s.key);
updateUserSettings$ = createEffect(() => {
return this.actions$.pipe(
ofType(fromActions.updateUserSettingsAction),
concatLatestFrom(() => [this.user$]),
takeWhile(([_, user]) => !!user),
switchMap(([{ property }, user]) => {
const propertyKeys = Object.keys(property) as SettingsKey[];
const newProperty: Record<string, any> = {};
const useExternalSettings = propertyKeys.some((k) =>
SETTINGS_KEYS.includes(k),
);
propertyKeys.forEach((key) => {
newProperty[key] = property[key];
});
const apiCall$ = useExternalSettings
? this.usersService.usersControllerPatchExternalSettingsV3(
user?.id,
JSON.stringify(newProperty) as any,
)
: this.usersService.usersControllerPatchSettingsV3(
user?.id,
JSON.stringify(newProperty) as any,
);
return apiCall$.pipe(
map((userSettings: UserSettings) =>
fromActions.updateUserSettingsCompleteAction({ userSettings }),
),
catchError(() => of(fromActions.updateUserSettingsFailedAction())),
);
}),
);
});Now both fetching and updating use the same SETTINGS_CONFIG, removing duplication and reducing the chance of missing a key in one place.
3. Extract the conditions selection logic from setConditions$
The length-based choice between incoming and existing conditions can be hidden behind a pure helper, making the effect easier to scan:
function resolveConditions(
incoming: any[] | undefined,
existing: any[] | undefined,
) {
if (incoming && incoming.length > 0) {
return incoming;
}
return existing || [];
}Then setConditions$ becomes:
setConditions$ = createEffect(() => {
return this.actions$.pipe(
ofType(fromActions.fetchUserSettingsCompleteAction),
concatLatestFrom(() => this.store.select(selectConditions)),
mergeMap(([{ userSettings }, existingConditions]) => {
const incomingConditions =
(userSettings as any).externalSettings?.fe_dataset_table_conditions ||
[];
const conditions = resolveConditions(
incomingConditions,
existingConditions,
);
const actions = [];
conditions
.filter((condition) => condition.enabled)
.forEach((condition) => {
actions.push(
addScientificConditionAction({
condition: condition.condition,
}),
);
});
actions.push(
fromActions.updateConditionsConfigs({
conditionConfigs: conditions,
}),
);
return actions;
}),
);
});This keeps the current behaviour (prefer incoming if any, otherwise fall back) but moves the decision logic into a small, testable function and makes the effect itself more declarative.
Member
There was a problem hiding this comment.
@abdimo101 Could you check this review and see if suggestion is valid, I like the idea Define a single source of truth for external setting keys and their scopes: which will be beneficial for the future either we decide to have a dto or not in the future
Contributor
There was a problem hiding this comment.
Yes, the suggestion is valid and fits nicely with the changes in this PR.
Right now in user.effects.ts you can see the duplicated, string-based logic in two places:
fetchUserSettings$hassettingsToCheckand then astartsWith("fe_dataset_table_")/replace(...)chain.updateUserSettings$has its ownsettingsToNestarray with the same keys.
The proposed SETTINGS_CONFIG works well for this because:
-
Single source of truth for keys and scopes
It encodes exactly what you already have insettingsToCheck/settingsToNest:- The external settings key (e.g.
fe_dataset_table_columns) - Its semantic scope (
dataset,proposal,sample, etc.) - The corresponding config key (
columns,filters,conditions)
That matches how you’re currently deriving
keyviasetting.replace("fe_dataset_table_", "")to look upconfig.defaultDatasetsListSettings[key]orconfig.defaultProposalsListSettings[key]. - The external settings key (e.g.
-
Behaviour stays the same, complexity goes down
AnormalizeExternalSettinghelper can mirror the existing logic exactly:- If there’s a non-empty array in
userSettings.externalSettings[key], use it. - Otherwise, fall back to the appropriate
config.default*ListSettings[configKey]based on scope. - If that’s missing, fall back to
initialUserState.settings[key]or[].
So you’re mostly just moving the logic into a typed, declarative structure instead of relying on string prefixes and manual arrays.
- If there’s a non-empty array in
-
Prevents drift between fetch/update paths
settingsToCheckandsettingsToNestmust stay in sync; the config-based approach lets you derive both from the sameSETTINGS_CONFIGso you can’t accidentally add a newfe_*setting in only one place. -
Future-proof for a DTO or additional entities
Even if you introduce a DTO later, this config will still be valuable:- you can keep mapping DTO fields to
SETTINGS_CONFIGentries in one place; - adding a new entity (e.g.
fe_logbook_table_columns) is a single-line change in the config.
- you can keep mapping DTO fields to
If you want to keep this PR’s risk small, one compromise is:
- Introduce the
SETTINGS_CONFIGand a smallnormalizeExternalSettinghelper now and use them only infetchUserSettings$andupdateUserSettings$, matching the current behaviour. - Leave other parts (like
loadDefaultSettings$) as they are, and optionally refactor them in a follow-up PR once this is merged.
So overall: the idea is sound, compatible with the current data shape, and gives a cleaner path forward whether or not you later add a DTO. I’d support adopting it here or in a focused follow-up refactor PR, depending on how much change you want in this one.
4 tasks
Junjiequan
reviewed
Mar 26, 2026
Comment on lines
+397
to
+399
| filterConfigs: | ||
| (userSettings as any).externalSettings?.fe_dataset_table_filters || | ||
| [], |
Member
There was a problem hiding this comment.
Do we still need any type here?
Member
There was a problem hiding this comment.
Please check sourcery reviews, some comments are quite useful
Contributor
There was a problem hiding this comment.
Hey - I've found 1 issue, and left some high level feedback:
- SharedConditionComponent now hard-codes only two conditionSettingsKey values and repeats branching logic in ngOnInit, updateStore, updateConditionField, etc.; consider centralizing the mapping from conditionSettingsKey to selector/dispatch functions so adding new entities (or handling bad keys) is less fragile and avoids duplicated switch/if blocks.
- updateConditionsConfigs in the user reducer now only writes to settings.fe_dataset_table_conditions and no longer maintains the top-level state.conditions field, but UserState still declares conditions and some code/tests may still reference it; either keep that field in sync or remove it entirely to avoid subtle divergence.
- In fetchUserSettings$ the defaulting logic uses app config for dataset and proposal settings but falls back only to initialUserState.settings for sample, instrument, and file table settings; if there are corresponding default*ListSettings for those entities, it would be more consistent to use them as the primary source for defaults.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- SharedConditionComponent now hard-codes only two conditionSettingsKey values and repeats branching logic in ngOnInit, updateStore, updateConditionField, etc.; consider centralizing the mapping from conditionSettingsKey to selector/dispatch functions so adding new entities (or handling bad keys) is less fragile and avoids duplicated switch/if blocks.
- updateConditionsConfigs in the user reducer now only writes to settings.fe_dataset_table_conditions and no longer maintains the top-level state.conditions field, but UserState still declares conditions and some code/tests may still reference it; either keep that field in sync or remove it entirely to avoid subtle divergence.
- In fetchUserSettings$ the defaulting logic uses app config for dataset and proposal settings but falls back only to initialUserState.settings for sample, instrument, and file table settings; if there are corresponding default*ListSettings for those entities, it would be more consistent to use them as the primary source for defaults.
## Individual Comments
### Comment 1
<location path="src/app/shared/modules/shared-condition/shared-condition.component.ts" line_range="85" />
<code_context>
) {}
ngOnInit() {
+ switch (this.conditionSettingsKey) {
+ case "fe_dataset_table_conditions":
</code_context>
<issue_to_address>
**issue (complexity):** Consider centralizing the conditionSettingsKey-specific selector and dispatch logic so that all condition updates flow through a single configuration and helper method.
You can centralize the `conditionSettingsKey` branching and remove the duplicate dispatch logic in `updateConditionField` by deriving the selector + update action once and always going through `updateStore`.
For example:
```ts
// class fields
conditionConfigs$;
private updateConditionConfigs: (conditionConfigs: ConditionConfig[]) => void;
```
Init both in one place:
```ts
ngOnInit() {
const configByKey = {
fe_dataset_table_conditions: {
selector: selectConditions,
updateAction: updateConditionsConfigs,
},
fe_sample_table_conditions: {
selector: selectSampleConditions,
updateAction: updateSampleConditionsConfigs,
},
} as const;
const config = configByKey[this.conditionSettingsKey];
if (!config) {
throw new Error("Invalid conditionSettingsKey");
}
this.conditionConfigs$ = this.store.select(config.selector);
this.updateConditionConfigs = (conditionConfigs: ConditionConfig[]) =>
this.store.dispatch(config.updateAction({ conditionConfigs }));
if (this.showConditions) {
this.buildMetadataMaps();
this.applyEnabledConditions();
}
}
```
Then `updateStore` no longer needs to re-switch on the key:
```ts
private updateStore(updatedConditions: ConditionConfig[]) {
this.updateConditionConfigs(updatedConditions);
this.store.dispatch(
updateUserSettingsAction({
property: { [this.conditionSettingsKey]: updatedConditions },
}),
);
}
```
And `updateConditionField` can delegate through `updateStore` instead of re-implementing the branching:
```ts
updateConditionField(index: number, updates: Partial<ScientificCondition>) {
this.subscriptions.push(
this.conditionConfigs$.pipe(take(1)).subscribe((conditions = []) => {
if (!conditions[index]) return;
const actualIndex = conditions.findIndex(
(c) => c.condition.lhs === conditions[index].condition.lhs,
);
if (actualIndex === -1) return;
const updatedConditions = [...conditions];
const conditionConfig = updatedConditions[actualIndex];
updatedConditions[actualIndex] = {
...conditionConfig,
condition: {
...conditionConfig.condition,
...updates,
human_name: this.humanNameMap[conditionConfig.condition.lhs],
},
};
this.updateStore(updatedConditions);
}),
);
}
```
This keeps all current behavior (including the new user settings key) but removes duplicated `if/else`/`switch` logic and consolidates the dataset vs sample mapping in one place.
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
| @@ -82,6 +83,16 @@ export class SharedConditionComponent implements OnDestroy { | |||
| ) {} | |||
|
|
|||
| ngOnInit() { | |||
Contributor
There was a problem hiding this comment.
issue (complexity): Consider centralizing the conditionSettingsKey-specific selector and dispatch logic so that all condition updates flow through a single configuration and helper method.
You can centralize the conditionSettingsKey branching and remove the duplicate dispatch logic in updateConditionField by deriving the selector + update action once and always going through updateStore.
For example:
// class fields
conditionConfigs$;
private updateConditionConfigs: (conditionConfigs: ConditionConfig[]) => void;Init both in one place:
ngOnInit() {
const configByKey = {
fe_dataset_table_conditions: {
selector: selectConditions,
updateAction: updateConditionsConfigs,
},
fe_sample_table_conditions: {
selector: selectSampleConditions,
updateAction: updateSampleConditionsConfigs,
},
} as const;
const config = configByKey[this.conditionSettingsKey];
if (!config) {
throw new Error("Invalid conditionSettingsKey");
}
this.conditionConfigs$ = this.store.select(config.selector);
this.updateConditionConfigs = (conditionConfigs: ConditionConfig[]) =>
this.store.dispatch(config.updateAction({ conditionConfigs }));
if (this.showConditions) {
this.buildMetadataMaps();
this.applyEnabledConditions();
}
}Then updateStore no longer needs to re-switch on the key:
private updateStore(updatedConditions: ConditionConfig[]) {
this.updateConditionConfigs(updatedConditions);
this.store.dispatch(
updateUserSettingsAction({
property: { [this.conditionSettingsKey]: updatedConditions },
}),
);
}And updateConditionField can delegate through updateStore instead of re-implementing the branching:
updateConditionField(index: number, updates: Partial<ScientificCondition>) {
this.subscriptions.push(
this.conditionConfigs$.pipe(take(1)).subscribe((conditions = []) => {
if (!conditions[index]) return;
const actualIndex = conditions.findIndex(
(c) => c.condition.lhs === conditions[index].condition.lhs,
);
if (actualIndex === -1) return;
const updatedConditions = [...conditions];
const conditionConfig = updatedConditions[actualIndex];
updatedConditions[actualIndex] = {
...conditionConfig,
condition: {
...conditionConfig.condition,
...updates,
human_name: this.humanNameMap[conditionConfig.condition.lhs],
},
};
this.updateStore(updatedConditions);
}),
);
}This keeps all current behavior (including the new user settings key) but removes duplicated if/else/switch logic and consolidates the dataset vs sample mapping in one place.
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
2 participants
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.
Description
This PR updates selectors/reducers/effects, table components, condition filter handling and related tests with new externalSettings table keys for dataset, proposal, instrument and file entities.
New externalSettings structure looks like this:(SciCatProject/backend#2624)
{
externalSettings: {
fe_dataset_table_columns:{...},
fe_dataset_table_conditions:{...},
fe_dataset_table_filters:{...},
fe_proposal_table_columns:{...},
fe_proposal_table_filters:{...},
fe_sample_table_columns:{...},
fe_sample_table_conditions:{...},
fe_instrument_table_columns:{...},
fe_file_table_columns:{...},
}
}
Motivation
Background on use case, changes needed
Fixes:
Please provide a list of the fixes implemented in this PR
Changes:
Please provide a list of the changes implemented by this PR
Tests included
Documentation
official documentation info
If you have updated the official documentation, please provide PR # and URL of the pages where the updates are included
Backend version
Summary by Sourcery
Align frontend user settings, selectors, reducers, effects, and table/condition components with the new externalSettings structure and per-entity keys for columns, filters, and conditions.
Enhancements: