Skip to content

[NAE-2416] AbstractFileDefaultFieldComponent does not push upload event #326

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
renczesstefan wants to merge 7 commits into
base: release/7.0.0-rev10
Choose a base branch
from
Open

[NAE-2416] AbstractFileDefaultFieldComponent does not push upload event #326

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
c3755d2
[NAE-2416] AbstractFileDefaultFieldComponent does not push upload eve…
renczesstefan Apr 22, 2026
4d83a81
Add FrontActionService to test providers in file components
renczesstefan Apr 27, 2026
1a5e592
Fix logging and error handling in file upload components
renczesstefan Apr 28, 2026
7594f6a
Update file handling and event parsing, add test cases
renczesstefan Apr 28, 2026
340d259
Update file list field test with additional imports and template
renczesstefan Apr 28, 2026
b859995
Add unit tests for download and upload methods
renczesstefan Apr 28, 2026
26db136
Add unit tests for download and upload methods
renczesstefan Apr 28, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 2 additions & 1 deletion projects/netgrif-components-core/src/lib/actions/front-action.module.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
import { NgModule } from '@angular/core';
import { CommonModule } from '@angular/common';
import {FrontActionRegistryService} from "../registry/front-action-registry.service";
import {redirectAction} from "./model/router-action-definitions";
import {redirectAction, snackBarAction} from "./model/router-action-definitions";
import {reloadTaskAction, validateTaskAction} from "./model/task-action-definitions";

@NgModule({
Expand All @@ -16,5 +16,6 @@ export class FrontActionModule {
frontActionsRegistry.register('redirect', redirectAction);
frontActionsRegistry.register('validate', validateTaskAction);
frontActionsRegistry.register('reloadTask', reloadTaskAction);
frontActionsRegistry.register('snackBar', snackBarAction);
}
}
1 change: 1 addition & 0 deletions projects/netgrif-components-core/src/lib/actions/services/front-action.service.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@ export class FrontActionService {
const fn = this._frontActionRegistry.get(frontAction.id)
if (!fn) {
this._log.error("Frontend action is not defined for ID [" + frontAction.id +"]")
return;
}
fn.call(this._injector, frontAction)
}
Expand Down
22 changes: 17 additions & 5 deletions ...b/data-fields/file-field/file-default-field/abstract-file-default-field.component.spec.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,9 +16,6 @@ import {MockUserResourceService} from "../../../utility/tests/mocks/mock-user-re
import {ConfigurationService} from "../../../configuration/configuration.service";
import {TestConfigurationService} from "../../../utility/tests/test-config";
import {Component, CUSTOM_ELEMENTS_SCHEMA, Inject, Optional} from "@angular/core";
import {BrowserDynamicTestingModule} from "@angular/platform-browser-dynamic/testing";
import {ErrorSnackBarComponent} from "../../../snack-bar/components/error-snack-bar/error-snack-bar.component";
import {SuccessSnackBarComponent} from "../../../snack-bar/components/success-snack-bar/success-snack-bar.component";
import {TaskResourceService} from "../../../resources/engine-endpoint/task-resource.service";
import {LoggerService} from "../../../logger/services/logger.service";
import {SnackBarService} from "../../../snack-bar/services/snack-bar.service";
Expand All @@ -29,6 +26,7 @@ import {AbstractFileDefaultFieldComponent} from "./abstract-file-default-field.c
import {DATA_FIELD_PORTAL_DATA, DataFieldPortalData} from "../../models/data-field-portal-data-injection-token";
import {FormControl} from "@angular/forms";
import {WrappedBoolean} from "../../data-field-template/models/wrapped-boolean";
import {FrontActionService} from "../../../actions/services/front-action.service";

describe('AbstractFileDefaultFieldComponent', () => {
let component: TestFileComponent;
Expand All @@ -48,6 +46,7 @@ describe('AbstractFileDefaultFieldComponent', () => {
providers: [
SideMenuService,
EventService,
FrontActionService,
{provide: AuthenticationMethodService, useClass: MockAuthenticationMethodService},
{provide: AuthenticationService, useClass: MockAuthenticationService},
{provide: UserResourceService, useClass: MockUserResourceService},
Expand Down Expand Up @@ -81,14 +80,26 @@ describe('AbstractFileDefaultFieldComponent', () => {
expect(component).toBeTruthy();
});

it('should call download method successfully', () => {
spyOn(component, 'download').and.callThrough(); // Spy on the method
component.download(); // Call the method
expect(component.download).toHaveBeenCalled(); // Assert that it was called
});

it('should call upload method successfully', () => {
spyOn(component, 'upload').and.callThrough(); // Spy on the method
component.upload(); // Call the method
expect(component.upload).toHaveBeenCalled(); // Assert that it was called
});
Comment on lines +83 to +93
Copy link
Copy Markdown

@coderabbitai coderabbitai Bot Apr 28, 2026

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major

These tests are tautological and can still miss regressions.

At Line 84 and Line 90, the spec spies on a method and then directly calls that same method, so it only proves the test invocation happened—not upload/download behavior. With callThrough(), it can also execute real side effects and make tests flaky. Please assert observable behavior/dependency interactions (success + error paths) instead of self-invocation.

🤖 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-field/file-default-field/abstract-file-default-field.component.spec.ts`
around lines 83 - 93, The tests are tautological because they spyOn
component.download and component.upload and then call those same methods;
instead, remove callThrough and instead stub the underlying dependencies and
assert observable side effects: replace spyOn(component,
'download').and.callThrough() with spying/stubbing the service or utility that
download() delegates to (e.g., fileService.downloadFile or httpClient.get) and
assert that when you call component.download() it triggers that dependency with
correct args and handles success and error paths (mock success
observable/Promise and mock error to verify error handling); do the same for
component.upload() by spying on the upload dependency (e.g.,
fileService.uploadFile or formSubmit) and assert successful response handling
and error handling rather than asserting the method call itself.


afterEach(() => {
TestBed.resetTestingModule();
});
});

@Component({
selector: 'ncc-test-file',
template: ''
template: '<input type="file" #fileUploadInput name="fileUpload" [multiple]="true" accept="{{dataField.allowTypes}}" class="invisible-input"/>'
})
class TestFileComponent extends AbstractFileDefaultFieldComponent {
constructor(taskResourceService: TaskResourceService,
Expand All @@ -97,8 +108,9 @@ class TestFileComponent extends AbstractFileDefaultFieldComponent {
translate: TranslateService,
sanitizer: DomSanitizer,
eventService: EventService,
frontActionService: FrontActionService,
@Optional() @Inject(DATA_FIELD_PORTAL_DATA) dataFieldPortalData: DataFieldPortalData<FileField>) {
super(taskResourceService, log, snackbar, translate, eventService, sanitizer, dataFieldPortalData);
super(taskResourceService, log, snackbar, translate, eventService, sanitizer, frontActionService, dataFieldPortalData);
}
}

Expand Down
100 changes: 55 additions & 45 deletions ...rc/lib/data-fields/file-field/file-default-field/abstract-file-default-field.component.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,8 @@
import {FILE_FIELD_HEIGHT, FILE_FIELD_PADDING, PREVIEW, PREVIEW_BUTTON} from '../models/file-field-constants';
import {FileFieldRequest} from "../../../resources/interface/file-field-request-body";
import {AbstractFileFieldDefaultComponent} from '../../models/abstract-file-field-default-component';
import {FrontAction} from "../../models/changed-fields";
import {FrontActionService} from "../../../actions/services/front-action.service";

export interface FileState {
progress: number;
Expand Down Expand Up @@ -114,6 +116,7 @@
protected _translate: TranslateService,
protected _eventService: EventService,
protected _sanitizer: DomSanitizer,
protected _frontActionService: FrontActionService,
@Optional() @Inject(DATA_FIELD_PORTAL_DATA) dataFieldPortalData: DataFieldPortalData<FileField>) {
super(_log, _snackbar, _translate, dataFieldPortalData);
this.state = this.defaultState;
Expand Down Expand Up @@ -217,60 +220,65 @@
fileFormData.append('file', fileToUpload);
fileFormData.append('data', new Blob([JSON.stringify(this.createRequestBody())], {type: 'application/json'}));
this._taskResourceService.uploadFile(this.taskId, fileFormData, false)
.subscribe((response: EventOutcomeMessageResource) => {
if ((response as ProviderProgress).type && (response as ProviderProgress).type === ProgressType.UPLOAD) {
this.state.progress = (response as ProviderProgress).progress;
} else {
this.state.completed = true;
this.state.uploading = false;
this.state.progress = 0;
.subscribe({
next: (response: EventOutcomeMessageResource) => {
if ((response as ProviderProgress).type && (response as ProviderProgress).type === ProgressType.UPLOAD) {
this.state.progress = (response as ProviderProgress).progress;
} else {
this.state.completed = true;
this.state.uploading = false;
this.state.progress = 0;

if (response.error) {
this.state.error = true;
this._log.error(
`File [${this.dataField.stringId}] ${this.fileUploadEl.nativeElement.files.item(0)} uploading has failed!`, response.error
);
if (response.error) {
this.state.error = true;
this._log.error(
`File [${this.dataField.stringId}] ${this.fileUploadEl.nativeElement.files.item(0)?.name} uploading has failed!`, response.error
);
Comment thread
coderabbitai[bot] marked this conversation as resolved.
this._snackbar.openErrorSnackBar(this._translate.instant(response.error));

} else {
this._snackbar.openErrorSnackBar(this._translate.instant('dataField.snackBar.fileUploadFailed'));
const changedFieldsMap: ChangedFieldsMap = this._eventService.parseChangedFieldsFromOutcomeTree(response.outcome);
this.dataField.emitChangedFields(changedFieldsMap);
this._log.debug(
`File [${this.dataField.stringId}] ${this.fileUploadEl.nativeElement.files.item(0)?.name} was successfully uploaded`
);
this.state.error = false;
this.dataField.downloaded = false;
this.dataField.value.name = fileToUpload.name;
if (this.isFilePreview) {
this.initializePreviewIfDisplayable();
}
this.fullSource.next(undefined);
this.fileForDownload = undefined;
this.formControlRef.setValue(this.dataField.value.name);
this._snackbar.openSuccessSnackBar(!!response.outcome.message ? response.outcome.message : this._translate.instant('tasks.snackbar.dataSaved'));

Check warning on line 254 in projects/netgrif-components-core/src/lib/data-fields/file-field/file-default-field/abstract-file-default-field.component.ts

View check run for this annotation

SonarQubeCloud / SonarCloud Code Analysis

Unexpected negated condition. 


See more on https://sonarcloud.io/project/issues?id=netgrif_components&issues=AZ3O0hl5cCPq9sm4VVs5&open=AZ3O0hl5cCPq9sm4VVs5&pullRequest=326

Check warning on line 254 in projects/netgrif-components-core/src/lib/data-fields/file-field/file-default-field/abstract-file-default-field.component.ts

View check run for this annotation

SonarQubeCloud / SonarCloud Code Analysis

Redundant double negation. 


See more on https://sonarcloud.io/project/issues?id=netgrif_components&issues=AZ3O0hl5cCPq9sm4VVs6&open=AZ3O0hl5cCPq9sm4VVs6&pullRequest=326
const frontActions: Array<FrontAction> = this._eventService.parseFrontActionsFromOutcomeTree(response.outcome);
if (frontActions?.length > 0) {
this._frontActionService.runAll(frontActions);
Comment thread
coderabbitai[bot] marked this conversation as resolved.
}
Comment thread
coderabbitai[bot] marked this conversation as resolved.
}
this.dataField.touch = true;
this.dataField.update();
this.fileUploadEl.nativeElement.value = '';
}
},
error: (error) => {
this.state.completed = true;
this.state.error = true;
this.state.uploading = false;
this.state.progress = 0;
this._log.error(
`File [${this.dataField.stringId}] ${this.fileUploadEl.nativeElement.files.item(0)?.name} uploading has failed!`, error
);
if (error?.error?.message) {
this._snackbar.openErrorSnackBar(this._translate.instant(error.error.message));
} else {
const changedFieldsMap: ChangedFieldsMap = this._eventService.parseChangedFieldsFromOutcomeTree(response.outcome);
this.dataField.emitChangedFields(changedFieldsMap);
this._log.debug(
`File [${this.dataField.stringId}] ${this.fileUploadEl.nativeElement.files.item(0).name} was successfully uploaded`
);
this.state.error = false;
this.dataField.downloaded = false;
this.dataField.value.name = fileToUpload.name;
if (this.isFilePreview) {
this.initializePreviewIfDisplayable();
}
this.fullSource.next(undefined);
this.fileForDownload = undefined;
this.formControlRef.setValue(this.dataField.value.name);
this._snackbar.openErrorSnackBar(this._translate.instant('dataField.snackBar.fileUploadFailed'));
}
this.dataField.touch = true;
this.dataField.update();
this.fileUploadEl.nativeElement.value = '';
}
}, error => {
this.state.completed = true;
this.state.error = true;
this.state.uploading = false;
this.state.progress = 0;
this._log.error(
`File [${this.dataField.stringId}] ${this.fileUploadEl.nativeElement.files.item(0)} uploading has failed!`, error
);
if (error?.error?.message) {
this._snackbar.openErrorSnackBar(this._translate.instant(error.error.message));
} else {
this._snackbar.openErrorSnackBar(this._translate.instant('dataField.snackBar.fileUploadFailed'));
}
this.dataField.touch = true;
this.dataField.update();
this.fileUploadEl.nativeElement.value = '';
});
}

Expand Down Expand Up @@ -410,7 +418,8 @@
this.state.downloading = true;
let params = new HttpParams()
params = params.set("fieldId", this.dataField.stringId);
this._taskResourceService.downloadFilePreview(this.resolveParentTaskId(), params).subscribe(response => { if (response instanceof Blob) {
this._taskResourceService.downloadFilePreview(this.resolveParentTaskId(), params).subscribe(response => {

Check warning on line 421 in projects/netgrif-components-core/src/lib/data-fields/file-field/file-default-field/abstract-file-default-field.component.ts

View check run for this annotation

SonarQubeCloud / SonarCloud Code Analysis

'(next?: (value: Blob | ProviderProgress) => void, error?: (error: any) => void, complete?: () => void): Subscription' is deprecated. 


See more on https://sonarcloud.io/project/issues?id=netgrif_components&issues=AZ3O0hl5cCPq9sm4VVs7&open=AZ3O0hl5cCPq9sm4VVs7&pullRequest=326
if (response instanceof Blob) {
this._log.debug(`Preview of file [${this.dataField.stringId}] ${this.dataField.value.name} was successfully downloaded`);
this.fileForPreview = new Blob([response], {type: 'application/octet-stream'});
this.previewSource = this._sanitizer.bypassSecurityTrustUrl(URL.createObjectURL(this.fileForPreview));
Expand Down Expand Up @@ -445,7 +454,8 @@
}
let params = new HttpParams();
params = params.set("fieldId", this.dataField.stringId);
this._taskResourceService.downloadFile(this.resolveParentTaskId(), params).subscribe(response => { if (!(response as ProviderProgress).type || (response as ProviderProgress).type !== ProgressType.DOWNLOAD) {
this._taskResourceService.downloadFile(this.resolveParentTaskId(), params).subscribe(response => {

Check warning on line 457 in projects/netgrif-components-core/src/lib/data-fields/file-field/file-default-field/abstract-file-default-field.component.ts

View check run for this annotation

SonarQubeCloud / SonarCloud Code Analysis

'(next?: (value: Blob | ProviderProgress) => void, error?: (error: any) => void, complete?: () => void): Subscription' is deprecated. 


See more on https://sonarcloud.io/project/issues?id=netgrif_components&issues=AZ3O0hl5cCPq9sm4VVs8&open=AZ3O0hl5cCPq9sm4VVs8&pullRequest=326
if (!(response as ProviderProgress).type || (response as ProviderProgress).type !== ProgressType.DOWNLOAD) {
this._log.debug(`File [${this.dataField.stringId}] ${this.dataField.value.name} was successfully downloaded`);
this.initDownloadFile(response);
}
Expand Down
28 changes: 21 additions & 7 deletions ...ile-list-field/file-list-default-field/abstract-file-list-default-field.component.spec.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,8 +1,8 @@
import {ComponentFixture, TestBed, waitForAsync} from "@angular/core/testing";
import {ComponentFixture, inject, TestBed, waitForAsync} from "@angular/core/testing";
import {MaterialModule} from "../../../material/material.module";
import {AngularResizeEventModule} from "angular-resize-event";
import {BrowserAnimationsModule} from "@angular/platform-browser/animations";
import {HttpClientTestingModule} from "@angular/common/http/testing";
import {HttpClientTestingModule, HttpTestingController} from "@angular/common/http/testing";
import {TranslateLibModule} from "../../../translate/translate-lib.module";
import {SnackBarModule} from "../../../snack-bar/snack-bar.module";
import {SideMenuService} from "../../../side-menu/services/side-menu.service";
Expand All @@ -16,9 +16,6 @@ import {MockUserResourceService} from "../../../utility/tests/mocks/mock-user-re
import {ConfigurationService} from "../../../configuration/configuration.service";
import {TestConfigurationService} from "../../../utility/tests/test-config";
import {Component, CUSTOM_ELEMENTS_SCHEMA, Inject, Optional} from "@angular/core";
import {BrowserDynamicTestingModule} from "@angular/platform-browser-dynamic/testing";
import {ErrorSnackBarComponent} from "../../../snack-bar/components/error-snack-bar/error-snack-bar.component";
import {SuccessSnackBarComponent} from "../../../snack-bar/components/success-snack-bar/success-snack-bar.component";
import {TaskResourceService} from "../../../resources/engine-endpoint/task-resource.service";
import {LoggerService} from "../../../logger/services/logger.service";
import {SnackBarService} from "../../../snack-bar/services/snack-bar.service";
Expand All @@ -28,6 +25,9 @@ import {DATA_FIELD_PORTAL_DATA, DataFieldPortalData} from "../../models/data-fie
import {AbstractFileListDefaultFieldComponent} from "./abstract-file-list-default-field.component";
import {FormControl} from "@angular/forms";
import {WrappedBoolean} from "../../data-field-template/models/wrapped-boolean";
import {FrontActionService} from "../../../actions/services/front-action.service";
import {AbstractFileListFieldComponent} from "../abstract-file-list-field.component";
import {NAE_INFORM_ABOUT_INVALID_DATA} from "../../models/invalid-data-policy-token";

describe('AbstractFileListDefaultFieldComponent', () => {
let component: TestFileListComponent;
Expand All @@ -46,6 +46,7 @@ describe('AbstractFileListDefaultFieldComponent', () => {
providers: [
SideMenuService,
EventService,
FrontActionService,
{provide: AuthenticationMethodService, useClass: MockAuthenticationMethodService},
{provide: AuthenticationService, useClass: MockAuthenticationService},
{provide: UserResourceService, useClass: MockUserResourceService},
Expand Down Expand Up @@ -79,23 +80,36 @@ describe('AbstractFileListDefaultFieldComponent', () => {
expect(component).toBeTruthy();
});

it('should call download method successfully', () => {
spyOn(component, 'download').and.callThrough(); // Spy on the method
component.download("test"); // Call the method
expect(component.download).toHaveBeenCalled(); // Assert that it was called
});

it('should call upload method successfully', () => {
spyOn(component, 'upload').and.callThrough(); // Spy on the method
component.upload(); // Call the method
expect(component.upload).toHaveBeenCalled(); // Assert that it was called
});
Comment on lines +83 to +93
Copy link
Copy Markdown

@coderabbitai coderabbitai Bot Apr 28, 2026

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟡 Minor

These tests are self-fulfilling and don’t verify behavior.

On Lines 84-93, each test spies on a method, calls the same method, then asserts it was called. This still passes even if internal logic breaks. Please assert observable effects (e.g., service calls, state changes, snackbar calls, or early-return guards).

🤖 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.spec.ts`
around lines 83 - 93, The tests currently only spy on component.download and
component.upload and then call them, which doesn't verify behavior; replace
these self-fulfilling assertions by spying on the observable side-effects of
those methods (e.g., the file service methods, HTTP calls, snackbar/dialog
interactions, or component state flags) so download and upload are exercised but
asserted via dependencies: for download, spyOn the file download service or
window.open/anchor trigger that download() should call and assert it was invoked
with the "test" argument (or that a download URL was set); for upload, spyOn the
upload/fileService.upload method or the component's isUploading/isDisabled flags
and assert the service call and the state change or snackbar notification; keep
using component.download and component.upload to invoke behavior but assert on
the external service calls or state changes rather than on the methods
themselves.


afterEach(() => {
TestBed.resetTestingModule();
});
});

@Component({
selector: 'ncc-test-filelist',
template: ''
template: '<input type="file" #fileUploadInput name="fileUpload" [multiple]="true" accept="{{dataField.allowTypes}}" class="invisible-input"/>'
})
class TestFileListComponent extends AbstractFileListDefaultFieldComponent {
constructor(taskResourceService: TaskResourceService,
log: LoggerService,
snackbar: SnackBarService,
translate: TranslateService,
eventService: EventService,
frontActionService: FrontActionService,
@Optional() @Inject(DATA_FIELD_PORTAL_DATA) dataFieldPortalData: DataFieldPortalData<FileListField>) {
super(taskResourceService, log, snackbar, translate, eventService, dataFieldPortalData);
super(taskResourceService, log, snackbar, translate, eventService, frontActionService, dataFieldPortalData);
}
}

Expand Down
Loading
Loading