Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
Original file line number Diff line number Diff line change
Expand Up @@ -221,6 +221,45 @@ describe("WorkflowPersistService", () => {
expect(result?.isPublished).toBe(1);
});

it("sends saves one at a time, in order, each caller getting its own result", () => {
// Two saves in flight at once can land out of order and the older content would win; the
// autosave and a Save or a view switch are independent callers, so the ordering lives here.
const wf = (name: string) => ({ wid: 9, name, description: "", content: validContent }) as unknown as Workflow;
const seen: string[] = [];
service.persistWorkflow(wf("first")).subscribe(w => seen.push("first:" + w.name));
service.persistWorkflow(wf("second")).subscribe(w => seen.push("second:" + w.name));

// Only the first request has gone out; the second waits for it.
const first = httpTestingController.expectOne(`${API}/${WORKFLOW_PERSIST_URL}`);
expect(first.request.body.name).toBe("first");
expect(httpTestingController.match(`${API}/${WORKFLOW_PERSIST_URL}`)).toHaveLength(0);

first.flush({ wid: 9, name: "first", content: "{}" });
const second = httpTestingController.expectOne(`${API}/${WORKFLOW_PERSIST_URL}`);
expect(second.request.body.name).toBe("second");
second.flush({ wid: 9, name: "second", content: "{}" });

expect(seen).toEqual(["first:first", "second:second"]);
});

it("fails only its own caller when a save fails, and still sends the next", () => {
const wf = (name: string) => ({ wid: 9, name, description: "", content: validContent }) as unknown as Workflow;
let firstError: unknown;
let secondName: string | undefined;
service.persistWorkflow(wf("first")).subscribe({ error: (e: unknown) => (firstError = e) });
service.persistWorkflow(wf("second")).subscribe(w => (secondName = w.name));

httpTestingController
.expectOne(`${API}/${WORKFLOW_PERSIST_URL}`)
.flush("boom", { status: 500, statusText: "Server Error" });
expect(firstError).toBeDefined();

httpTestingController
.expectOne(`${API}/${WORKFLOW_PERSIST_URL}`)
.flush({ wid: 9, name: "second", content: "{}" });
expect(secondName).toBe("second");
});

