Ma77Ball commented on code in PR #5704:
URL: https://github.com/apache/texera/pull/5704#discussion_r3463589673


##########
frontend/src/app/dashboard/component/user/list-item/card-item/card-item.component.ts:
##########
@@ -150,6 +207,13 @@ export class CardItemComponent implements OnChanges {
           this.entryLink = [HUB_WORKFLOW_RESULT_DETAIL, String(this.entry.id)];
         }
         this.size = this.entry.size;
+        this.workflowCoverService
+          .getCover(this.entry.id)
+          .pipe(untilDestroyed(this))
+          .subscribe(image => {
+            this.customImage = image;
+            this.cdr.markForCheck();
+          });

Review Comment:
   Fixed the flash + ordering: cover fetch now goes through a single `Subject` 
+ `switchMap`, with `customImage` cleared eagerly so recycled cards don't flash 
a stale cover and late responses are cancelled. The N+1 fetch volume needs a 
`hasCover` flag in the list payload, which touches the list query, so I'll do 
it as a follow-up issue and link it here.



##########
frontend/src/app/dashboard/component/user/list-item/card-item/card-item.component.ts:
##########
@@ -131,12 +136,64 @@ export class CardItemComponent implements OnChanges {
     private hubService: HubService,
     private downloadService: DownloadService,
     private cdr: ChangeDetectorRef,
-    private notificationService: NotificationService
+    private notificationService: NotificationService,
+    private workflowCoverService: WorkflowCoverService
   ) {}
 
-  /** The top image src for the card preview. */
+  /** The top image src: the user's custom cover if present, otherwise the 
default. */
   get previewImage(): string {
-    return CardItemComponent.DEFAULT_PREVIEW_IMAGE;
+    return this.customImage ?? CardItemComponent.DEFAULT_PREVIEW_IMAGE;
+  }
+
+  get hasCustomImage(): boolean {
+    return this.customImage !== undefined;
+  }
+
+  /** Whether the cover-image controls are shown: an editable workflow in 
private search. */
+  get canEditCover(): boolean {
+    return this.isPrivateSearch && this.entry.type === "workflow" && 
this.entry.workflow.isOwner;
+  }
+
+  openImagePicker(): void {
+    this.backgroundInput?.nativeElement.click();
+  }
+
+  async onImageSelected(event: Event): Promise<void> {
+    const input = event.target as HTMLInputElement;
+    const file = input.files?.[0];
+    input.value = ""; // allow re-selecting the same file
+    if (!file) {
+      return;
+    }
+    if (!file.type.startsWith("image/")) {
+      this.notificationService.error("Please choose an image file.");
+      return;
+    }
+    if (typeof this.entry.id !== "number") {
+      return;
+    }
+    try {
+      this.customImage = await 
this.workflowCoverService.setCoverFromFile(this.entry.id, file);
+      this.cdr.markForCheck();
+    } catch (e) {
+      this.notificationService.error("Failed to set the cover image.");
+    }
+  }

Review Comment:
   Done. `onImageSelected` now uses Observable + `untilDestroyed` like 
`resetImage` (`setCoverFromFile` returns an Observable now), so it won't touch 
a destroyed component mid-upload.



##########
frontend/src/app/dashboard/service/user/workflow-cover/workflow-cover.service.ts:
##########
@@ -0,0 +1,87 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *   http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied.  See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+import { HttpClient } from "@angular/common/http";
+import { Injectable } from "@angular/core";
+import { Observable, firstValueFrom, of } from "rxjs";
+import { catchError, map } from "rxjs/operators";
+import { AppSettings } from "../../../../common/app-setting";
+
+export const WORKFLOW_COVER_URL = "workflow";
+
+/** Longest edge (px) a custom cover image is downscaled to before being 
stored. */
+const MAX_IMAGE_EDGE = 640;
+/** JPEG quality used when re-encoding a custom cover image for storage. */
+const IMAGE_QUALITY = 0.8;
+
+/** Stores an optional custom cover image per workflow on the backend, 
downscaled and re-encoded as a JPEG data URL. */
+@Injectable({
+  providedIn: "root",
+})
+export class WorkflowCoverService {
+  constructor(private http: HttpClient) {}
+
+  /** The workflow's custom cover image data URL, or undefined if it has none. 
*/
+  getCover(wid: number): Observable<string | undefined> {
+    return this.http.get<{ image: string 
}>(`${AppSettings.getApiEndpoint()}/${WORKFLOW_COVER_URL}/${wid}/cover`).pipe(
+      map(response => response.image),
+      catchError(() => of(undefined))

Review Comment:
   Fixed. Only 404 maps to `undefined` now; other errors propagate, and the 
card logs them instead of silently showing the default.



##########
amber/src/main/scala/org/apache/texera/web/resource/dashboard/user/workflow/WorkflowResource.scala:
##########
@@ -711,6 +730,75 @@ class WorkflowResource extends LazyLogging {
     workflowDao.update(workflow)
   }
 
+  /** Returns the workflow's cover image; 404 if none set. */
+  @GET
+  @RolesAllowed(Array("REGULAR", "ADMIN"))
+  @Path("/{wid}/cover")
+  def getCoverImage(@PathParam("wid") wid: Integer, @Auth user: SessionUser): 
CoverImageRequest = {

Review Comment:
   Done. Added a separate `CoverImageResponse`; `CoverImageRequest` is now 
PUT-only.



##########
amber/src/main/scala/org/apache/texera/web/resource/dashboard/user/workflow/WorkflowResource.scala:
##########
@@ -711,6 +730,75 @@ class WorkflowResource extends LazyLogging {
     workflowDao.update(workflow)
   }
 
+  /** Returns the workflow's cover image; 404 if none set. */
+  @GET
+  @RolesAllowed(Array("REGULAR", "ADMIN"))
+  @Path("/{wid}/cover")
+  def getCoverImage(@PathParam("wid") wid: Integer, @Auth user: SessionUser): 
CoverImageRequest = {
+    if (!WorkflowAccessResource.hasReadAccess(wid, user.getUid)) {
+      throw new ForbiddenException(s"You do not have access to workflow $wid")
+    }
+    val image = context
+      .select(WORKFLOW_COVER_IMAGE.IMAGE)
+      .from(WORKFLOW_COVER_IMAGE)
+      .where(WORKFLOW_COVER_IMAGE.WID.eq(wid))
+      .fetchOne(WORKFLOW_COVER_IMAGE.IMAGE)
+    if (image == null) {
+      throw new NotFoundException(s"Workflow $wid has no cover image")
+    }
+    CoverImageRequest(image)
+  }
+
+  /** Sets or replaces the workflow's cover image. */
+  @PUT
+  @RolesAllowed(Array("REGULAR", "ADMIN"))
+  @Path("/{wid}/cover")
+  def setCoverImage(
+      @PathParam("wid") wid: Integer,
+      request: CoverImageRequest,
+      @Auth user: SessionUser
+  ): Unit = {
+    if (!WorkflowAccessResource.hasWriteAccess(wid, user.getUid)) {

Review Comment:
   Intentional. The cover is per-workflow metadata, authorized like 
`makePublic`/`updateWorkflowName`/`updateWorkflowDescription` (all 
`hasWriteAccess`). Owner-only UI is just where we surface the control, not a 
security boundary. Per-user covers would be a separate feature.



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]

Reply via email to