Skip to content

Add element not found for deleted generic filter #3767

Open
ghazwarhili wants to merge 29 commits intomainfrom
add-element-not-found-case-of-generic-filter-deleted
Open

Add element not found for deleted generic filter #3767
ghazwarhili wants to merge 29 commits intomainfrom
add-element-not-found-case-of-generic-filter-deleted

Conversation

@ghazwarhili
Copy link
Contributor

@ghazwarhili ghazwarhili commented Feb 27, 2026

PR Summary

Add 'Element Not Found' for deleted generic filter

@ghazwarhili ghazwarhili requested a review from flomillot March 3, 2026 11:12
@ghazwarhili ghazwarhili changed the title add element for filter generic not found Add 'Element Not Found' for deleted generic filter Mar 4, 2026
@ghazwarhili ghazwarhili changed the title Add 'Element Not Found' for deleted generic filter Add element not found for deleted generic filter Mar 4, 2026
@ghazwarhili ghazwarhili requested a review from flomillot March 6, 2026 15:56
@ghazwarhili ghazwarhili requested a review from flomillot March 9, 2026 12:02
@ghazwarhili ghazwarhili requested a review from flomillot March 11, 2026 11:06
Copy link
Contributor

@flomillot flomillot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code OK

@flomillot flomillot self-requested a review March 11, 2026 14:09
Copy link
Contributor

@flomillot flomillot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We don't send the updated filter but the previous version to the server.

Image

@ghazwarhili ghazwarhili requested a review from flomillot March 12, 2026 09:05
Copy link
Contributor

@flomillot flomillot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

edit : mistake

@coderabbitai
Copy link

coderabbitai bot commented Mar 24, 2026

📝 Walkthrough

Walkthrough

Provider now marks missing generic global filters with a deleted flag instead of removing them, propagating that flag through types, provider logic, styles, autocomplete, option labels, chip rendering, and redux action/middleware signatures which now accept optional table metadata.

Changes

Cohort / File(s) Summary
Provider & types
src/components/results/common/global-filter/global-filter-provider.tsx, src/components/results/common/global-filter/global-filter-types.ts
Provider switched from sequential per-UUID async updates to a memoized updateGenericFilter + Promise.all batch. Missing generic filters are marked with deleted: true via addToGlobalFilterOptions(..., tableType, tableUuid). Added optional deleted?: boolean to GlobalFilter.
Presentation & utils
src/components/results/common/global-filter/global-filter-styles.ts, src/components/results/common/global-filter/global-filter-autocomplete.tsx, src/components/results/common/global-filter/global-filter-utils.ts, src/components/results/common/global-filter/selected-global-filters.tsx
getResultsGlobalFiltersChipStyle signature changed to accept a GlobalFilter and applies chipNotFound when element.deleted is true. Autocomplete and middleware exclude deleted options. getOptionLabel returns localized "elementNotFound" for deleted GENERIC_FILTER and SUBSTATION_* types. Chip rendering updated to pass full element.
Redux action & middleware
src/redux/actions.ts, src/redux/globalFiltersMiddleware.ts
AddToGlobalFilterOptionsAction and addToGlobalFilterOptions() now accept optional tableType and tableId. Middleware treats ADD_TO_GLOBAL_FILTER_OPTIONS like other add/remove actions, computes index = tableId ?? tableType, aborts backend sync if studyUuid or index falsy, and excludes deleted entries from payload sent to backend.

Sequence Diagram

sequenceDiagram
    participant Provider as global-filter-provider
    participant Backend as Backend API
    participant Store as Redux Store
    participant UI as Chip Components

    rect rgba(100,150,200,0.5)
    Note over Provider,UI: Batch fetching and marking missing filters deleted
    end

    Provider->>Backend: Fetch generic filter(s) (parallel)
    Backend-->>Provider: 404 Not Found
    Provider->>Store: Dispatch addToGlobalFilterOptions([{...genericFilter, deleted: true}], tableType, tableUuid)
    Store-->>UI: State updated with deleted flag
    UI->>UI: getResultsGlobalFiltersChipStyle(element)
    alt element.deleted == true
        UI->>UI: Apply resultsGlobalFilterStyles.chipNotFound
    else
        UI->>UI: Apply style from FILTER_TYPE_STYLE_MAP
    end
    UI-->>Provider: Render chips with updated styles/labels