it("persistWorkflow notifies the user when the workflow is broken but still POSTs", () => {
const errorSpy = vi.spyOn(notificationService, "error").mockImplementation(() => {});
const workflow = {
Expand Down Expand Up @@ -274,6 +313,24 @@ describe("WorkflowPersistService", () => {
expect(result).toEqual(created);
});

it("createWorkflow sends the default view when given one, and omits it otherwise", () => {
const content = jsonCast<WorkflowContent>(testContent);

service.createWorkflow(content, "form default", DefaultView.FORM).subscribe();
const withView = httpTestingController.expectOne(`${API}/${WORKFLOW_CREATE_URL}`);
expect(withView.request.body).toEqual({
name: "form default",
content: JSON.stringify(content),
defaultView: DefaultView.FORM,
});
withView.flush({ workflow: { wid: 1 } } as unknown as DashboardWorkflow);

service.createWorkflow(content, "no view").subscribe();
const withoutView = httpTestingController.expectOne(`${API}/${WORKFLOW_CREATE_URL}`);
expect(withoutView.request.body).toEqual({ name: "no view", content: JSON.stringify(content) });
withoutView.flush({ workflow: { wid: 2 } } as unknown as DashboardWorkflow);
});

it("createWorkflow filters out a null response so no value is emitted", () => {
let emitted = false;
service.createWorkflow(jsonCast<WorkflowContent>(testContent)).subscribe(() => (emitted = true));
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -19,8 +19,8 @@

import { HttpClient, HttpParams } from "@angular/common/http";
import { Injectable } from "@angular/core";
import { Observable, throwError } from "rxjs";
import { catchError, filter, map } from "rxjs/operators";
import { EMPTY, Observable, ReplaySubject, Subject, throwError } from "rxjs";
import { catchError, concatMap, filter, map, tap } from "rxjs/operators";
import { AppSettings } from "../../app-setting";
import { Workflow, WorkflowContent } from "../../type/workflow";
import { DashboardWorkflow } from "../../../dashboard/type/dashboard-workflow.interface";
Expand Down Expand Up @@ -59,13 +59,42 @@ export class WorkflowPersistService {
// flag to disable workflow persist when displaying the read only particular version
private workflowPersistFlag = true;

/**
* Saves, one at a time and in call order. Two saves in flight at once can reach the backend out
* of order, and then the older content wins: the canvas's autosave (debounced) and a Save or a
* view switch are independent requests, and the Form View's own queue only orders that page's
* saves. Ordering them here, at the one place every save goes through, covers all of them and
* lets a caller that hands over on completion (the view switches) know that everything asked for
* before it has landed too. Each request snapshots its payload when asked for; it is sent when its
* turn comes, and its outcome is relayed to that caller alone. A failed save fails its own caller
* and does not hold up the next.
*/
private readonly persistQueue = new Subject<{ send: Observable<Workflow>; result: Subject<Workflow> }>();

constructor(
private http: HttpClient,
private notificationService: NotificationService
) {}
) {
this.persistQueue
.pipe(
concatMap(({ send, result }) =>
send.pipe(
tap({
next: updated => result.next(updated),
error: (err: unknown) => result.error(err),
complete: () => result.complete(),
}),
catchError(() => EMPTY)
)
)
)
.subscribe();
}

/**
* persists a workflow to backend database and returns its updated information (e.g., new wid)
* persists a workflow to backend database and returns its updated information (e.g., new wid).
* The request is queued behind any save still in flight (see persistQueue); the returned
* observable completes once this save has come back.
* @param workflow
*/
public persistWorkflow(workflow: Workflow): Observable<Workflow> {
Expand All @@ -79,7 +108,7 @@ export class WorkflowPersistService {
// backend does not read it on this endpoint (publishing goes through /public and /private),
// and it is not reliably known here anyway, since the metadata fed back after a save names
// it differently (see WorkflowUtilService.parseWorkflowInfo).
return this.http
const send = this.http
.post<Workflow>(`${AppSettings.getApiEndpoint()}/${WORKFLOW_PERSIST_URL}`, {
wid: workflow.wid,
name: workflow.name,
Expand All @@ -90,6 +119,11 @@ export class WorkflowPersistService {
filter((updatedWorkflow: Workflow) => updatedWorkflow != null),
map(WorkflowUtilService.parseWorkflowInfo)
);
// Replayed, so a caller that subscribes after the queue has already relayed the outcome (a
// save that was quick, or a synchronous test double) still receives it.
const result = new ReplaySubject<Workflow>(1);
this.persistQueue.next({ send, result });
return result.asObservable();
}

/**
Expand All @@ -99,12 +133,16 @@ export class WorkflowPersistService {
*/
public createWorkflow(
newWorkflowContent: WorkflowContent,
newWorkflowName: string = DEFAULT_WORKFLOW_NAME
newWorkflowName: string = DEFAULT_WORKFLOW_NAME,
defaultView?: DefaultView
): Observable<DashboardWorkflow> {
return this.http
.post<DashboardWorkflow>(`${AppSettings.getApiEndpoint()}/${WORKFLOW_CREATE_URL}`, {
name: newWorkflowName,
content: JSON.stringify(newWorkflowContent),
// Bound onto the workflow row on the server, so an uploaded form-default workflow
// still opens as a form. Omitted (server default CANVAS) when the file carries none.
...(defaultView === undefined ? {} : { defaultView }),
})
.pipe(filter((createdWorkflow: DashboardWorkflow) => createdWorkflow != null));
}
Expand Down
16 changes: 15 additions & 1 deletion frontend/src/app/common/type/workflow.ts
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@
* under the License.
*/

import { WorkflowMetadata } from "../../dashboard/type/workflow-metadata.interface";
import { DefaultView, WorkflowMetadata } from "../../dashboard/type/workflow-metadata.interface";
import { CommentBox, OperatorLink, OperatorPredicate, Point } from "../../workspace/types/workflow-common.interface";

export enum ExecutionMode {
Expand Down Expand Up @@ -104,3 +104,17 @@ export interface WorkflowContent
}> {}

export type Workflow = { content: WorkflowContent } & WorkflowMetadata;

/**
* The JSON a workflow is exported as, from the dashboard download and the canvas menu alike: the
* content plus, when the workflow has one, the landing view as one extra top-level key next to the
* content's own (operators/links/...). The importer (upload) destructures it back out onto the
* workflow row, so a download-then-upload keeps a form-default workflow opening as a form; an
* older importer that reads the whole object as content simply ignores the unknown key, and an
* older export without it imports unchanged.
*/
export type ExportedWorkflow = WorkflowContent & { defaultView?: DefaultView };

export function exportedWorkflow(content: WorkflowContent, defaultView: DefaultView | undefined): ExportedWorkflow {
return defaultView === undefined ? content : { ...content, defaultView };
}
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,7 @@
[(ngModel)]="entry.checked"
(ngModelChange)="onCheckboxChange(entry)"></label>
</div>
<!-- Cover-image controls -->
<!-- Cover image controls (owner only) -->
<div
class="card-image-controls"
*ngIf="canEditCover"
Expand All @@ -52,7 +52,7 @@
nzType="camera"></i>
</button>
<button
*ngIf="hasCustomImage"
*ngIf="canEditCover && hasCustomImage"
nz-button
nzType="text"
class="image-control-btn"
Expand Down Expand Up @@ -204,6 +204,24 @@
nz-icon
nzType="eye"></i>
</button>
<!-- The default-view toggle, in the same slot as on the list row (after Detail), for anyone
with write access. A toggle button: constant name, state in aria-pressed (a name that
changed with the state would announce the opposite of what the state says); the title
spells out what a click does. -->
<button
nz-button
nzType="text"
class="action-btn default-view-toggle"
*ngIf="canToggleDefaultView"
aria-label="Open in the Form View by default"
[attr.aria-pressed]="defaultsToForm"
[class.defaults-to-form-on]="defaultsToForm"
[title]="defaultsToForm ? 'Open on the canvas by default' : 'Open in the Form View by default'"
(click)="onToggleDefaultView(); $event.stopPropagation()">
<i
nz-icon
nzType="solution"></i>
</button>
<button
nz-button
nzType="text"
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -232,6 +232,18 @@
color: #1f1f1f;
background: #f0f0f0;
}

/* The default-view toggle shows its state, the same cue the list row's toggle gives: the accent
while the workflow opens in the Form View. */
&.default-view-toggle.defaults-to-form-on {
color: #1e90ff;
background: #e6f7ff;

&:hover {
color: #1e90ff;
background: #cceeff;
}
}
}

.like-btn {
Expand Down
Loading
Loading