Repository navigation
Conversation
Contributor
There was a problem hiding this comment.
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>
Member
Author
|
@sourcery-ai review plz |
minottic
force-pushed
the
archive_checkbox
branch
from
October 9, 2026 13:04
8720860 to
60f3eb2
Compare
4 tasks
minottic
force-pushed
the
archive_dropdown
branch
from
October 9, 2026 20:07
8047124 to
23cc5fd
Compare
minottic
force-pushed
the
archive_checkbox
branch
from
October 9, 2026 20:20
60f3eb2 to
f94318d
Compare
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.
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
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
Add optional configurable checkboxes to dataset views so users can narrow results according to their profile and access groups.
New Features:
Enhancements:
Documentation:
Tests: