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 1 commit 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
1 commit
Select commit
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);
}
}
8 changes: 4 additions & 4 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 @@ -97,8 +96,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
105 changes: 59 additions & 46 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 {DATA_FIELD_PORTAL_DATA, DataFieldPortalData} from "../../models/data-fie
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 @@ export abstract class AbstractFileDefaultFieldComponent extends AbstractFileFiel
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,68 @@ export abstract class AbstractFileDefaultFieldComponent extends AbstractFileFiel
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._snackbar.openErrorSnackBar(this._translate.instant(response.error));
this.state.error = true;
this._log.error(
`File [${this.dataField.stringId}] ${this.fileUploadEl.nativeElement.files.item(0)} uploading has failed!`, response.error
);
Comment on lines +234 to +236
Copy link
Copy Markdown

@coderabbitai coderabbitai Bot Apr 24, 2026

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟡 Minor

Error logs print File object instead of file name.

this.fileUploadEl.nativeElement.files.item(0) returns a File reference that renders as [object File] in most log sinks; the success-path log at line 246 correctly uses .name. Make the failure logs consistent so operators can identify which file failed.

🛠️ Proposed fix 
-                            this._log.error(
-                                `File [${this.dataField.stringId}] ${this.fileUploadEl.nativeElement.files.item(0)} uploading has failed!`, response.error
-                            );
+                            this._log.error(
+                                `File [${this.dataField.stringId}] ${this.fileUploadEl.nativeElement.files.item(0)?.name} uploading has failed!`, response.error
+                            );
-                    this._log.error(
-                        `File [${this.dataField.stringId}] ${this.fileUploadEl.nativeElement.files.item(0)} uploading has failed!`, error
-                    );
+                    this._log.error(
+                        `File [${this.dataField.stringId}] ${this.fileUploadEl.nativeElement.files.item(0)?.name} uploading has failed!`, error
+                    );

Also applies to: 273-275

🤖 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.ts`
around lines 234 - 236, The failure log is printing the File object instead of
its name; update the calls to this._log.error in the upload-failure paths (the
one using this.dataField.stringId and
this.fileUploadEl.nativeElement.files.item(0) and the similar block around lines
273-275) to reference the file's name property (use
this.fileUploadEl.nativeElement.files.item(0).name) so the log matches the
success-path format and operators see the actual filename; keep the same
message/context and still pass response.error as the second argument to
this._log.error.

if (response.error) {
this._snackbar.openErrorSnackBar(this._translate.instant(response.error));
} else {
this._snackbar.openErrorSnackBar(this._translate.instant('dataField.snackBar.fileUploadFailed'));
}
Comment on lines 232 to +241
Copy link
Copy Markdown

@coderabbitai coderabbitai Bot Apr 24, 2026

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟡 Minor

Dead code: inner if (response.error) is always true.

Line 232 already guards response.error being truthy, so the nested if (response.error) on line 237 cannot be false and the else fallback to dataField.snackBar.fileUploadFailed is unreachable. Either drop the inner branch or replace the outer guard with a different condition (e.g. check for a truthy outcome/response shape) so the fallback is actually reachable.

🛠️ Proposed fix 
                         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._snackbar.openErrorSnackBar(this._translate.instant(response.error));
-                            } else {
-                                this._snackbar.openErrorSnackBar(this._translate.instant('dataField.snackBar.fileUploadFailed'));
-                            }
+                            this._snackbar.openErrorSnackBar(this._translate.instant(response.error));
                         } else {
🤖 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.ts`
around lines 232 - 241, The outer if checks response.error and the inner if
(response.error) is dead code; remove the nested check and ensure the snackbar
fallback is reachable by using a single conditional: inside the existing block
where response.error is truthy call this._log.error(...) with response.error and
then call
this._snackbar.openErrorSnackBar(this._translate.instant(response.error ||
'dataField.snackBar.fileUploadFailed')); update references to response.error,
this._log.error, this._snackbar.openErrorSnackBar and this._translate to
implement this single check (keep fileUploadEl/nativeElement and
this.dataField.stringId unchanged).

} 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'));
const frontActions: Array<FrontAction> = this._eventService.parseFrontActionsFromOutcomeTree(response.outcome);
if (frontActions?.length > 0) {
this._frontActionService.runAll(frontActions);
}
Comment on lines +258 to +261
Copy link
Copy Markdown

@coderabbitai coderabbitai Bot Apr 24, 2026

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
# Confirm the service still dereferences fn unconditionally.
rg -nP -C3 'public\s+run\s*\(' --type=ts -g '**/front-action.service.ts'
# And check parseFrontActionsFromOutcomeTree return contract.
ast-grep --pattern 'parseFrontActionsFromOutcomeTree($_) { $$$ }'

Repository: netgrif/components

Length of output: 891

🏁 Script executed:

# Get the full implementation of the run() method and surrounding context
cat -n projects/netgrif-components-core/src/lib/actions/services/front-action.service.ts | head -30

Repository: netgrif/components

Length of output: 1156

🏁 Script executed:

# Also check the runAll() method to understand the call pattern
rg -nP -A 10 'runAll\s*\(' --type=ts -g '**/front-action.service.ts'

Repository: netgrif/components

Length of output: 596

🏁 Script executed:

# Check the context around the runAll call in the file-field component
rg -nP -B5 -A5 'runAll\s*\(' --type=ts -g '**/abstract-file-default-field.component.ts'

Repository: netgrif/components

Length of output: 2295

Fix unhandled TypeError when unregistered action IDs are in upload outcome.

In FrontActionService.run() (projects/netgrif-components-core/src/lib/actions/services/front-action.service.ts:15-21), the if-guard logs an error when the action ID is missing but lacks a return statement, allowing execution to proceed to fn.call(...) with undefined. When parseFrontActionsFromOutcomeTree() returns an entry with an unknown ID, runAll() iterates through and invokes the faulty run(), throwing TypeError: Cannot read properties of undefined (reading 'call') inside the subscriber after the upload succeeds. Add a return statement in the if-block to prevent this unhandled exception.

🤖 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.ts`
around lines 258 - 261, FrontActionService.run currently logs an error when an
action ID is not registered but does not return, causing fn to be undefined and
a TypeError when fn.call(...) is invoked; update FrontActionService.run so that
inside the if-block that checks for missing action (the block that logs the
error) you add an early return to exit the method and avoid calling fn.call,
ensuring runAll() and subscribers won’t throw when
parseFrontActionsFromOutcomeTree yields unknown IDs.

}
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)} 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 +421,8 @@ export abstract class AbstractFileDefaultFieldComponent extends AbstractFileFiel
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 => {
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 +457,8 @@ export abstract class AbstractFileDefaultFieldComponent extends AbstractFileFiel
}
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 => {
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
8 changes: 4 additions & 4 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
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 @@ -28,6 +25,7 @@ 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";

describe('AbstractFileListDefaultFieldComponent', () => {
let component: TestFileListComponent;
Expand All @@ -46,6 +44,7 @@ describe('AbstractFileListDefaultFieldComponent', () => {
providers: [
SideMenuService,
EventService,
FrontActionService,
{provide: AuthenticationMethodService, useClass: MockAuthenticationMethodService},
{provide: AuthenticationService, useClass: MockAuthenticationService},
{provide: UserResourceService, useClass: MockUserResourceService},
Expand Down Expand Up @@ -94,8 +93,9 @@ class TestFileListComponent extends AbstractFileListDefaultFieldComponent {
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