refactor: update frontend to be compatible with the new externalsettings structure

[abdimo101]

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:{...},
}
}

Reason for the removal of selectColumnAction:
The selectColumnAction dispatch was removed because this flow is no longer functional. Enabling a scientific condition does not automatically add it to table columns, since scientific metadata keys are not part of the current column list. But we plan to introduce customizable column functionality in the future, and we will open a follow-up PR to remove the remaining legacy code related to this old flow.

Motivation

Background on use case, changes needed

Fixes:

Please provide a list of the fixes implemented in this PR

• Items added

Changes:

Please provide a list of the changes implemented by this PR

• changes made

Tests included

• Included for each change/fix?
• Passing? (Merge will not be approved unless this is checked)

Documentation

• swagger documentation updated [required]
• official documentation updated [nice-to-have]

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

• Does it require a specific version of the backend
• which version of the backend is required:

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:

• Refactor shared condition handling to use explicit conditionSettingsKey values and separate dataset vs sample condition configs in state.
• Migrate user settings state from flat columns/filters/conditions and tablesSettings to namespaced externalSettings keys for datasets, proposals, samples, instruments, and files.
• Update NgRx effects and selectors to read/write per-entity table columns, filters, and conditions via externalSettings while preserving defaults and normalization.
• Adjust dashboard/table components for datasets, samples, proposals, instruments, and files to save table column settings using the new externalSettings keys.
• Extend Settings and UserState models plus related unit tests to cover the new externalSettings-based configuration and sample condition configs.

[sourcery-ai]

Hey - I've found 2 issues, and left some high level feedback:

• 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.

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>

---

Sourcery is free for open source - if you like our reviews please consider sharing them ✨

• X
• Mastodon
• LinkedIn
• Facebook

_{Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.}

[sourcery-ai]

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>

---

Sourcery is free for open source - if you like our reviews please consider sharing them ✨

• X
• Mastodon
• LinkedIn
• Facebook

_{Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.}

[Junjiequan]

maybe we can align this with the switch used in ngOnInit for consistency?

[Junjiequan]

Isn’t samples clear enough already? Is there any specific reason to use longer text when the property name is self-explanatory?

[Junjiequan]

Can we make select configs dynamically and remove individual selectors?

Something like this:

export const selectConditions = (type: 'dataset' | 'proposal | ...') =>
  createSelector(selectUserState, (state) => {
    return state.settings[type] || [];
  });

[Junjiequan]

updateConditionsConfigs should be dynamic?

[Junjiequan]

maybe we can align this with the switch for consistency?

[Junjiequan]

The action is called addCustomColumnsAction, but updates only dataset table columns.
Maybe it should accepts params? something like : addCustomColumnsAction({fe_dataset_table_columns:custom columns})

[Junjiequan]

Same as fromActions.addCustomColumnsAction reducer

[Junjiequan]

Same as fromActions.addCustomColumnsAction reducer

[Junjiequan]

Maybe one generic updateConditions reducer which update configs dynamically?

[Junjiequan]

It looks like can be simplified with:

   tableSettings: {
        columns: settings.fe_sample_table_columns,
    },

same in other selectors