[NAE-2412] EnumerationMap, MultichoiceMap with 'caseref' component are editable on unassigned task#330
Open
SamuelPalaj wants to merge 5 commits into
Open
[NAE-2412] EnumerationMap, MultichoiceMap with 'caseref' component are editable on unassigned task#330SamuelPalaj wants to merge 5 commits into
SamuelPalaj wants to merge 5 commits into
Conversation
…e editable on unassigned task - added disabled property to task ref component - fixed behavior of task and case list components on unassigned task
…e editable on unassigned task - lint fix
|
Warning Rate limit exceeded
You’ve run out of usage credits. Purchase more in the billing tab. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: Organization UI Review profile: ASSERTIVE Plan: Pro Run ID: 📒 Files selected for processing (3)
WalkthroughAdds a SelectionBehavior enum and showSelection inputs across headers, panels, and lists, introduces selection-mode helpers, reflows templates to pass showSelection, and propagates disabled state (including optional portal-data detection) to disable UI controls and pagination. ChangesSelectionBehavior integration & disabled propagation
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Suggested labels
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. Warning Review ran into problems🔥 ProblemsGit: Failed to clone repository. Please run the 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. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In
`@projects/netgrif-components-core/src/lib/panel/task-panel-list/default-task-panel-list/abstract-default-task-list.component.ts`:
- Around line 106-122: Guard access to this.route and avoid the nested
subscription: only subscribe if this.route is present (e.g. check this.route !=
null), then replace the inner .subscribe on this._tasks$ with a pipe operator
(use switchMap or map + take(1)) so the tasks stream is consumed once (and add
takeUntil(this._unsubscribe$) on the outer chain to prevent leaks). When
resolving the panel ref, store const panelRef =
this._taskPanelRefs.get(this._redirectTaskId) and null-check it before calling
panelRef.open() or setting panelRef.expanded to avoid crashes if the ref isn't
registered yet; keep using this._unsubscribe$ for teardown.
In
`@projects/netgrif-components/src/lib/header/header-modes/edit-mode/edit-mode.component.html`:
- Around line 2-4: The enumeration control (mat-icon) invokes setValue() even
when the component is disabled; update the click handling to respect the
disabled flag so enumeration approvals aren't editable when disabled. Locate the
mat-icon with (click)="setValue();$event.stopPropagation();" and change its
handler to guard on the disabled property (for example: (click)="disabled ?
$event.stopPropagation() : (setValue(); $event.stopPropagation())") or
alternatively restrict rendering with *ngIf="approval && typeApproval ===
'enumeration' && !disabled" so setValue() is never called when disabled.
In
`@projects/netgrif-components/src/lib/navigation/group-navigation-component-resolver/default-components/refs/default-task-view/default-task-view.component.ts`:
- Around line 60-62: The disabled() method currently returns a possibly
undefined value due to optional chaining; update it so it always returns a
boolean by coercing the optional expression to boolean (e.g. using !! or
Boolean(...) or the nullish coalescing operator with false) when reading
this._dataFieldPortalData?.dataField?.formControlRef.disabled; keep the same
method name disabled and the same properties (_dataFieldPortalData, dataField,
formControlRef) but ensure the final returned value is a boolean.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: e510ad99-2e9f-49c2-a2ae-98609372171b
📒 Files selected for processing (19)
projects/netgrif-components-core/src/lib/header/abstract-header.component.tsprojects/netgrif-components-core/src/lib/header/header-modes/abstract-header-mode.component.tsprojects/netgrif-components-core/src/lib/panel/case-panel/abstract-case-panel.component.tsprojects/netgrif-components-core/src/lib/panel/task-panel-list/default-task-panel-list/abstract-default-task-list.component.tsprojects/netgrif-components-core/src/lib/panel/task-panel/abstract-task-panel.component.tsprojects/netgrif-components-core/src/lib/view/case-view/components/case-list-paginator/abstract-case-list-paginator.component.tsprojects/netgrif-components-core/src/lib/view/case-view/components/default-case-list/abstract-default-case-list.component.tsprojects/netgrif-components/src/lib/header/header-modes/edit-mode/edit-mode.component.htmlprojects/netgrif-components/src/lib/header/header-modes/search-mode/search-mode.component.htmlprojects/netgrif-components/src/lib/header/header-modes/sort-mode/sort-mode.component.htmlprojects/netgrif-components/src/lib/header/header.component.htmlprojects/netgrif-components/src/lib/navigation/group-navigation-component-resolver/default-components/refs/default-case-ref-list-view/default-case-ref-list-view.component.htmlprojects/netgrif-components/src/lib/navigation/group-navigation-component-resolver/default-components/refs/default-task-view/default-task-view.component.htmlprojects/netgrif-components/src/lib/navigation/group-navigation-component-resolver/default-components/refs/default-task-view/default-task-view.component.tsprojects/netgrif-components/src/lib/panel/case-panel/case-panel.component.htmlprojects/netgrif-components/src/lib/panel/task-panel-list-pagination/task-list-pagination.component.htmlprojects/netgrif-components/src/lib/panel/task-panel/task-panel.component.htmlprojects/netgrif-components/src/lib/view/case-view/components/case-list-paginator/case-list-paginator.component.htmlprojects/netgrif-components/src/lib/view/case-view/components/case-list/case-list.component.html
💤 Files with no reviewable changes (1)
- projects/netgrif-components-core/src/lib/view/case-view/components/case-list-paginator/abstract-case-list-paginator.component.ts
Kovy95
reviewed
May 20, 2026
Contributor
There was a problem hiding this comment.
can we rename all "approval" properties to "showSelection"? it is more intuitive (my bad naming from previous implementation)
change type boolean to ENUM with 3 values - Editable, Visible, Hidden, default value hidden
Kovy95
reviewed
May 20, 2026
| @Input() showCasePanelIcon = true; | ||
| @Input() showDeleteMenu = false; | ||
| @Input() textEllipsis = false; | ||
| @Input() public disabled: boolean = false; |
Contributor
There was a problem hiding this comment.
this is replaced with enu, in previous comment
Kovy95
reviewed
May 20, 2026
| @Input() public indeterminate: boolean; | ||
| @Input() public approvalFormControl: FormControl; | ||
| @Input() public typeApproval: string; | ||
| @Input() public disabled: boolean = false; |
Contributor
There was a problem hiding this comment.
rename approval and rework to enum, and remove this disabled property
Kovy95
reviewed
May 20, 2026
| @Input() showSearchButton = true; | ||
| @Input() showTableSection = true; | ||
| @Input() public approval: boolean; | ||
| @Input() public disabled: boolean = false; |
Contributor
There was a problem hiding this comment.
rename approval and rework to enum, and remove this disabled property
Kovy95
reviewed
May 20, 2026
| <div fxLayout="row" fxFlex="100" fxLayoutAlign="start center" class="netgrif-input netgrif-header-input netgrif-input-fix netgrif-zero-field-wrapper"> | ||
| <mat-checkbox *ngIf="approval && typeApproval === 'multichoice'" [formControl]="approvalFormControl" [indeterminate]="indeterminate" | ||
| (click)="$event.stopPropagation();" color='primary' class="checkbox-padding"></mat-checkbox> | ||
| (click)="$event.stopPropagation();" color='primary' class="checkbox-padding" [disabled]="disabled"></mat-checkbox> |
Contributor
There was a problem hiding this comment.
turn of the ripple if you change this to button
…e editable on unassigned task - selection-behavior.ts enum added - unification of disabled and approval flags into showSelection in case-list component of caseRef field
There was a problem hiding this comment.
Actionable comments posted: 3
♻️ Duplicate comments (1)
projects/netgrif-components-core/src/lib/panel/task-panel-list/default-task-panel-list/abstract-default-task-list.component.ts (1)
105-109:⚠️ Potential issue | 🟠 Major | ⚡ Quick win
takeUntilnotifier usage still leaves redirect subscriptions hanging.The nested
_tasks$subscription is still open-ended, andtakeUntil(this._unsubscribe$)won’t fire on destroy if the notifier is only completed (nonext). This can keep subscriptions alive longer than intended.Suggested fix
-import {filter, takeUntil} from 'rxjs/operators'; +import {filter, take, takeUntil} from 'rxjs/operators'; ... - this._tasks$.pipe(takeUntil(this._unsubscribe$)).subscribe(tasks => { + this._tasks$.pipe(take(1), takeUntil(this._unsubscribe$)).subscribe(tasks => { const task = tasks.find(t => t.task.stringId === this._redirectTaskId); const panelRef = this._taskPanelRefs.get(this._redirectTaskId); if (!!task && !task.initiallyExpanded && !!panelRef) { panelRef.open(); panelRef.expanded = true; this._unsubscribe$.next(); } });ngOnDestroy(): void { super.ngOnDestroy(); this.taskEvent.complete(); + this._unsubscribe$.next(); this._unsubscribe$.complete(); if (this._unsub) { this._unsub.unsubscribe(); } }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@projects/netgrif-components-core/src/lib/panel/task-panel-list/default-task-panel-list/abstract-default-task-list.component.ts` around lines 105 - 109, The nested subscription to this._tasks$ inside the pm subscription can leak because takeUntil(this._unsubscribe$) relies on notifier emission and the inner subscribe is started per pm; replace the nested subscribe with a composed operator chain: when piping from the outer observable (the one emitting pm with taskId) use switchMap or exhaustMap to switch into this._tasks$ (or use withLatestFrom(this._tasks$) or take(1) on this._tasks$) so each pm emission maps to a single inner value and auto-unsubscribes; update references to _redirectTaskId, pm, and this._tasks$ accordingly and remove the inner .subscribe call so takeUntil(this._unsubscribe$) is applied to the whole chain.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In
`@projects/netgrif-components-core/src/lib/panel/configuration/selection-behavior.ts`:
- Around line 1-5: Replace the numeric enum with a string-based enum for
SelectionBehavior so values serialize and log as readable names; update the enum
declaration SelectionBehavior { EDITABLE = "EDITABLE", VISIBLE = "VISIBLE",
HIDDEN = "HIDDEN" } and then adjust any places that compare or persist these
values (references to SelectionBehavior, usages in templates, inputs,
serialization/deserialization code) to expect and handle string values instead
of numeric indexes to avoid reverse-mapping surprises.
In
`@projects/netgrif-components/src/lib/header/header-modes/search-mode/search-mode.component.html`:
- Around line 4-6: The icon-only button rendered when isInSelectionMode() &&
typeApproval === 'enumeration' lacks an accessible name; update the <button> in
the search-mode template (the one that calls setValue() and stops propagation)
to include an appropriate aria-label (or aria-label plus matTooltip) such as
"Clear selection" or a translation key so screen readers can announce the
action; ensure you place the aria-label on the same element that has
(click)="setValue(); $event.stopPropagation();" and keep existing attributes
like [disabled] and [disableRipple].
In
`@projects/netgrif-components/src/lib/view/case-view/components/case-list-paginator/case-list-paginator.component.html`:
- Line 31: The paginator is being disabled by the broad isSelectionDisabled()
call in the mat-paginator binding which blocks normal navigation even when
selection UI is hidden; change the template to bind [disabled] to a narrower
predicate (e.g., isPaginationDisabled()) and implement that method in the
CaseListPaginatorComponent so it only returns true when selection UI is visible
AND not editable (for example check selectionVisible/selectionHidden flag plus
the existing editable check or accept a parameter to isSelectionDisabled).
Update the component class (CaseListPaginatorComponent) to add the new helper
method (or overload isSelectionDisabled with a context flag) and use its name in
the template so pagination remains enabled for hidden-selection scenarios.
---
Duplicate comments:
In
`@projects/netgrif-components-core/src/lib/panel/task-panel-list/default-task-panel-list/abstract-default-task-list.component.ts`:
- Around line 105-109: The nested subscription to this._tasks$ inside the pm
subscription can leak because takeUntil(this._unsubscribe$) relies on notifier
emission and the inner subscribe is started per pm; replace the nested subscribe
with a composed operator chain: when piping from the outer observable (the one
emitting pm with taskId) use switchMap or exhaustMap to switch into this._tasks$
(or use withLatestFrom(this._tasks$) or take(1) on this._tasks$) so each pm
emission maps to a single inner value and auto-unsubscribes; update references
to _redirectTaskId, pm, and this._tasks$ accordingly and remove the inner
.subscribe call so takeUntil(this._unsubscribe$) is applied to the whole chain.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: c7f125f7-90ff-4eca-841d-8fca24bfabcf
📒 Files selected for processing (18)
projects/netgrif-components-core/src/lib/header/abstract-header.component.tsprojects/netgrif-components-core/src/lib/header/header-modes/abstract-header-mode.component.tsprojects/netgrif-components-core/src/lib/panel/case-panel/abstract-case-panel.component.tsprojects/netgrif-components-core/src/lib/panel/configuration/selection-behavior.tsprojects/netgrif-components-core/src/lib/panel/public-api.tsprojects/netgrif-components-core/src/lib/panel/task-panel-list/default-task-panel-list/abstract-default-task-list.component.tsprojects/netgrif-components-core/src/lib/view/case-view/components/case-list-paginator/abstract-case-list-paginator.component.tsprojects/netgrif-components-core/src/lib/view/case-view/components/default-case-list/abstract-default-case-list.component.tsprojects/netgrif-components/src/lib/header/header-modes/edit-mode/edit-mode.component.htmlprojects/netgrif-components/src/lib/header/header-modes/search-mode/search-mode.component.htmlprojects/netgrif-components/src/lib/header/header-modes/sort-mode/sort-mode.component.htmlprojects/netgrif-components/src/lib/header/header.component.htmlprojects/netgrif-components/src/lib/navigation/group-navigation-component-resolver/default-components/refs/default-case-ref-list-view/default-case-ref-list-view.component.htmlprojects/netgrif-components/src/lib/navigation/group-navigation-component-resolver/default-components/refs/default-case-ref-list-view/default-case-ref-list-view.component.tsprojects/netgrif-components/src/lib/navigation/group-navigation-component-resolver/default-components/refs/default-task-view/default-task-view.component.tsprojects/netgrif-components/src/lib/panel/case-panel/case-panel.component.htmlprojects/netgrif-components/src/lib/view/case-view/components/case-list-paginator/case-list-paginator.component.htmlprojects/netgrif-components/src/lib/view/case-view/components/case-list/case-list.component.html
💤 Files with no reviewable changes (1)
- projects/netgrif-components-core/src/lib/view/case-view/components/case-list-paginator/abstract-case-list-paginator.component.ts
Comment on lines
+1
to
+5
| export enum SelectionBehavior { | ||
| EDITABLE, | ||
| VISIBLE, | ||
| HIDDEN | ||
| } |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial | 💤 Low value
Consider using string enum values.
Numeric enums reverse-map and serialize as integers, which makes logs and any persisted/config representation harder to read (e.g., 0 vs "EDITABLE"). Since this enum is part of the public API and likely to appear in template-facing inputs, string values are easier to debug and safer to refactor (no accidental positional dependency).
♻️ Proposed change
export enum SelectionBehavior {
- EDITABLE,
- VISIBLE,
- HIDDEN
+ EDITABLE = 'EDITABLE',
+ VISIBLE = 'VISIBLE',
+ HIDDEN = 'HIDDEN'
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| export enum SelectionBehavior { | |
| EDITABLE, | |
| VISIBLE, | |
| HIDDEN | |
| } | |
| export enum SelectionBehavior { | |
| EDITABLE = 'EDITABLE', | |
| VISIBLE = 'VISIBLE', | |
| HIDDEN = 'HIDDEN' | |
| } |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In
`@projects/netgrif-components-core/src/lib/panel/configuration/selection-behavior.ts`
around lines 1 - 5, Replace the numeric enum with a string-based enum for
SelectionBehavior so values serialize and log as readable names; update the enum
declaration SelectionBehavior { EDITABLE = "EDITABLE", VISIBLE = "VISIBLE",
HIDDEN = "HIDDEN" } and then adjust any places that compare or persist these
values (references to SelectionBehavior, usages in templates, inputs,
serialization/deserialization code) to expect and handle string values instead
of numeric indexes to avoid reverse-mapping surprises.
| </div> | ||
| </mat-accordion> | ||
| <mat-paginator [length]="length" [pageSize]="pageSize" [pageIndex]="pageIndex" color="primary" [disabled]="disabled" | ||
| <mat-paginator [length]="length" [pageSize]="pageSize" [pageIndex]="pageIndex" color="primary" [disabled]="isSelectionDisabled()" |
There was a problem hiding this comment.
Paginator disable condition is too broad and can block normal navigation.
Line 31 disables paging whenever selection is not editable. That also affects hidden-selection scenarios, so regular case-list pagination can become unintentionally disabled.
Suggested fix
- <mat-paginator [length]="length" [pageSize]="pageSize" [pageIndex]="pageIndex" color="primary" [disabled]="isSelectionDisabled()"
+ <mat-paginator [length]="length" [pageSize]="pageSize" [pageIndex]="pageIndex" color="primary"
+ [disabled]="isInSelectionMode() && isSelectionDisabled()"
[pageSizeOptions]="pageSizeOptions" (page)="onPageChanged($event)" showFirstLastButtons>🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In
`@projects/netgrif-components/src/lib/view/case-view/components/case-list-paginator/case-list-paginator.component.html`
at line 31, The paginator is being disabled by the broad isSelectionDisabled()
call in the mat-paginator binding which blocks normal navigation even when
selection UI is hidden; change the template to bind [disabled] to a narrower
predicate (e.g., isPaginationDisabled()) and implement that method in the
CaseListPaginatorComponent so it only returns true when selection UI is visible
AND not editable (for example check selectionVisible/selectionHidden flag plus
the existing editable check or accept a parameter to isSelectionDisabled).
Update the component class (CaseListPaginatorComponent) to add the new helper
method (or overload isSelectionDisabled with a context flag) and use its name in
the template so pagination remains enabled for hidden-selection scenarios.
…e editable on unassigned task - aria-label added to clear selection buttons
|
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
Fixes NAE-2412
How Has Been This Tested?
manualy
Test Configuration
Checklist:
Summary by CodeRabbit
New Features
Bug Fixes