Skip to content

feat: add checkbox below dropdown view to filter by allowed - #2568

Closed
minottic wants to merge 2 commits into
archive_dropdownfrom
archive_checkbox
Closed

minottic wants to merge 2 commits into
archive_dropdownfrom
archive_checkbox

Conversation

@minottic

@minottic minottic commented Oct 7, 2026 •

Copy link
Copy Markdown
Member

Description

It adds a new checkbox under the list view which can be controlled by the config.json.

The config can control if this is checked by default, who is exempted and what it should do once checked. It accepts v4 GET syntax to add to the view query. The use is, for example, showing the data with a given lifecycle state, e.g. "archivable" true, and then use the checkbox to further filter based on access, e.g. if the user is also the owner of the datasets (and not only the reader). This is useful when users want to see all data which could support a given action and then filter the ones where this action is allowed to them by providing a suitable additional filter

image
  "datasetViews": {
    "label": "Archive status",
    "modes": [
      {
        "id": "all",
        "label": "All",
        "where": {}
      },
      {
        "id": "archivable",
        "label": "Archivable",
        "tooltip": "Datasets that can be sent to tape",
        "where": {
          "datasetlifecycle.archivable": true,
          "datasetlifecycle.retrievable": false
        },
        "checkbox": {
          "label": "Only datasets owned by my groups",
          "default": true,
          "where": {
            "ownerGroup": {
              "$in": "#user.accessGroups"
            }
          },
          "exemptGroups": [
            "admin",
            "archivemanager"
          ]
        }
      },

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

Add optional configurable checkboxes to dataset views so users can narrow results according to their profile and access groups.

New Features:

  • Add configurable checkboxes to dataset view modes for applying optional, user-specific narrowing filters.

Enhancements:

  • Support default checkbox state, exempt user groups, profile placeholders, and state management for checkbox selections.
  • Combine view and checkbox filters safely and clear dataset selections when the active filtering changes.

Documentation:

  • Document checkbox configuration, exemptions, user placeholders, supported query operators, and filtering limitations.

Tests:

  • Add unit and end-to-end coverage for checkbox defaults, query narrowing, exemptions, placeholder resolution, and selection behavior.

@minottic
minottic added this pull request to stack #2567 October 7, 2026 09:01
@minottic
minottic requested a review from a team as a code owner October 7, 2026 09:01

@sourcery-ai sourcery-ai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Hey - I've found 5 issues

Prompt for AI Agents
Please address the comments from this code review:

## Individual Comments

### Comment 1
<location path="src/app/state-management/selectors/datasets.selectors.ts" line_range="270" />
<code_context>
     };

-    return { query, limits };
+    return { query, limits, include: state.viewModeInclude };
   },
 );
</code_context>
<issue_to_address>
**Selected views do not narrow results**

When a configured view or checkbox with a non-empty query is selected, `selectFullqueryParams` removes `filters.mode` but does not add its query to the list request, so the configured view and checkbox conditions never reach the endpoint and the displayed datasets are not narrowed.

Include the `filters.mode` query when building the request query.

Also at `src/app/datasets/dataset-view-modes.utils.ts:61`.
</issue_to_address>

### Comment 2
<location path="src/app/datasets/datasets-filter/datasets-filter.component.ts" line_range="128" />
<code_context>
+  ) {
+    const { query, include } = buildViewModeFilter(
+      viewMode,
+      checked,
+      this.user,
+    );
</code_context>
<issue_to_address>
**Views use an unavailable profile**

When the user selects a view while their profile is unavailable, including while identity loading is pending or after it fails, `setViewMode` passes the unavailable profile to `buildViewModeFilter`, which resolves placeholders to `null` and can fail to recognize exempt users. Updating `this.user` later does not rebuild the active filter, so the list keeps using incorrect query or include values.

Wait for the profile before building the view filter, or rebuild the active filter when the profile becomes available.

Also at `src/app/datasets/datasets-filter/datasets-filter.component.ts:129`, `src/app/datasets/datasets-filter/datasets-filter.component.ts:216-219`, `src/app/datasets/datasets-filter/datasets-filter.component.ts:236-238`, `src/app/datasets/dataset-view-modes.utils.ts:21`.
</issue_to_address>

### Comment 3
<location path="src/app/datasets/dataset-view-modes.utils.ts" line_range="29-31" />
<code_context>
+  user?: ViewModeUser | null,
+): unknown {
+  return cloneDeepWith(query, (value) =>
+    typeof value === "string" && value.startsWith(USER)
+      ? resolvePlaceholder(value, user)
+      : undefined,
+  );
+}
</code_context>
<issue_to_address>
**Literal values are substituted**

When a configured query or include value starts with `#user.` but contains additional literal text, `resolveUserPlaceholders` matches any string with the `#user.` prefix, so it treats the entire value as a profile path and replaces it with a profile value or `null`. The request then uses a different value than the configuration specifies.

Only resolve strings that exactly match the documented placeholder syntax.
</issue_to_address>

### Comment 4
<location path="src/app/datasets/dataset-view-modes.utils.ts" line_range="62-68" />
<code_context>
+  const { query: checkboxQuery, include } = viewMode.checkbox;
+  return {
+    query: { $and: [query, resolveUserPlaceholders(checkboxQuery, user)] },
+    ...(include?.length
+      ? {
+          include: resolveUserPlaceholders(
+            include,
+            user,
+          ) as DatasetViewModeInclude[],
+        }
+      : {}),
+  };
</code_context>
<issue_to_address>
**Related-document includes do not filter datasets**

When a checkbox relies on an include relation scope to identify which datasets match, without a dataset-level query that enforces that relation, `buildViewModeFilter` sends the checkbox `include` separately from its dataset `query`, and the list effect forwards it as an include. A relation scope such as `proposals.where.pi_email` filters the included proposals, not the dataset rows, so datasets without a matching proposal still appear as eligible.

Use a backend-supported relation filter that excludes datasets without a match, or make clear that includes only add related data and cannot implement this checkbox filter.
</issue_to_address>

### Comment 5
<location path="src/app/datasets/dataset-view-modes.utils.ts" line_range="17" />
<code_context>
+
+function resolvePlaceholder(value: string, user?: ViewModeUser | null) {
+  // same syntax as configurable action selectors: #user.<path>
+  const resolved = get(user, value.slice(USER.length));
+  if (resolved !== undefined) return resolved;
+  // most likely a typo in the config; null keeps it visible in the query
</code_context>
<issue_to_address>
**OIDC users get null filters**

When a user logs in through OIDC or functional authentication and selects a view with profile placeholders or exempt groups, `resolvePlaceholder` and `checkboxAppliesTo` read user data only from `selectProfile`, but the OIDC and functional login flows set the user as logged in without dispatching `fetchUserIdentityAction`. Their profile remains null, so placeholders resolve to null and exempt-group checks fail; the checkbox sends the wrong filter and is shown to exempt users.

Populate the profile on these login paths, or resolve placeholders and groups from a user object that is available for every supported login type.

Also at `src/app/datasets/dataset-view-modes.utils.ts:42`.
</issue_to_address>

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

Comment thread src/app/state-management/selectors/datasets.selectors.ts Outdated
Comment thread src/app/datasets/datasets-filter/datasets-filter.component.ts Outdated
Comment thread src/app/datasets/dataset-view-modes.utils.ts Outdated
Comment thread src/app/datasets/dataset-view-modes.utils.ts Outdated
Comment thread src/app/datasets/dataset-view-modes.utils.ts
@minottic

minottic commented Oct 7, 2026

Copy link
Copy Markdown
Member Author

@sourcery-ai review plz

@sourcery-ai sourcery-ai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Hey - I've reviewed your changes and they look great!


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

@minottic
minottic marked this pull request as draft October 20, 2026 12:19
@minottic
minottic removed this pull request from stack #2567 October 21, 2026 08:09
@minottic minottic closed this Oct 21, 2026
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.

1 participant