Conversation
- implement exporting cases from case view - add export service - add attribute allow export to default tab view and default tabbed case view - add export button to default tabbed case view
- resolve comments - implement onDestroy - added translation - fix import
- resolve comments regarding translations
- fix test - update url for request
- add tests for export service
- update version - add changelog
NAE-2100 - Case view export button as NAE feature
…ut error message - fix reloading a non-existing task
- flatten the array with values of taskrefs
[NAE-2268] Fix resolving of referenced taskId on frontend
[NAE-2231] Unable to change behavior of taskRef on finish event witho…
…vent outcome - change forEach to for
# Conflicts: # projects/netgrif-components-core/src/lib/task-content/services/task-content.service.ts
[NAE-2290] Couldn't find referenced task in a taskref incoming from event outcome
- fix problem with fields while switching the tabs, add block fields to get data event
- add a check of property validateData on setData event from frontend - if you want to validate all datafields, put property validateData on validating datafield
- fix according to PR
[NAE-2394] - Fields looks editable when switching tabs
[NAE-2393] Validation property on set data event
- increment version and changelog
[NAE-2101] release 6.4.2
WalkthroughRelease 6.4.2 introduces a case view export feature with filtered case data download, adds task execution state tracking via Changes
Sequence DiagramsequenceDiagram
actor User
participant CaseViewComponent as Case View Component
participant ExportService as ExportService
participant HttpClient as HttpClient
participant Backend as Backend Server
participant BrowserAPI as Browser API
User->>CaseViewComponent: Click export button
CaseViewComponent->>CaseViewComponent: Set loading = true
CaseViewComponent->>ExportService: downloadExcelFromCurrentSelection(filter, headers)
ExportService->>HttpClient: POST /export/filteredCases<br/>(filter, headers)
HttpClient->>Backend: Send request with payload
Backend->>Backend: Generate Excel file
Backend->>HttpClient: Return arraybuffer response
HttpClient->>ExportService: Receive blob data
ExportService->>BrowserAPI: Create blob URL
ExportService->>BrowserAPI: Create temporary anchor element
BrowserAPI->>BrowserAPI: Trigger download<br/>(export.xlsx)
ExportService->>ExportService: Clean up temporary element
ExportService->>CaseViewComponent: Return Observable<boolean> true
CaseViewComponent->>CaseViewComponent: Set loading = false
CaseViewComponent->>User: Download initiated
alt No response body
Backend-->>HttpClient: Empty/null response
HttpClient-->>ExportService: Handle error
ExportService-->>CaseViewComponent: Return Observable<boolean> false
CaseViewComponent->>CaseViewComponent: Set loading = false
CaseViewComponent->>CaseViewComponent: Show error snackbar
CaseViewComponent->>User: Display error message
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Suggested labels
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ 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. Comment |
| @@ -29,7 +29,7 @@ | |||
| "nae frontend" | |||
| ], | |||
| "peerDependencies": { | |||
| "@netgrif/components-core": "6.4.1", | |||
| "@netgrif/components-core": "6.4.2-rc.6", | |||
There was a problem hiding this comment.
Actionable comments posted: 16
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@projects/netgrif-components-core/src/assets/i18n/sk.json`:
- Around line 517-519: The key "export.errorExportDownload" currently contains
English text; replace its value with the Slovak translation (e.g., "Súbor sa
nepodarilo stiahnuť!") so the locale is consistent with other messages like
"Súbor sa nepodarilo nahrať"; update the JSON entry for
export.errorExportDownload accordingly.
In
`@projects/netgrif-components-core/src/lib/data-fields/file-list-field/file-list-default-field/abstract-file-list-default-field.component.ts`:
- Around line 158-160: The early return in the validation gate (the if checking
this.dataField.component?.properties?.validateData === 'true' &&
this._taskContentService && !this._taskContentService.validateTaskData()) exits
without clearing the file input, so selecting the same file later won't
retrigger; before returning call the component's file-reset logic (e.g., clear
the input element or invoke an existing resetFileInput/clearFileSelection
method) to set the <input type="file"> value back to empty (for example via
this.fileInput.nativeElement.value = '' or this.resetFileInput()), then return.
Ensure you reference the same symbols (this.dataField, this._taskContentService,
validateTaskData()) when adding the reset call so the change is colocated with
the existing check.
In
`@projects/netgrif-components-core/src/lib/export/services/export.service.spec.ts`:
- Around line 65-88: The test for downloadExcelFromCurrentSelection sets spies
on document.body.appendChild and removeChild but never asserts they were called;
update the spec to assert that document.body.appendChild was called (once) and
that document.body.removeChild was called (once) after flushing the mock
response, and optionally assert the appendChild call received an
HTMLAnchorElement (the download link) to ensure the DOM download trigger was
executed; keep these assertions within the subscribe callback (alongside
expect(result).toBeTrue()) so they run after the HTTP flush.
In `@projects/netgrif-components-core/src/lib/export/services/export.service.ts`:
- Around line 41-49: The created object URL (urlBlob from
window.URL.createObjectURL(blob)) is never revoked, causing a memory leak;
update the export method in export.service.ts (the block creating linkElement,
blob, urlBlob, setting href/download and clicking) to revoke the URL after
use—e.g., call URL.revokeObjectURL(urlBlob) once the download is initiated
(onlink click handler, link.onload, or in a finally block after click) and
ensure removal of linkElement still occurs; ensure revocation happens even on
errors so repeated exports don't accumulate memory.
- Around line 26-28: In downloadExcelFromCurrentSelection replace the fragile
private-field access (activeFilter as any)._operator with the public getter on
MergedFilter: use (activeFilter as MergedFilter).operator (or guard-cast and
read operator) to obtain the MergeOperator; update the mergeOperation assignment
to use that getter so you rely on the MergedFilter.operator API rather than the
private _operator field.
- Around line 56-70: The getResourceAddress function uses an untyped
resourcesArray (any); define a Resource interface (e.g., interface Resource {
name: string; address: string }) and change the function signature to accept
Resource | Resource[] (or Resource[] | Resource) instead of any; update the type
guard to use Array.isArray(resourcesArray) and iterate over Resource items
(resource.name/resource.address) so TypeScript can enforce property types and
catch errors at compile time.
In
`@projects/netgrif-components-core/src/lib/navigation/model/group-navigation-constants.ts`:
- Around line 98-100: Update the docstring above the boolean constant in
group-navigation-constants.ts that currently mentions "table mode" to accurately
describe that this flag controls case export visibility/permission (i.e.,
whether case export is shown/enabled); locate the boolean constant near the
existing comment and replace the text so it clearly states it governs export
permission/visibility for cases.
In
`@projects/netgrif-components-core/src/lib/panel/task-panel/abstract-task-panel.component.ts`:
- Around line 200-205: The null-pointer risk is that the current guard uses
`this._taskContentService.task.user === undefined`, which misses `null` and can
make `_userComparator.compareUsers(this._taskContentService.task.user)` throw;
update the predicate passed into `_taskDataService.initializeTaskDataFields(...,
true)` to treat `task.user` as falsy (e.g. use
`!this._taskContentService.task.user`) so the `taskShouldBeBlocked` calculation
checks for missing user correctly before calling `_userComparator.compareUsers`,
then keep calling `_taskContentService.blockFields(taskShouldBeBlocked)` as
before.
- Around line 200-205: The tabSelected callback calls
this._taskContentService.blockFields using only user-assignment logic, which can
overwrite the permission-based blocking applied in afterExpand; update the
tabSelected invocation (the call inside
this._taskDataService.initializeTaskDataFields(this._callChain.create(() => {
... })) ) to combine both conditions so blockFields is called with
(!this._userComparator.compareUsers(this._taskContentService.task.user) ||
this.hasNoFinishPermission()), ensuring fields stay blocked if either the
user-assignment check or hasNoFinishPermission() indicates blocking; keep the
call site and method names (_taskDataService.initializeTaskDataFields,
_callChain.create, _taskContentService.blockFields,
_userComparator.compareUsers, hasNoFinishPermission) but change the boolean
expression passed to blockFields accordingly.
In
`@projects/netgrif-components-core/src/lib/task-content/services/task-content.service.ts`:
- Around line 313-325: In findTaskRefId, accessing
this.taskFieldsIndex[id].fields can throw if taskFieldsIndex[id] is undefined;
change the loop over referencedTaskIds to first check that
this.taskFieldsIndex[id] exists (and has a .fields property) before recursing,
skipping or continuing when missing so you don't attempt to access .fields of
undefined; ensure taskRefId behavior remains the same (return when found) and
keep references to findTaskRefId, taskFieldsIndex, referencedTaskIds, and
taskRefId to locate the change.
In `@projects/netgrif-components/package.json`:
- Line 32: The peer dependency in package.json currently targets the
release-candidate tag "@netgrif/components-core": "6.4.2-rc.6"; update this
entry to the published stable version "6.4.2" so the peer dependency matches the
released core package and avoids npm resolution warnings—locate the package.json
dependency line referencing "@netgrif/components-core" and replace "6.4.2-rc.6"
with "6.4.2".
In
`@projects/netgrif-components/src/lib/navigation/group-navigation-component-resolver/default-components/default-tab-view/default-tab-view.component.ts`:
- Line 86: The code incorrectly extracts caseViewAllowExport as a boolean[] and
doesn't handle missing fields; update the extraction to expect a boolean and
make it robust by wrapping
extractFieldValueFromData(this._navigationItemTaskData,
GroupNavigationConstants.ITEM_FIELD_ID_CASE_ALLOW_EXPORT) in a safe helper
(e.g., extractOptionalBooleanField) that catches exceptions, returns a default
(false) when the field is absent or null, and uses the boolean generic type;
replace usages of the old extraction (caseViewAllowExport) with this helper to
avoid runtime throws during rendering.
In
`@projects/netgrif-components/src/lib/navigation/group-navigation-component-resolver/default-components/default-tabbed-case-view/default-tabbed-case-view.component.html`:
- Line 13: The *ngIf condition on the mat-icon uses an unnecessary parenthesized
call and explicit comparison; replace the expression on the mat-icon element so
it uses a direct boolean negation of isLoading() (i.e., use !isLoading()) to
simplify the check and keep the element selector (mat-icon with class
"net-upload") unchanged.
- Around line 10-16: The button in default-tabbed-case-view.component.html that
triggers export() and uses [disabled]="isLoading()" is missing an explicit type,
causing it to default to submit in forms; update the <button> element (the one
with class "export-mat-mini-fab" and (click)="export()") to include
type="button" to prevent unintended form submissions while keeping the existing
click and disabled behavior.
In
`@projects/netgrif-components/src/lib/navigation/group-navigation-component-resolver/default-components/default-tabbed-case-view/default-tabbed-case-view.component.ts`:
- Around line 236-238: ngOnDestroy currently only completes loading$ but leaves
the _headersSub subscription open and doesn't invoke parent cleanup; update the
component's ngOnDestroy to unsubscribe or teardown _headersSub (call
_headersSub.unsubscribe() or use a safe null-check/cleanup routine) and then
call super.ngOnDestroy() after performing local cleanup so parent classes (e.g.,
AbstractCaseViewComponent/AbstractViewWithHeadersComponent) run their teardown;
ensure loading$.complete() is still called and the order is: clean local
subscriptions (unsubscribe _headersSub), complete loading$, then call
super.ngOnDestroy().
- Around line 222-234: The export() method starts a subscription to
_exportService.downloadExcelFromCurrentSelection but doesn't unsubscribe, so
callbacks may run after the component is destroyed; update export() to pipe the
observable through a teardown operator (e.g., takeUntil(this._destroyed$)) or
add the subscription to a class-level Subscription and unsubscribe in
ngOnDestroy; specifically, create a private _destroyed$ = new Subject<void>()
(or a private _subscriptions: Subscription) on the component, change
_exportService.downloadExcelFromCurrentSelection(...).subscribe(...) to use
.pipe(takeUntil(this._destroyed$)).subscribe(...) (or add the returned
Subscription to _subscriptions), and ensure ngOnDestroy calls
this._destroyed$.next(); this._destroyed$.complete(); (or
this._subscriptions.unsubscribe()) so callbacks that touch loading$, _snackbar,
or _loggerService won't run after destruction.
🪄 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: 582036dc-cc59-4bbc-a285-0a850134cdee
📒 Files selected for processing (33)
CHANGELOG.mdpackage.jsonprojects/netgrif-components-core/package.jsonprojects/netgrif-components-core/src/assets/i18n/de.jsonprojects/netgrif-components-core/src/assets/i18n/en.jsonprojects/netgrif-components-core/src/assets/i18n/sk.jsonprojects/netgrif-components-core/src/lib/data-fields/file-field/file-default-field/abstract-file-default-field.component.spec.tsprojects/netgrif-components-core/src/lib/data-fields/file-field/file-default-field/abstract-file-default-field.component.tsprojects/netgrif-components-core/src/lib/data-fields/file-list-field/file-list-default-field/abstract-file-list-default-field.component.spec.tsprojects/netgrif-components-core/src/lib/data-fields/file-list-field/file-list-default-field/abstract-file-list-default-field.component.tsprojects/netgrif-components-core/src/lib/event/model/event-outcomes/task-outcomes/finish-task-event-outcome.tsprojects/netgrif-components-core/src/lib/export/public-api.tsprojects/netgrif-components-core/src/lib/export/services/export.service.spec.tsprojects/netgrif-components-core/src/lib/export/services/export.service.tsprojects/netgrif-components-core/src/lib/navigation/model/group-navigation-constants.tsprojects/netgrif-components-core/src/lib/panel/task-panel/abstract-task-panel.component.spec.tsprojects/netgrif-components-core/src/lib/panel/task-panel/abstract-task-panel.component.tsprojects/netgrif-components-core/src/lib/resources/interface/task.tsprojects/netgrif-components-core/src/lib/task-content/services/task-content.service.tsprojects/netgrif-components-core/src/lib/task/services/finish-task.service.tsprojects/netgrif-components-core/src/lib/task/services/task-data.service.tsprojects/netgrif-components-core/src/public-api.tsprojects/netgrif-components/package.jsonprojects/netgrif-components/src/lib/data-fields/file-field/file-default-field/file-default-field.component.tsprojects/netgrif-components/src/lib/data-fields/file-list-field/file-list-default-field/file-list-default-field.component.tsprojects/netgrif-components/src/lib/navigation/group-navigation-component-resolver/default-components/default-tab-view/default-tab-view.component.spec.tsprojects/netgrif-components/src/lib/navigation/group-navigation-component-resolver/default-components/default-tab-view/default-tab-view.component.tsprojects/netgrif-components/src/lib/navigation/group-navigation-component-resolver/default-components/default-tabbed-case-view/default-tabbed-case-view.component.htmlprojects/netgrif-components/src/lib/navigation/group-navigation-component-resolver/default-components/default-tabbed-case-view/default-tabbed-case-view.component.scssprojects/netgrif-components/src/lib/navigation/group-navigation-component-resolver/default-components/default-tabbed-case-view/default-tabbed-case-view.component.spec.tsprojects/netgrif-components/src/lib/navigation/group-navigation-component-resolver/default-components/default-tabbed-case-view/default-tabbed-case-view.component.tsprojects/netgrif-components/src/lib/navigation/group-navigation-component-resolver/default-components/model/injected-tabbed-case-view-data-with-navigation-item-task-data.tsprojects/netgrif-components/src/lib/panel/task-panel/task-panel.component.ts
| "export": { | ||
| "errorExportDownload": "File failed to download!" | ||
| } |
There was a problem hiding this comment.
Missing Slovak translation - English text in Slovak locale file.
The export.errorExportDownload value is in English ("File failed to download!") instead of Slovak. For consistency with other error messages in this file (e.g., "Súbor sa nepodarilo nahrať" at line 254), this should be translated.
🌐 Proposed fix for Slovak translation
"export": {
- "errorExportDownload": "File failed to download!"
+ "errorExportDownload": "Súbor sa nepodarilo stiahnuť!"
}📝 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.
| "export": { | |
| "errorExportDownload": "File failed to download!" | |
| } | |
| "export": { | |
| "errorExportDownload": "Súbor sa nepodarilo stiahnuť!" | |
| } |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@projects/netgrif-components-core/src/assets/i18n/sk.json` around lines 517 -
519, The key "export.errorExportDownload" currently contains English text;
replace its value with the Slovak translation (e.g., "Súbor sa nepodarilo
stiahnuť!") so the locale is consistent with other messages like "Súbor sa
nepodarilo nahrať"; update the JSON entry for export.errorExportDownload
accordingly.
| if (this.dataField.component?.properties?.validateData === 'true' && this._taskContentService && !this._taskContentService.validateTaskData()) { | ||
| return; | ||
| } |
There was a problem hiding this comment.
Reset file input before returning from validation gate.
At Line 158, early return leaves <input type="file"> unchanged. After fixing invalid fields, selecting the same file may not retrigger upload.
Proposed fix
- if (this.dataField.component?.properties?.validateData === 'true' && this._taskContentService && !this._taskContentService.validateTaskData()) {
- return;
- }
+ if (this.dataField.component?.properties?.validateData === 'true' && this._taskContentService && !this._taskContentService.validateTaskData()) {
+ this.fileUploadEl.nativeElement.value = '';
+ return;
+ }📝 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.
| if (this.dataField.component?.properties?.validateData === 'true' && this._taskContentService && !this._taskContentService.validateTaskData()) { | |
| return; | |
| } | |
| if (this.dataField.component?.properties?.validateData === 'true' && this._taskContentService && !this._taskContentService.validateTaskData()) { | |
| this.fileUploadEl.nativeElement.value = ''; | |
| return; | |
| } |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@projects/netgrif-components-core/src/lib/data-fields/file-list-field/file-list-default-field/abstract-file-list-default-field.component.ts`
around lines 158 - 160, The early return in the validation gate (the if checking
this.dataField.component?.properties?.validateData === 'true' &&
this._taskContentService && !this._taskContentService.validateTaskData()) exits
without clearing the file input, so selecting the same file later won't
retrigger; before returning call the component's file-reset logic (e.g., clear
the input element or invoke an existing resetFileInput/clearFileSelection
method) to set the <input type="file"> value back to empty (for example via
this.fileInput.nativeElement.value = '' or this.resetFileInput()), then return.
Ensure you reference the same symbols (this.dataField, this._taskContentService,
validateTaskData()) when adding the reset call so the change is colocated with
the existing check.
| it('should return true and trigger file download on valid response', (done) => { | ||
| spyOn(document.body, 'appendChild'); | ||
| spyOn(document.body, 'removeChild'); | ||
|
|
||
| const mockFilter: Filter = { | ||
| getRequestBody: () => ({ some: 'query' }) | ||
| } as any; | ||
|
|
||
| const headers: HeaderColumn[] = [ | ||
| new HeaderColumn(HeaderColumnType.IMMEDIATE, 'name', 'Name', 'string', true, 'net-id'), | ||
| new HeaderColumn(HeaderColumnType.META, 'date', 'Date', 'date') | ||
| ]; | ||
|
|
||
| service.downloadExcelFromCurrentSelection(mockFilter, headers).subscribe((result) => { | ||
| expect(result).toBeTrue(); | ||
| done(); | ||
| }); | ||
|
|
||
| const req = httpMock.expectOne('http://mock-api/export/filteredCases'); | ||
| expect(req.request.method).toBe('POST'); | ||
| req.flush(new ArrayBuffer(10), { | ||
| headers: { 'Content-Type': 'application/vnd.openxmlformats-officedocument.spreadsheetml.sheet' } | ||
| }); | ||
| }); |
There was a problem hiding this comment.
Missing assertions for DOM manipulation spies.
The spies on document.body.appendChild and removeChild are created but never asserted. This means the test doesn't actually verify that the download trigger mechanism works correctly.
Proposed fix to add spy assertions
service.downloadExcelFromCurrentSelection(mockFilter, headers).subscribe((result) => {
expect(result).toBeTrue();
+ expect(document.body.appendChild).toHaveBeenCalled();
+ expect(document.body.removeChild).toHaveBeenCalled();
done();
});📝 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.
| it('should return true and trigger file download on valid response', (done) => { | |
| spyOn(document.body, 'appendChild'); | |
| spyOn(document.body, 'removeChild'); | |
| const mockFilter: Filter = { | |
| getRequestBody: () => ({ some: 'query' }) | |
| } as any; | |
| const headers: HeaderColumn[] = [ | |
| new HeaderColumn(HeaderColumnType.IMMEDIATE, 'name', 'Name', 'string', true, 'net-id'), | |
| new HeaderColumn(HeaderColumnType.META, 'date', 'Date', 'date') | |
| ]; | |
| service.downloadExcelFromCurrentSelection(mockFilter, headers).subscribe((result) => { | |
| expect(result).toBeTrue(); | |
| done(); | |
| }); | |
| const req = httpMock.expectOne('http://mock-api/export/filteredCases'); | |
| expect(req.request.method).toBe('POST'); | |
| req.flush(new ArrayBuffer(10), { | |
| headers: { 'Content-Type': 'application/vnd.openxmlformats-officedocument.spreadsheetml.sheet' } | |
| }); | |
| }); | |
| it('should return true and trigger file download on valid response', (done) => { | |
| spyOn(document.body, 'appendChild'); | |
| spyOn(document.body, 'removeChild'); | |
| const mockFilter: Filter = { | |
| getRequestBody: () => ({ some: 'query' }) | |
| } as any; | |
| const headers: HeaderColumn[] = [ | |
| new HeaderColumn(HeaderColumnType.IMMEDIATE, 'name', 'Name', 'string', true, 'net-id'), | |
| new HeaderColumn(HeaderColumnType.META, 'date', 'Date', 'date') | |
| ]; | |
| service.downloadExcelFromCurrentSelection(mockFilter, headers).subscribe((result) => { | |
| expect(result).toBeTrue(); | |
| expect(document.body.appendChild).toHaveBeenCalled(); | |
| expect(document.body.removeChild).toHaveBeenCalled(); | |
| done(); | |
| }); | |
| const req = httpMock.expectOne('http://mock-api/export/filteredCases'); | |
| expect(req.request.method).toBe('POST'); | |
| req.flush(new ArrayBuffer(10), { | |
| headers: { 'Content-Type': 'application/vnd.openxmlformats-officedocument.spreadsheetml.sheet' } | |
| }); | |
| }); |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@projects/netgrif-components-core/src/lib/export/services/export.service.spec.ts`
around lines 65 - 88, The test for downloadExcelFromCurrentSelection sets spies
on document.body.appendChild and removeChild but never asserts they were called;
update the spec to assert that document.body.appendChild was called (once) and
that document.body.removeChild was called (once) after flushing the mock
response, and optionally assert the appendChild call received an
HTMLAnchorElement (the download link) to ensure the DOM download trigger was
executed; keep these assertions within the subscribe callback (alongside
expect(result).toBeTrue()) so they run after the HTTP flush.
| public downloadExcelFromCurrentSelection(activeFilter: Filter, currentHeaders: Array<HeaderColumn>): Observable<boolean> { | ||
| const mergeOperation = activeFilter instanceof MergedFilter ? (activeFilter as any)._operator : MergeOperator.AND; | ||
|
|
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major
Accessing private field _operator is fragile; use public getter instead.
MergedFilter exposes _operator through a public getter operator. Accessing the private field via (activeFilter as any)._operator bypasses encapsulation and will break if the internal implementation changes.
Proposed fix
- const mergeOperation = activeFilter instanceof MergedFilter ? (activeFilter as any)._operator : MergeOperator.AND;
+ const mergeOperation = activeFilter instanceof MergedFilter ? (activeFilter as MergedFilter).operator : MergeOperator.AND;📝 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.
| public downloadExcelFromCurrentSelection(activeFilter: Filter, currentHeaders: Array<HeaderColumn>): Observable<boolean> { | |
| const mergeOperation = activeFilter instanceof MergedFilter ? (activeFilter as any)._operator : MergeOperator.AND; | |
| public downloadExcelFromCurrentSelection(activeFilter: Filter, currentHeaders: Array<HeaderColumn>): Observable<boolean> { | |
| const mergeOperation = activeFilter instanceof MergedFilter ? (activeFilter as MergedFilter).operator : MergeOperator.AND; | |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@projects/netgrif-components-core/src/lib/export/services/export.service.ts`
around lines 26 - 28, In downloadExcelFromCurrentSelection replace the fragile
private-field access (activeFilter as any)._operator with the public getter on
MergedFilter: use (activeFilter as MergedFilter).operator (or guard-cast and
read operator) to obtain the MergeOperator; update the mergeOperation assignment
to use that getter so you rely on the MergedFilter.operator API rather than the
private _operator field.
| const linkElement = document.createElement('a'); | ||
| const blob = new Blob([response.body], {type: contentType}); | ||
| const urlBlob = window.URL.createObjectURL(blob); | ||
| linkElement.setAttribute('href', urlBlob); | ||
| linkElement.setAttribute('download', 'export.xlsx'); | ||
| document.body.appendChild(linkElement); | ||
| linkElement.click(); | ||
| document.body.removeChild(linkElement); | ||
| return of(true); |
There was a problem hiding this comment.
Memory leak: Object URL is never revoked.
window.URL.createObjectURL(blob) creates a persistent reference that must be explicitly released with URL.revokeObjectURL(). Without this, repeated exports will accumulate memory.
Proposed fix
const linkElement = document.createElement('a');
const blob = new Blob([response.body], {type: contentType});
const urlBlob = window.URL.createObjectURL(blob);
linkElement.setAttribute('href', urlBlob);
linkElement.setAttribute('download', 'export.xlsx');
document.body.appendChild(linkElement);
linkElement.click();
document.body.removeChild(linkElement);
+ window.URL.revokeObjectURL(urlBlob);
return of(true);📝 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.
| const linkElement = document.createElement('a'); | |
| const blob = new Blob([response.body], {type: contentType}); | |
| const urlBlob = window.URL.createObjectURL(blob); | |
| linkElement.setAttribute('href', urlBlob); | |
| linkElement.setAttribute('download', 'export.xlsx'); | |
| document.body.appendChild(linkElement); | |
| linkElement.click(); | |
| document.body.removeChild(linkElement); | |
| return of(true); | |
| const linkElement = document.createElement('a'); | |
| const blob = new Blob([response.body], {type: contentType}); | |
| const urlBlob = window.URL.createObjectURL(blob); | |
| linkElement.setAttribute('href', urlBlob); | |
| linkElement.setAttribute('download', 'export.xlsx'); | |
| document.body.appendChild(linkElement); | |
| linkElement.click(); | |
| document.body.removeChild(linkElement); | |
| window.URL.revokeObjectURL(urlBlob); | |
| return of(true); |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@projects/netgrif-components-core/src/lib/export/services/export.service.ts`
around lines 41 - 49, The created object URL (urlBlob from
window.URL.createObjectURL(blob)) is never revoked, causing a memory leak;
update the export method in export.service.ts (the block creating linkElement,
blob, urlBlob, setting href/download and clicking) to revoke the URL after
use—e.g., call URL.revokeObjectURL(urlBlob) once the download is initiated
(onlink click handler, link.onload, or in a finally block after click) and
ensure removal of linkElement still occurs; ensure revocation happens even on
errors so repeated exports don't accumulate memory.
| const caseViewHeadersMode = extractFieldValueFromData<string[]>(this._navigationItemTaskData, GroupNavigationConstants.ITEM_FIELD_ID_CASE_HEADERS_MODE); | ||
| const caseViewAllowTableMode = extractFieldValueFromData<boolean>(this._navigationItemTaskData, GroupNavigationConstants.ITEM_FIELD_ID_CASE_ALLOW_TABLE_MODE); | ||
| const caseViewDefaultHeadersMode = extractFieldValueFromData<string[]>(this._navigationItemTaskData, GroupNavigationConstants.ITEM_FIELD_ID_CASE_DEFAULT_HEADERS_MODE); | ||
| const caseViewAllowExport = extractFieldValueFromData<boolean[]>(this._navigationItemTaskData, GroupNavigationConstants.ITEM_FIELD_ID_CASE_ALLOW_EXPORT); |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# 1) Confirm extraction currently uses boolean[] for case allow export
ast-grep --pattern 'extractFieldValueFromData<boolean[]>($_, GroupNavigationConstants.ITEM_FIELD_ID_CASE_ALLOW_EXPORT)'
# 2) Confirm injected interface expects a scalar boolean
rg -n 'caseViewAllowExport:\s*boolean;' \
projects/netgrif-components/src/lib/navigation/group-navigation-component-resolver/default-components/model/injected-tabbed-case-view-data-with-navigation-item-task-data.ts
# 3) Inspect current test coverage for this field presence (likely only happy-path true case)
rg -n 'caseViewAllowExport|ITEM_FIELD_ID_CASE_ALLOW_EXPORT|case_allow_export' \
projects/netgrif-components/src/lib/navigation/group-navigation-component-resolver/default-components --iglob '*.spec.ts'Repository: netgrif/components
Length of output: 888
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# 1) Find and inspect extractFieldValueFromData implementation
rg -n 'export.*extractFieldValueFromData|function extractFieldValueFromData|const extractFieldValueFromData' \
projects/netgrif-components/src --iglob '*.ts' -A 10 | head -50
# 2) Check for optional extraction patterns in the same file
rg -n 'extractFieldValueFromData' \
projects/netgrif-components/src/lib/navigation/group-navigation-component-resolver/default-components/default-tab-view/default-tab-view.component.ts \
-B 2 -A 2
# 3) Check how caseViewAllowExport is used at line 120
sed -n '110,130p' projects/netgrif-components/src/lib/navigation/group-navigation-component-resolver/default-components/default-tab-view/default-tab-view.component.tsRepository: netgrif/components
Length of output: 44
🏁 Script executed:
#!/bin/bash
# 1) Find extractFieldValueFromData implementation
echo "=== Searching for extractFieldValueFromData implementation ==="
rg 'export.*extractFieldValueFromData|function extractFieldValueFromData|const extractFieldValueFromData' \
projects/netgrif-components/src --iglob '*.ts' -A 8 2>/dev/null | head -60 || echo "Pattern not found with exact match"
# 2) All usages of extractFieldValueFromData in the target file
echo -e "\n=== All extractFieldValueFromData usages in default-tab-view.component.ts ==="
rg 'extractFieldValueFromData' \
projects/netgrif-components/src/lib/navigation/group-navigation-component-resolver/default-components/default-tab-view/default-tab-view.component.ts \
-B 1 -A 1 2>/dev/null || echo "File not found"
# 3) Context around line 120
echo -e "\n=== Lines 115-130 in default-tab-view.component.ts ==="
cat -n projects/netgrif-components/src/lib/navigation/group-navigation-component-resolver/default-components/default-tab-view/default-tab-view.component.ts \
2>/dev/null | sed -n '115,130p' || echo "File not found"
# 4) Check interface file to see full context
echo -e "\n=== Interface definition context (lines 20-30) ==="
cat -n projects/netgrif-components/src/lib/navigation/group-navigation-component-resolver/default-components/model/injected-tabbed-case-view-data-with-navigation-item-task-data.ts \
2>/dev/null | sed -n '20,30p' || echo "File not found"Repository: netgrif/components
Length of output: 6614
🏁 Script executed:
#!/bin/bash
# 1) Search for extractFieldValueFromData implementation
echo "=== Looking for extractFieldValueFromData definition ==="
fd -e ts | xargs rg -l 'export.*function.*extractFieldValueFromData|export const extractFieldValueFromData' 2>/dev/null | head -5
# 2) Broader search for the function
echo -e "\n=== Searching for function definition ==="
rg 'function extractFieldValueFromData|const extractFieldValueFromData.*=' projects/netgrif-components/src --iglob '*.ts' -B 2 -A 10 2>/dev/null || true
# 3) Check imports in the component file
echo -e "\n=== Import statements in default-tab-view.component.ts ==="
head -30 projects/netgrif-components/src/lib/navigation/group-navigation-component-resolver/default-components/default-tab-view/default-tab-view.component.tsRepository: netgrif/components
Length of output: 1364
🏁 Script executed:
#!/bin/bash
# Read the implementation file
echo "=== extractFieldValueFromData implementation ==="
cat -n projects/netgrif-components-core/src/lib/navigation/utility/navigation-item-task-utility-methods.ts | head -100Repository: netgrif/components
Length of output: 5798
🏁 Script executed:
#!/bin/bash
# Continue reading the implementation file to find extractFieldValueFromData
echo "=== Looking for extractFieldValueFromData function (lines 100-200) ==="
cat -n projects/netgrif-components-core/src/lib/navigation/utility/navigation-item-task-utility-methods.ts | sed -n '100,200p'Repository: netgrif/components
Length of output: 1574
Fix caseViewAllowExport extraction: incorrect type and missing-field error handling.
Line 86 extracts caseViewAllowExport as boolean[] but the interface at line 24 expects boolean. Additionally, extractFieldValueFromData throws an error if the field is absent, which will break rendering for existing navigation items that don't yet define case_allow_export.
🔧 Proposed fix
- const caseViewAllowExport = extractFieldValueFromData<boolean[]>(this._navigationItemTaskData, GroupNavigationConstants.ITEM_FIELD_ID_CASE_ALLOW_EXPORT);
+ const caseViewAllowExport = this.extractOptionalBooleanField(
+ GroupNavigationConstants.ITEM_FIELD_ID_CASE_ALLOW_EXPORT,
+ false
+ );private extractOptionalBooleanField(fieldId: string, defaultValue = false): boolean {
try {
return extractFieldValueFromData<boolean>(this._navigationItemTaskData, fieldId) ?? defaultValue;
} catch {
return defaultValue;
}
}📝 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.
| const caseViewAllowExport = extractFieldValueFromData<boolean[]>(this._navigationItemTaskData, GroupNavigationConstants.ITEM_FIELD_ID_CASE_ALLOW_EXPORT); | |
| const caseViewAllowExport = this.extractOptionalBooleanField( | |
| GroupNavigationConstants.ITEM_FIELD_ID_CASE_ALLOW_EXPORT, | |
| false | |
| ); | |
| // ... (rest of component code) | |
| private extractOptionalBooleanField(fieldId: string, defaultValue = false): boolean { | |
| try { | |
| return extractFieldValueFromData<boolean>(this._navigationItemTaskData, fieldId) ?? defaultValue; | |
| } catch { | |
| return defaultValue; | |
| } | |
| } |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@projects/netgrif-components/src/lib/navigation/group-navigation-component-resolver/default-components/default-tab-view/default-tab-view.component.ts`
at line 86, The code incorrectly extracts caseViewAllowExport as a boolean[] and
doesn't handle missing fields; update the extraction to expect a boolean and
make it robust by wrapping
extractFieldValueFromData(this._navigationItemTaskData,
GroupNavigationConstants.ITEM_FIELD_ID_CASE_ALLOW_EXPORT) in a safe helper
(e.g., extractOptionalBooleanField) that catches exceptions, returns a default
(false) when the field is absent or null, and uses the boolean generic type;
replace usages of the old extraction (caseViewAllowExport) with this helper to
avoid runtime throws during rendering.
| <div fxLayoutAlign="end center" *ngIf="allowExport"> | ||
| <button mat-mini-fab color="primary" class="export-mat-mini-fab" (click)="export()" | ||
| [disabled]="isLoading()"> | ||
| <mat-icon class="net-upload" *ngIf="(isLoading()) === false" aria-hidden="false" >save_alt</mat-icon> | ||
| <mat-icon class="net-upload" *ngIf="isLoading()" aria-hidden="false">schedule</mat-icon> | ||
| </button> | ||
| </div> |
There was a problem hiding this comment.
Add type="button" to prevent unintended form submissions.
The <button> element is missing a type attribute. Without it, buttons default to type="submit" inside forms, which can cause unintended behavior. This was also flagged by static analysis.
🔧 Proposed fix
- <button mat-mini-fab color="primary" class="export-mat-mini-fab" (click)="export()"
+ <button type="button" mat-mini-fab color="primary" class="export-mat-mini-fab" (click)="export()"
[disabled]="isLoading()">📝 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.
| <div fxLayoutAlign="end center" *ngIf="allowExport"> | |
| <button mat-mini-fab color="primary" class="export-mat-mini-fab" (click)="export()" | |
| [disabled]="isLoading()"> | |
| <mat-icon class="net-upload" *ngIf="(isLoading()) === false" aria-hidden="false" >save_alt</mat-icon> | |
| <mat-icon class="net-upload" *ngIf="isLoading()" aria-hidden="false">schedule</mat-icon> | |
| </button> | |
| </div> | |
| <div fxLayoutAlign="end center" *ngIf="allowExport"> | |
| <button type="button" mat-mini-fab color="primary" class="export-mat-mini-fab" (click)="export()" | |
| [disabled]="isLoading()"> | |
| <mat-icon class="net-upload" *ngIf="(isLoading()) === false" aria-hidden="false" >save_alt</mat-icon> | |
| <mat-icon class="net-upload" *ngIf="isLoading()" aria-hidden="false">schedule</mat-icon> | |
| </button> | |
| </div> |
🧰 Tools
🪛 HTMLHint (1.9.2)
[warning] 11-11: The type attribute must be present on elements.
(button-type-require)
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@projects/netgrif-components/src/lib/navigation/group-navigation-component-resolver/default-components/default-tabbed-case-view/default-tabbed-case-view.component.html`
around lines 10 - 16, The button in default-tabbed-case-view.component.html that
triggers export() and uses [disabled]="isLoading()" is missing an explicit type,
causing it to default to submit in forms; update the <button> element (the one
with class "export-mat-mini-fab" and (click)="export()") to include
type="button" to prevent unintended form submissions while keeping the existing
click and disabled behavior.
| <div fxLayoutAlign="end center" *ngIf="allowExport"> | ||
| <button mat-mini-fab color="primary" class="export-mat-mini-fab" (click)="export()" | ||
| [disabled]="isLoading()"> | ||
| <mat-icon class="net-upload" *ngIf="(isLoading()) === false" aria-hidden="false" >save_alt</mat-icon> |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Minor: Simplify the boolean expression.
The parentheses around isLoading() and the explicit === false comparison are unnecessary.
✨ Suggested simplification
- <mat-icon class="net-upload" *ngIf="(isLoading()) === false" aria-hidden="false" >save_alt</mat-icon>
+ <mat-icon class="net-upload" *ngIf="!isLoading()" aria-hidden="false">save_alt</mat-icon>📝 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.
| <mat-icon class="net-upload" *ngIf="(isLoading()) === false" aria-hidden="false" >save_alt</mat-icon> | |
| <mat-icon class="net-upload" *ngIf="!isLoading()" aria-hidden="false">save_alt</mat-icon> |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@projects/netgrif-components/src/lib/navigation/group-navigation-component-resolver/default-components/default-tabbed-case-view/default-tabbed-case-view.component.html`
at line 13, The *ngIf condition on the mat-icon uses an unnecessary
parenthesized call and explicit comparison; replace the expression on the
mat-icon element so it uses a direct boolean negation of isLoading() (i.e., use
!isLoading()) to simplify the check and keep the element selector (mat-icon with
class "net-upload") unchanged.
| export(): void { | ||
| if (this.loading$.isActive) { | ||
| return; | ||
| } | ||
| this.loading$.on(); | ||
| this._exportService.downloadExcelFromCurrentSelection(this._searchService.activeFilter, this._currentHeaders).subscribe(() => { | ||
| this.loading$.off(); | ||
| },error => { | ||
| this._loggerService.error('File download failed', error); | ||
| this._snackbar.openErrorSnackBar(this._translate.instant('export.errorExportDownload')); | ||
| this.loading$.off(); | ||
| }); | ||
| } |
There was a problem hiding this comment.
Export subscription not managed; callbacks may execute after component destruction.
The subscription to downloadExcelFromCurrentSelection is not tracked. If the component is destroyed while an export is in progress, the callbacks will still execute, potentially causing errors (e.g., accessing this.loading$ after completion, or showing a snackbar on a destroyed view).
Proposed fix using takeUntil or subscription tracking
+ private _exportSub: Subscription;
+
export(): void {
if (this.loading$.isActive) {
return;
}
this.loading$.on();
- this._exportService.downloadExcelFromCurrentSelection(this._searchService.activeFilter, this._currentHeaders).subscribe(() => {
+ this._exportSub = this._exportService.downloadExcelFromCurrentSelection(this._searchService.activeFilter, this._currentHeaders).subscribe(() => {
this.loading$.off();
- },error => {
+ }, error => {
this._loggerService.error('File download failed', error);
this._snackbar.openErrorSnackBar(this._translate.instant('export.errorExportDownload'));
this.loading$.off();
});
}
ngOnDestroy(): void {
+ super.ngOnDestroy();
+ this._headersSub?.unsubscribe();
+ this._exportSub?.unsubscribe();
this.loading$.complete()
}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@projects/netgrif-components/src/lib/navigation/group-navigation-component-resolver/default-components/default-tabbed-case-view/default-tabbed-case-view.component.ts`
around lines 222 - 234, The export() method starts a subscription to
_exportService.downloadExcelFromCurrentSelection but doesn't unsubscribe, so
callbacks may run after the component is destroyed; update export() to pipe the
observable through a teardown operator (e.g., takeUntil(this._destroyed$)) or
add the subscription to a class-level Subscription and unsubscribe in
ngOnDestroy; specifically, create a private _destroyed$ = new Subject<void>()
(or a private _subscriptions: Subscription) on the component, change
_exportService.downloadExcelFromCurrentSelection(...).subscribe(...) to use
.pipe(takeUntil(this._destroyed$)).subscribe(...) (or add the returned
Subscription to _subscriptions), and ensure ngOnDestroy calls
this._destroyed$.next(); this._destroyed$.complete(); (or
this._subscriptions.unsubscribe()) so callbacks that touch loading$, _snackbar,
or _loggerService won't run after destruction.
| ngOnDestroy(): void { | ||
| this.loading$.complete() | ||
| } |
There was a problem hiding this comment.
Memory leak and missing parent cleanup in ngOnDestroy().
Two issues:
_headersSubsubscription (created at line 120) is never unsubscribed, causing a memory leak.super.ngOnDestroy()is not called, skipping parent class cleanup (e.g.,AbstractCaseViewComponentandAbstractViewWithHeadersComponentcleanup).
Proposed fix
ngOnDestroy(): void {
+ super.ngOnDestroy();
+ this._headersSub?.unsubscribe();
this.loading$.complete()
}📝 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.
| ngOnDestroy(): void { | |
| this.loading$.complete() | |
| } | |
| ngOnDestroy(): void { | |
| super.ngOnDestroy(); | |
| this._headersSub?.unsubscribe(); | |
| this.loading$.complete() | |
| } |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@projects/netgrif-components/src/lib/navigation/group-navigation-component-resolver/default-components/default-tabbed-case-view/default-tabbed-case-view.component.ts`
around lines 236 - 238, ngOnDestroy currently only completes loading$ but leaves
the _headersSub subscription open and doesn't invoke parent cleanup; update the
component's ngOnDestroy to unsubscribe or teardown _headersSub (call
_headersSub.unsubscribe() or use a safe null-check/cleanup routine) and then
call super.ngOnDestroy() after performing local cleanup so parent classes (e.g.,
AbstractCaseViewComponent/AbstractViewWithHeadersComponent) run their teardown;
ensure loading$.complete() is still called and the order is: clean local
subscriptions (unsubscribe _headersSub), complete loading$, then call
super.ngOnDestroy().
|



Instruction for the developer
For starters, congratulation on opening a PR of your work. I hope you're proud of your work and yourself 😉 .
There are some things you need to do before this PR is submitted, so read carefully.
First, set yourself as an assignee. You can also add other people responsible for code in this PR (i.e. your team lead).
Then set reviewers. There should be at least two reviewers. You can set your team lead, projects owner, or senior developer
(after his or her agreement, of course) as a PR reviewer.
The last element on the right panel are labels. Labels should be based on what type of change the PR brings to the table.
You can add one or more labels that apply to the PR:
Next thing, please read and edit the PR description below the following way:
<>symbols, replace it with your content according to the instructions in the statement./is between two statements, choose to fill only one or the other.[ ]brackets.@symbol.#Land the number of the desired line of code.For whole markdown documentation please read GitHub Markdown.
If you read these whole instructions and have done everything, you are the best 👍 .
You can now safely delete all between two horizontal lines, so the instructions don't look bad in your PR.
Description
<Fixes [JIRA-ISSUE]>/<Implements [JIRA-ISSUE]>
Dependencies
Third party dependencies
Blocking Pull requests
<Depends on #(PR id)>/
How Has Been This Tested?
Test Configuration
<Please describe configuration for tests to run if applicable, like program parameters, host OS, VM configuration etc. You can use >
Checklist:
Summary by CodeRabbit
Release Notes - Version 6.4.2
New Features
Bug Fixes