Loading

Suggested reviewers

  • Meklo
  • etiennehomer
🚥 Pre-merge checks | ✅ 2 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (2 passed)
Check name Status Explanation
Title check ✅ Passed The title accurately describes the main change: adding support for marking generic filters as 'deleted' and displaying them as 'Element Not Found' in the UI.
Description check ✅ Passed The description is related to the changeset, though brief. It indicates the intent to add 'Element Not Found' for deleted generic filters, which aligns with the code changes.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.


Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@src/redux/globalFiltersMiddleware.ts`:
- Around line 37-47: The switch case handling ADD_GLOBAL_FILTERS /
ADD_TO_GLOBAL_FILTER_OPTIONS / REMOVE_GLOBAL_FILTERS / CLEAR_GLOBAL_FILTERS uses
index = tableId ?? tableType and proceeds to schedule backend syncs and call
setComputationResultGlobalFilters, markEditingGlobalFilter,
unmarkEditingGlobalFilter even when index is undefined; add an early guard in
that switch block (inside the case handling these action types in
globalFiltersMiddleware) that checks if tableType and tableId are both missing
(index == null/undefined) and if so skip the backend sync/marking logic and
simply continue (e.g., return next(action) or break), preventing calls to
setComputationResultGlobalFilters(studyUuid, undefined, ...), creation of
debouncedSyncTimers["undefined"], and
markEditingGlobalFilter/unmarkEditingGlobalFilter with undefined keys.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: fe9636de-2741-4ba9-b9f1-29ab289bdc03

📥 Commits

Reviewing files that changed from the base of the PR and between f7c3f16 and 14e5d22.

📒 Files selected for processing (8)
  • src/components/results/common/global-filter/global-filter-autocomplete.tsx
  • src/components/results/common/global-filter/global-filter-provider.tsx
  • src/components/results/common/global-filter/global-filter-styles.ts
  • src/components/results/common/global-filter/global-filter-types.ts
  • src/components/results/common/global-filter/global-filter-utils.ts
  • src/components/results/common/global-filter/selected-global-filters.tsx
  • src/redux/actions.ts
  • src/redux/globalFiltersMiddleware.ts

@ghazwarhili ghazwarhili requested a review from flomillot March 25, 2026 12:22
const filteredOptions = options
// Allows to find the translated countries (and not their countryCodes) when the user inputs a search value
.filter((option: GlobalFilter) => {
if (option.deleted) return false;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As described in the comment :

// Filter / sort the options 'on the fly' based on the user's search input value and the category he selected (country, voltage level, recent...)

Deleted filters should not appear whatever the input or the category.
So this code should be in the use memo below.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I totally agree
fixed

if (path !== genericFilter.path || label !== genericFilter.label) {
dispatch(
addToGlobalFilterOptions(
[{ ...genericFilter, path: path, label: label ?? genericFilter.label }],
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This code is problematic if label is undefined. I don't remember why we have this line ?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

image

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

because the find + optional chaining (?.)
if find doesn’t locate anything, it returns undefined
So this becomes: undefined?.elementName => label = undefined
fixed by
const label = response.find((parent) => parent.type === ElementType.FILTER)?.elementName ?? genericFilter.label;

@flomillot
Copy link
Contributor

flomillot commented Mar 25, 2026

We don't send the updated filter but the previous version to the server.

Image

This is not fixed btw.
If you manually unselect the deleted filters, then we continue to send it to the server and it's still stored in the database.

@ghazwarhili
Copy link
Contributor Author

We don't send the updated filter but the previous version to the server.
Image

This is not fixed btw. If you manually unselect the deleted filters, then we continue to send it to the server and it's still stored in the database.

Could you resend the image? It’s not displayed, so I can’t understand your test scenario

Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🧹 Nitpick comments (1)
src/components/results/common/global-filter/global-filter-provider.tsx (1)

100-100: Update the comment to reflect current behavior.

The comment says "remove them if they do not exist anymore" but the implementation now marks filters with deleted: true instead of removing them. This allows users to see "element not found" in the UI.

📝 Suggested comment update
-    // Check the selected global filters and remove them if they do not exist anymore.
+    // Check the selected global filters and mark them as deleted if they no longer exist.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@src/components/results/common/global-filter/global-filter-provider.tsx` at
line 100, Update the inline comment that currently reads "remove them if they do
not exist anymore" to reflect the current behavior: instead of removing missing
filters the code marks them with deleted: true so they remain visible in the UI
as "element not found"; change the comment near the filtering logic in
global-filter-provider.tsx (the check that processes selected global filters) to
explicitly state that missing filters are flagged with deleted: true rather than
removed.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Nitpick comments:
In `@src/components/results/common/global-filter/global-filter-provider.tsx`:
- Line 100: Update the inline comment that currently reads "remove them if they
do not exist anymore" to reflect the current behavior: instead of removing
missing filters the code marks them with deleted: true so they remain visible in
the UI as "element not found"; change the comment near the filtering logic in
global-filter-provider.tsx (the check that processes selected global filters) to
explicitly state that missing filters are flagged with deleted: true rather than
removed.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 023b92bc-7271-4dd0-8c50-ae3665d14c50

