diff --git a/core/amber/src/main/scala/edu/uci/ics/texera/web/resource/dashboard/user/workflow/WorkflowResource.scala b/core/amber/src/main/scala/edu/uci/ics/texera/web/resource/dashboard/user/workflow/WorkflowResource.scala index d06607445a3..d3d4b09d161 100644 --- a/core/amber/src/main/scala/edu/uci/ics/texera/web/resource/dashboard/user/workflow/WorkflowResource.scala +++ b/core/amber/src/main/scala/edu/uci/ics/texera/web/resource/dashboard/user/workflow/WorkflowResource.scala @@ -103,6 +103,28 @@ object WorkflowResource { case class WorkflowIDs(wids: List[UInteger], pid: Option[UInteger]) + private def updateWorkflowField( + workflow: Workflow, + sessionUser: SessionUser, + updateFunction: Workflow => Unit + ): Unit = { + val wid = workflow.getWid + val user = sessionUser.getUser + + if ( + workflowOfUserExists(wid, user.getUid) || WorkflowAccessResource.hasWriteAccess( + wid, + user.getUid + ) + ) { + val userWorkflow = workflowDao.fetchOneByWid(wid) + updateFunction(userWorkflow) + workflowDao.update(userWorkflow) + } else { + throw new ForbiddenException("No sufficient access privilege.") + } + } + } @Produces(Array(MediaType.APPLICATION_JSON)) @RolesAllowed(Array("REGULAR", "ADMIN")) @@ -466,11 +488,6 @@ class WorkflowResource extends LazyLogging { } } - /** - * This method updates the name of a given workflow - * - * @return Response - */ @POST @Consumes(Array(MediaType.APPLICATION_JSON)) @Produces(Array(MediaType.APPLICATION_JSON)) @@ -479,18 +496,7 @@ class WorkflowResource extends LazyLogging { workflow: Workflow, @Auth sessionUser: SessionUser ): Unit = { - val wid = workflow.getWid - val name = workflow.getName - val user = sessionUser.getUser - if (!WorkflowAccessResource.hasWriteAccess(wid, user.getUid)) { - throw new ForbiddenException("No sufficient access privilege.") - } else if (!workflowOfUserExists(wid, user.getUid)) { - throw new BadRequestException("The workflow does not exist.") - } else { - val userWorkflow = workflowDao.fetchOneByWid(wid) - userWorkflow.setName(name) - workflowDao.update(userWorkflow) - } + updateWorkflowField(workflow, sessionUser, _.setName(workflow.getName)) } @POST @@ -501,19 +507,7 @@ class WorkflowResource extends LazyLogging { workflow: Workflow, @Auth sessionUser: SessionUser ): Unit = { - val wid = workflow.getWid - val description = workflow.getDescription - val user = sessionUser.getUser - - if (!WorkflowAccessResource.hasWriteAccess(wid, user.getUid)) { - throw new ForbiddenException("No sufficient access privilege.") - } else if (!workflowOfUserExists(wid, user.getUid)) { - throw new BadRequestException("The workflow does not exist.") - } else { - val userWorkflow = workflowDao.fetchOneByWid(wid) - userWorkflow.setDescription(description) - workflowDao.update(userWorkflow) - } + updateWorkflowField(workflow, sessionUser, _.setDescription(workflow.getDescription)) } @PUT diff --git a/core/gui/src/app/dashboard/component/user/list-item/list-item.component.spec.ts b/core/gui/src/app/dashboard/component/user/list-item/list-item.component.spec.ts new file mode 100644 index 00000000000..cc6517b8c32 --- /dev/null +++ b/core/gui/src/app/dashboard/component/user/list-item/list-item.component.spec.ts @@ -0,0 +1,87 @@ +import { ComponentFixture, TestBed } from "@angular/core/testing"; +import { ListItemComponent } from "./list-item.component"; +import { WorkflowPersistService } from "src/app/common/service/workflow-persist/workflow-persist.service"; +import { HttpClientTestingModule } from "@angular/common/http/testing"; +import { NzModalService } from "ng-zorro-antd/modal"; +import { of, throwError } from "rxjs"; +import { NO_ERRORS_SCHEMA } from "@angular/core"; + +describe("ListItemComponent", () => { + let component: ListItemComponent; + let fixture: ComponentFixture; + let workflowPersistService: jasmine.SpyObj; + let nzModalService: jasmine.SpyObj; + + beforeEach(async () => { + const workflowPersistServiceSpy = jasmine.createSpyObj("WorkflowPersistService", [ + "updateWorkflowName", + "updateWorkflowDescription", + ]); + const nzModalServiceSpy = jasmine.createSpyObj("NzModalService", ["create"]); + + await TestBed.configureTestingModule({ + imports: [HttpClientTestingModule], + declarations: [ListItemComponent], + providers: [ + { provide: WorkflowPersistService, useValue: workflowPersistServiceSpy }, + { provide: NzModalService, useValue: nzModalServiceSpy }, + ], + schemas: [NO_ERRORS_SCHEMA], + }).compileComponents(); + + fixture = TestBed.createComponent(ListItemComponent); + component = fixture.componentInstance; + workflowPersistService = TestBed.inject(WorkflowPersistService) as jasmine.SpyObj; + nzModalService = TestBed.inject(NzModalService) as jasmine.SpyObj; + }); + + it("should update workflow name successfully", () => { + const newName = "New Workflow Name"; + component.entry = { id: 1, name: "Old Name" } as any; + workflowPersistService.updateWorkflowName.and.returnValue(of({} as Response)); + + component.confirmUpdateWorkflowCustomName(newName); + + expect(workflowPersistService.updateWorkflowName).toHaveBeenCalledWith(1, newName); + expect(component.entry.name).toBe(newName); + expect(component.editingName).toBeFalse(); + }); + + it("should handle error when updating workflow name", () => { + const newName = "New Workflow Name"; + component.entry = { id: 1, name: "Old Name" } as any; + component.originalName = "Old Name"; + workflowPersistService.updateWorkflowName.and.returnValue(throwError(() => new Error("Error"))); + + component.confirmUpdateWorkflowCustomName(newName); + + expect(workflowPersistService.updateWorkflowName).toHaveBeenCalledWith(1, newName); + expect(component.entry.name).toBe("Old Name"); + expect(component.editingName).toBeFalse(); + }); + + it("should update workflow description successfully", () => { + const newDescription = "New Description"; + component.entry = { id: 1, description: "Old Description" } as any; + workflowPersistService.updateWorkflowDescription.and.returnValue(of({} as Response)); + + component.confirmUpdateWorkflowCustomDescription(newDescription); + + expect(workflowPersistService.updateWorkflowDescription).toHaveBeenCalledWith(1, newDescription); + expect(component.entry.description).toBe(newDescription); + expect(component.editingDescription).toBeFalse(); + }); + + it("should handle error when updating workflow description", () => { + const newDescription = "New Description"; + component.entry = { id: 1, description: "Old Description" } as any; + component.originalDescription = "Old Description"; + workflowPersistService.updateWorkflowDescription.and.returnValue(throwError(() => new Error("Error"))); + + component.confirmUpdateWorkflowCustomDescription(newDescription); + + expect(workflowPersistService.updateWorkflowDescription).toHaveBeenCalledWith(1, newDescription); + expect(component.entry.description).toBe("Old Description"); + expect(component.editingDescription).toBeFalse(); + }); +}); diff --git a/core/gui/src/app/dashboard/component/user/list-item/list-item.component.ts b/core/gui/src/app/dashboard/component/user/list-item/list-item.component.ts index 480f4c3f9b9..897a779bb43 100644 --- a/core/gui/src/app/dashboard/component/user/list-item/list-item.component.ts +++ b/core/gui/src/app/dashboard/component/user/list-item/list-item.component.ts @@ -32,6 +32,8 @@ import { formatSize } from "src/app/common/util/size-formatter.util"; }) export class ListItemComponent implements OnInit, OnChanges { private owners: number[] = []; + public originalName: string = ""; + public originalDescription: string | undefined = undefined; @Input() currentUid: number | undefined; @ViewChild("nameInput") nameInput!: ElementRef; @ViewChild("descriptionInput") descriptionInput!: ElementRef; @@ -159,6 +161,7 @@ export class ListItemComponent implements OnInit, OnChanges { }; onEditName(): void { + this.originalName = this.entry.name; this.editingName = true; setTimeout(() => { if (this.nameInput) { @@ -171,6 +174,7 @@ export class ListItemComponent implements OnInit, OnChanges { } onEditDescription(): void { + this.originalDescription = this.entry.description; this.editingDescription = true; setTimeout(() => { if (this.descriptionInput) { @@ -182,30 +186,56 @@ export class ListItemComponent implements OnInit, OnChanges { }, 0); } - public confirmUpdateWorkflowCustomName(name: string): void { - this.workflowPersistService - .updateWorkflowName(this.entry.id, name || DEFAULT_WORKFLOW_NAME) + private updateWorkflowProperty( + updateMethod: (id: number | undefined, value: string) => any, + propertyName: "name" | "description", + newValue: string, + originalValue: string | undefined + ): void { + updateMethod(this.entry.id, newValue) .pipe(untilDestroyed(this)) - .subscribe(() => { - this.entry.name = name || DEFAULT_WORKFLOW_NAME; - }) - .add(() => { - this.editingName = false; + .subscribe({ + next: () => { + this.entry[propertyName] = newValue; + }, + error: (err: unknown) => { + console.error(`Failed to update workflow ${propertyName}:`, err); + // Use a fallback empty string if originalValue is undefined + this.entry[propertyName] = originalValue ?? ""; + this.setEditingState(propertyName, false); + }, + complete: () => { + this.setEditingState(propertyName, false); + }, }); } - public confirmUpdateWorkflowCustomDescription(description: string | undefined): void { - const updatedDescription = description !== undefined ? description : ""; + private setEditingState(propertyName: "name" | "description", state: boolean): void { + if (propertyName === "name") { + this.editingName = state; + } else if (propertyName === "description") { + this.editingDescription = state; + } + } - this.workflowPersistService - .updateWorkflowDescription(this.entry.id, updatedDescription) - .pipe(untilDestroyed(this)) - .subscribe(() => { - this.entry.description = updatedDescription; - }) - .add(() => { - this.editingDescription = false; - }); + public confirmUpdateWorkflowCustomName(name: string): void { + const workflowName = name || DEFAULT_WORKFLOW_NAME; + this.updateWorkflowProperty( + this.workflowPersistService.updateWorkflowName.bind(this.workflowPersistService), + "name", + workflowName, + this.originalName + ); + } + + public confirmUpdateWorkflowCustomDescription(description: string | undefined): void { + const updatedDescription = description ?? ""; + this.updateWorkflowProperty( + this.workflowPersistService.updateWorkflowDescription.bind(this.workflowPersistService), + "description", + updatedDescription, + this.originalDescription + ); } formatTime(timestamp: number | undefined): string {