📥 Commits

Reviewing files that changed from the base of the PR and between c215fb4 and 4770132.

📒 Files selected for processing (2)
  • src/components/results/common/global-filter/global-filter-autocomplete.tsx
  • src/components/results/common/global-filter/global-filter-provider.tsx
🚧 Files skipped from review as they are similar to previous changes (1)
  • src/components/results/common/global-filter/global-filter-autocomplete.tsx

Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)
src/redux/globalFiltersMiddleware.ts (1)

58-70: ⚠️ Potential issue | 🟠 Major

Deleted filters can still be persisted via the selected list.

Line 69 excludes deleted filters only from recentGlobalFilters, but Line 58 still includes deleted entries in selectedGlobalFilters. If a deleted generic filter is still selected, it can still be sent in globalFilters and re-saved server-side.

💡 Proposed fix
-            const selectedGlobalFilters = state.globalFilterOptions.filter((filter) =>
-                selectedFiltersIds.includes(filter.id)
-            );
+            const selectedGlobalFilters = state.globalFilterOptions.filter(
+                (filter) => selectedFiltersIds.includes(filter.id) && !filter.deleted
+            );
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@src/redux/globalFiltersMiddleware.ts` around lines 58 - 70,
selectedGlobalFilters currently includes entries from state.globalFilterOptions
without excluding deleted filters, so deleted global filters can be sent back;
update the selection logic in the selectedGlobalFilters computation (the filter
over state.globalFilterOptions that uses selectedFiltersIds) to also exclude
options where opt.deleted is true (consistent with the recentGlobalFilters
check), ensuring both selectedGlobalFilters and recentGlobalFilters only
contribute non-deleted entries to globalFilters.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Outside diff comments:
In `@src/redux/globalFiltersMiddleware.ts`:
- Around line 58-70: selectedGlobalFilters currently includes entries from
state.globalFilterOptions without excluding deleted filters, so deleted global
filters can be sent back; update the selection logic in the
selectedGlobalFilters computation (the filter over state.globalFilterOptions
that uses selectedFiltersIds) to also exclude options where opt.deleted is true
(consistent with the recentGlobalFilters check), ensuring both
selectedGlobalFilters and recentGlobalFilters only contribute non-deleted
entries to globalFilters.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 7f2ce882-fcb4-4081-a8af-922816065ce5

📥 Commits

Reviewing files that changed from the base of the PR and between 4770132 and 5c141a2.

📒 Files selected for processing (1)
  • src/redux/globalFiltersMiddleware.ts

@ghazwarhili ghazwarhili requested a review from flomillot March 26, 2026 12:18
@sonarqubecloud
Copy link

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants