You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@texera.apache.org by "Ma77Ball (via GitHub)" <gi...@apache.org> on 2026/06/14 00:13:49 UTC
[PR] feat(dashboard): add custom cover image for workflow cards [texera]
Ma77Ball opened a new pull request, #5704:
URL: https://github.com/apache/texera/pull/5704
### What changes were proposed in this PR?
- Add a `workflow_cover_image(wid, image)` table (cover stored as a downscaled JPEG data URL, FK to `workflow` with `ON DELETE CASCADE`) in `texera_ddl.sql` plus a `sql/updates/25.sql` migration.
- Add access-gated `GET`/`PUT`/`DELETE /workflow/{wid}/cover` endpoints on `WorkflowResource`: read access to fetch (404 when unset), write access to set/delete, with data-URL and size validation on set.
- Add `WorkflowCoverService` (frontend) that downscales/re-encodes the chosen file to a small JPEG data URL before upload, fetches the cover, and clears it.
- Add hover controls on the workflow card (camera to pick an image, rollback to reset), shown only to the owner in private search; `previewImage` now prefers the stored cover.
### Any related issues, documentation, discussions?
Closes: #5703
### How was this PR tested?
- Run `cd frontend && yarn test --include='**/workflow-cover.service.spec.ts' --watch=false`, expect the WorkflowCoverService specs to pass (GET maps the image, 404 resolves to undefined, DELETE is issued).
- Manual: open the dashboard in private search as a workflow owner, hover a card, click the camera icon, pick an image, expect the card preview to update; reload in another browser and expect the same cover (synced via backend).
- Manual: click the rollback icon, expect the card to revert to the default background and the cover to be gone after reload.
- Note: the three new `WorkflowResource` cover endpoints have no automated backend test (no DB-backed test harness exists for these jOOQ endpoints); they were exercised manually only.
### Was this PR authored or co-authored using generative AI tooling?
Co-authored with Claude Opus 4.8 in compliance with ASF
--
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: notifications-unsubscribe@texera.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] feat(dashboard): add custom cover image for workflow cards [texera]
Posted by "Ma77Ball (via GitHub)" <gi...@apache.org>.
Ma77Ball commented on PR #5704:
URL: https://github.com/apache/texera/pull/5704#issuecomment-4858875742
@Yicong-Huang or @aglinxinyuan, please merge as I fixed all the merge conflicts.
--
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: notifications-unsubscribe@texera.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] feat(dashboard): add custom cover image for workflow cards [texera]
Posted by "aglinxinyuan (via GitHub)" <gi...@apache.org>.
aglinxinyuan commented on code in PR #5704:
URL: https://github.com/apache/texera/pull/5704#discussion_r3457214123
##########
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:
Non-blocking — this path is promise/`await` with no `untilDestroyed`, whereas `resetImage` uses the Observable + `untilDestroyed` pattern. If the card is destroyed mid-upload we'd still touch `customImage`/`markForCheck()` on a dead component. Worth aligning the two for consistency.
##########
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:
Tiny nit — `CoverImageRequest` is doubling as the GET response type here. A `CoverImageResponse` (or a neutral `CoverImage`) would read a bit cleaner.
##########
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:
Worth confirming intent: set/delete authorize on `hasWriteAccess`, so any WRITE collaborator can change the cover — and since it's stored per-workflow, that changes it for everyone — while the UI only exposes the controls to the owner (`canEditCover`). The backend being broader than the UI is fine; I just want to make sure a shared, collaborator-editable cover is what #5703 intends (vs. per-user).
##########
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:
`getCover` fires here once per workflow card inside `initializeEntry`, so a dashboard/hub page of N cards issues N separate `GET /{wid}/cover` calls — each a DB hit + access check — re-fired on every `ngOnChanges`, with no caching. That'll scale poorly on a busy dashboard. Could we fold the cover (or just a `hasCover` flag to gate a lazy fetch) into the existing workflow-list/search payload instead?
Minor, same block: `customImage` isn't reset before the async result lands, so when Angular reuses a card instance for a different entry the previous cover flashes until the new fetch resolves — and overlapping `ngOnChanges` subscriptions can resolve out of order. A `switchMap` would address both.
##########
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:
Minor — this swallows every error to `undefined`, not just 404, so a transient 500 silently shows the default cover with no signal to the user. Fine if intentional; just flagging.
--
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: notifications-unsubscribe@texera.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] feat(dashboard): add custom cover image for workflow cards [texera]
Posted by "Ma77Ball (via GitHub)" <gi...@apache.org>.
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: notifications-unsubscribe@texera.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] feat(dashboard): add custom cover image for workflow cards [texera]
Posted by "xuang7 (via GitHub)" <gi...@apache.org>.
xuang7 commented on code in PR #5704:
URL: https://github.com/apache/texera/pull/5704#discussion_r3509359665
##########
frontend/src/app/dashboard/component/user/list-item/card-item/card-item.component.ts:
##########
@@ -131,9 +136,68 @@ 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: the user's custom cover if present, otherwise the default. */
+ get previewImage(): string {
Review Comment:
The previewImage getter appears to be unused outside of the test cases. Is it still needed?
--
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: notifications-unsubscribe@texera.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] feat(dashboard): add custom cover image for workflow cards [texera]
Posted by "aglinxinyuan (via GitHub)" <gi...@apache.org>.
aglinxinyuan commented on PR #5704:
URL: https://github.com/apache/texera/pull/5704#issuecomment-4814213629
Please resolve the conflicts, and we can merge.
--
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: notifications-unsubscribe@texera.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] feat(dashboard): add custom cover image for workflow cards [texera]
Posted by "aicam (via GitHub)" <gi...@apache.org>.
aicam merged PR #5704:
URL: https://github.com/apache/texera/pull/5704
--
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: notifications-unsubscribe@texera.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] feat(dashboard): add custom cover image for workflow cards [texera]
Posted by "Yicong-Huang (via GitHub)" <gi...@apache.org>.
Yicong-Huang commented on PR #5704:
URL: https://github.com/apache/texera/pull/5704#issuecomment-4848598884
@aglinxinyuan, is this PR ready for merge?
--
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: notifications-unsubscribe@texera.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] feat(dashboard): add custom cover image for workflow cards [texera]
Posted by "Ma77Ball (via GitHub)" <gi...@apache.org>.
Ma77Ball commented on PR #5704:
URL: https://github.com/apache/texera/pull/5704#issuecomment-4701686341
/request-review @xuang7
--
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: notifications-unsubscribe@texera.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] feat(dashboard): add custom cover image for workflow cards [texera]
Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on PR #5704:
URL: https://github.com/apache/texera/pull/5704#issuecomment-4700198608
<!-- texera-benchmarks-comment -->
## ⚠️ Benchmark changes need a look
🟢 13 better · 🔴 2 worse · ⚪ 0 noise (<±5%) · 0 without baseline
> CI benchmark results are noisy; treat <±5% as noise unless repeated.
[Dashboard](https://apache.github.io/texera/dev/bench/) · [Run](https://github.com/apache/texera/actions/runs/27483205731)
| | config | throughput | MB/s | latency | max Δ latest / 7d |
|---|---|---:|---:|---:|---:|
| 🔴 | bs=10 sw=10 sl=64 | 540 | 0.329 | 17,759/37,485/37,485 us | 🟢 +30.3% / 🟢 +31.4% |
| 🟢 | bs=100 sw=10 sl=64 | 1,222 | 0.746 | 83,020/102,348/102,348 us | 🟢 +51.9% / 🟢 +37.0% |
| 🟢 | bs=1000 sw=10 sl=64 | 1,445 | 0.882 | 691,877/728,792/728,792 us | 🟢 +55.5% / 🟢 +38.8% |
<details><summary>Baseline details</summary>
Latest main `bdde6b9` from 2026-06-13T23:51:59.624Z
| config | metric | PR | latest main | 7d avg | Δ latest | Δ 7d |
|---|---|---:|---:|---:|---:|---:|
| bs=10 sw=10 sl=64 | throughput | 540 tuples/sec | 414.33 tuples/sec | 410.82 tuples/sec | +30.3% | +31.4% |
| bs=10 sw=10 sl=64 | MB/s | 0.329 MB/s | 0.253 MB/s | 0.251 MB/s | +30.1% | +31.2% |
| bs=10 sw=10 sl=64 | p50 | 17,759 us | 23,391 us | 23,785 us | -24.1% | -25.3% |
| bs=10 sw=10 sl=64 | p95 | 37,485 us | 29,323 us | 34,980 us | +27.8% | +7.2% |
| bs=10 sw=10 sl=64 | p99 | 37,485 us | 29,323 us | 34,980 us | +27.8% | +7.2% |
| bs=100 sw=10 sl=64 | throughput | 1,222 tuples/sec | 804.8 tuples/sec | 891.94 tuples/sec | +51.8% | +37.0% |
| bs=100 sw=10 sl=64 | MB/s | 0.746 MB/s | 0.491 MB/s | 0.544 MB/s | +51.9% | +37.0% |
| bs=100 sw=10 sl=64 | p50 | 83,020 us | 124,000 us | 112,277 us | -33.0% | -26.1% |
| bs=100 sw=10 sl=64 | p95 | 102,348 us | 145,811 us | 139,802 us | -29.8% | -26.8% |
| bs=100 sw=10 sl=64 | p99 | 102,348 us | 145,811 us | 139,802 us | -29.8% | -26.8% |
| bs=1000 sw=10 sl=64 | throughput | 1,445 tuples/sec | 929.58 tuples/sec | 1,041 tuples/sec | +55.4% | +38.8% |
| bs=1000 sw=10 sl=64 | MB/s | 0.882 MB/s | 0.567 MB/s | 0.635 MB/s | +55.5% | +38.8% |
| bs=1000 sw=10 sl=64 | p50 | 691,877 us | 1,073,040 us | 972,714 us | -35.5% | -28.9% |
| bs=1000 sw=10 sl=64 | p95 | 728,792 us | 1,136,639 us | 1,023,057 us | -35.9% | -28.8% |
| bs=1000 sw=10 sl=64 | p99 | 728,792 us | 1,136,639 us | 1,023,057 us | -35.9% | -28.8% |
</details>
<details><summary>Raw CSV</summary>
```csv
config_idx,batch_size,schema_width,string_len,num_batches,total_ms,total_tuples,total_bytes,tuples_per_sec,mb_per_sec,lat_p50_us,lat_p95_us,lat_p99_us
0,10,10,64,20,370.57,200,128000,540,0.329,17759.33,37484.50,37484.50
1,100,10,64,20,1636.27,2000,1280000,1222,0.746,83020.24,102348.33,102348.33
2,1000,10,64,20,13839.58,20000,12800000,1445,0.882,691877.22,728792.00,728792.00
```
</details>
--
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: notifications-unsubscribe@texera.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] feat(dashboard): add custom cover image for workflow cards [texera]
Posted by "Ma77Ball (via GitHub)" <gi...@apache.org>.
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:
##########
@@ -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:
Follow-up issue: #5920
--
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: notifications-unsubscribe@texera.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] feat(dashboard): add custom cover image for workflow cards [texera]
Posted by "Copilot (via GitHub)" <gi...@apache.org>.
Copilot commented on code in PR #5704:
URL: https://github.com/apache/texera/pull/5704#discussion_r3448008004
##########
amber/src/main/scala/org/apache/texera/web/resource/dashboard/user/workflow/WorkflowResource.scala:
##########
@@ -711,6 +717,73 @@ 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)) {
+ throw new ForbiddenException(s"You do not have permission to modify workflow $wid")
+ }
+ val image = Option(request.image).map(_.trim).getOrElse("")
Review Comment:
`setCoverImage` dereferences `request.image` without guarding against a null request entity. If the client sends an empty body (or fails JSON deserialization), Jersey can pass `null` for `request`, which will trigger a NullPointerException instead of a 400.
##########
amber/src/main/scala/org/apache/texera/web/resource/dashboard/user/workflow/WorkflowResource.scala:
##########
@@ -711,6 +717,73 @@ 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)) {
+ throw new ForbiddenException(s"You do not have permission to modify workflow $wid")
+ }
+ val image = Option(request.image).map(_.trim).getOrElse("")
+ if (image.isEmpty) {
+ throw new BadRequestException("Cover image is required")
+ }
+ if (!image.startsWith("data:image/")) {
+ throw new BadRequestException("Cover image must be an image data URL")
+ }
Review Comment:
Validation currently permits any `data:image/*` URL. Since this value is later rendered into an `<img src>`, it should at least reject SVG (and other non-raster types) and require the base64 form. A small MIME allowlist reduces stored-XSS surface while remaining compatible with common image uploads.
##########
common/workflow-operator/src/test/scala/org/apache/texera/amber/operator/huggingFace/codegen/ImageTaskCodegenSpec.scala:
##########
@@ -0,0 +1,214 @@
+/*
+ * 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.
+ */
+
+package org.apache.texera.amber.operator.huggingFace.codegen
+
+import org.apache.texera.amber.pybuilder.PyStringTypes.EncodableString
+import org.scalatest.flatspec.AnyFlatSpec
+import org.scalatest.matchers.should.Matchers
+
+class ImageTaskCodegenSpec extends AnyFlatSpec with Matchers {
+
Review Comment:
This PR is titled/described as a dashboard workflow-card feature, but this new spec file tests HuggingFace image task codegen and doesn't appear related to workflow cover images. Consider moving it to a separate PR or updating the PR description to explain why it's included here.
##########
frontend/src/app/dashboard/component/user/list-item/card-item/card-item.component.html:
##########
@@ -36,6 +36,39 @@
[(ngModel)]="entry.checked"
(ngModelChange)="onCheckboxChange(entry)"></label>
</div>
+ <!-- Cover-image controls -->
+ <div
+ class="card-image-controls"
+ *ngIf="canEditCover"
+ (click)="$event.stopPropagation()">
+ <button
+ nz-button
+ nzType="text"
+ class="image-control-btn"
+ title="Change cover image"
+ (click)="openImagePicker()">
+ <i
+ nz-icon
+ nzType="camera"></i>
+ </button>
+ <button
+ *ngIf="hasCustomImage"
+ nz-button
+ nzType="text"
+ class="image-control-btn"
+ title="Reset to default image"
+ (click)="resetImage()">
+ <i
+ nz-icon
+ nzType="rollback"></i>
+ </button>
Review Comment:
The cover-image icon-only buttons rely on `title`, which is not a reliable accessible name for screen readers. Adding `aria-label` (and explicitly setting `type="button"`) makes the controls accessible and avoids any unintended form-submit behavior if this markup is ever placed inside a form.
##########
frontend/src/app/dashboard/component/user/list-item/card-item/card-item.component.scss:
##########
@@ -69,6 +69,36 @@
z-index: 10;
}
+// Cover-image controls, revealed on hover.
+.card-image-controls {
+ position: absolute;
+ bottom: 4px;
+ right: 4px;
+ z-index: 11;
+ display: flex;
+ gap: 4px;
+ opacity: 0;
+ transition: opacity 0.2s;
+}
+
+.card-item:hover .card-image-controls {
+ opacity: 1;
+}
Review Comment:
The cover controls are hidden via `opacity: 0` only. This leaves an invisible element that can still intercept pointer/keyboard focus, creating a dead zone on the card (and making the buttons focusable but not visible). Consider disabling pointer events while hidden and also revealing on `:focus-within` for keyboard accessibility.
--
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: notifications-unsubscribe@texera.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] feat(dashboard): add custom cover image for workflow cards [texera]
Posted by "codecov-commenter (via GitHub)" <gi...@apache.org>.
codecov-commenter commented on PR #5704:
URL: https://github.com/apache/texera/pull/5704#issuecomment-4700195656
## [Codecov](https://app.codecov.io/gh/apache/texera/pull/5704?dropdown=coverage&src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) Report
:white_check_mark: All modified and coverable lines are covered by tests.
:white_check_mark: Project coverage is 53.19%. Comparing base ([`7ae9b35`](https://app.codecov.io/gh/apache/texera/commit/7ae9b35f12748616daf7bcc925fdde2e5def5187?dropdown=coverage&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache)) to head ([`30d4f9b`](https://app.codecov.io/gh/apache/texera/commit/30d4f9b262a126ed8c6edceda71ab0a7848af4bd?dropdown=coverage&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache)).
:warning: Report is 11 commits behind head on main.
<details><summary>Additional details and impacted files</summary>
```diff
@@ Coverage Diff @@
## main #5704 +/- ##
============================================
- Coverage 53.23% 53.19% -0.05%
+ Complexity 2561 2559 -2
============================================
Files 1075 1075
Lines 42388 42345 -43
Branches 4640 4633 -7
============================================
- Hits 22566 22526 -40
+ Misses 18502 18500 -2
+ Partials 1320 1319 -1
```
| [Flag](https://app.codecov.io/gh/apache/texera/pull/5704/flags?src=pr&el=flags&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | Coverage Δ | | *Carryforward flag |
|---|---|---|---|
| [access-control-service](https://app.codecov.io/gh/apache/texera/pull/5704/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `71.42% <ø> (-3.36%)` | :arrow_down: | |
| [agent-service](https://app.codecov.io/gh/apache/texera/pull/5704/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `34.36% <ø> (ø)` | | Carriedforward from [7ae9b35](https://app.codecov.io/gh/apache/texera/commit/7ae9b35f12748616daf7bcc925fdde2e5def5187?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) |
| [amber](https://app.codecov.io/gh/apache/texera/pull/5704/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `54.20% <ø> (ø)` | | Carriedforward from [7ae9b35](https://app.codecov.io/gh/apache/texera/commit/7ae9b35f12748616daf7bcc925fdde2e5def5187?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) |
| [computing-unit-managing-service](https://app.codecov.io/gh/apache/texera/pull/5704/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `1.65% <ø> (ø)` | | Carriedforward from [7ae9b35](https://app.codecov.io/gh/apache/texera/commit/7ae9b35f12748616daf7bcc925fdde2e5def5187?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) |
| [config-service](https://app.codecov.io/gh/apache/texera/pull/5704/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `56.71% <ø> (-1.86%)` | :arrow_down: | |
| [file-service](https://app.codecov.io/gh/apache/texera/pull/5704/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `57.06% <ø> (ø)` | | Carriedforward from [7ae9b35](https://app.codecov.io/gh/apache/texera/commit/7ae9b35f12748616daf7bcc925fdde2e5def5187?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) |
| [frontend](https://app.codecov.io/gh/apache/texera/pull/5704/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `47.49% <ø> (-0.01%)` | :arrow_down: | Carriedforward from [7ae9b35](https://app.codecov.io/gh/apache/texera/commit/7ae9b35f12748616daf7bcc925fdde2e5def5187?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) |
| [pyamber](https://app.codecov.io/gh/apache/texera/pull/5704/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `90.79% <ø> (ø)` | | Carriedforward from [7ae9b35](https://app.codecov.io/gh/apache/texera/commit/7ae9b35f12748616daf7bcc925fdde2e5def5187?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) |
| [python](https://app.codecov.io/gh/apache/texera/pull/5704/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `90.74% <ø> (ø)` | | Carriedforward from [7ae9b35](https://app.codecov.io/gh/apache/texera/commit/7ae9b35f12748616daf7bcc925fdde2e5def5187?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) |
| [workflow-compiling-service](https://app.codecov.io/gh/apache/texera/pull/5704/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `58.69% <ø> (ø)` | | Carriedforward from [7ae9b35](https://app.codecov.io/gh/apache/texera/commit/7ae9b35f12748616daf7bcc925fdde2e5def5187?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) |
*This pull request uses carry forward flags. [Click here](https://docs.codecov.io/docs/carryforward-flags?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) to find out more.
</details>
[:umbrella: View full report in Codecov by Harness](https://app.codecov.io/gh/apache/texera/pull/5704?dropdown=coverage&src=pr&el=continue&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache).
:loudspeaker: Have feedback on the report? [Share it here](https://about.codecov.io/codecov-pr-comment-feedback/?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache).
<details><summary> :rocket: New features to boost your workflow: </summary>
- :package: [JS Bundle Analysis](https://docs.codecov.com/docs/javascript-bundle-analysis): Save yourself from yourself by tracking and limiting bundle sizes in JS merges.
</details>
--
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: notifications-unsubscribe@texera.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] feat(dashboard): add custom cover image for workflow cards [texera]
Posted by "Ma77Ball (via GitHub)" <gi...@apache.org>.
Ma77Ball commented on PR #5704:
URL: https://github.com/apache/texera/pull/5704#issuecomment-4761198402
/request-review @aglinxinyuan
--
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: notifications-unsubscribe@texera.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] feat(dashboard): add custom cover image for workflow cards [texera]
Posted by "Ma77Ball (via GitHub)" <gi...@apache.org>.
Ma77Ball commented on code in PR #5704:
URL: https://github.com/apache/texera/pull/5704#discussion_r3463591302
##########
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:
Follow-up issue: #5920
--
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: notifications-unsubscribe@texera.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] feat(dashboard): add custom cover image for workflow cards [texera]
Posted by "Ma77Ball (via GitHub)" <gi...@apache.org>.
Ma77Ball commented on code in PR #5704:
URL: https://github.com/apache/texera/pull/5704#discussion_r3464630270
##########
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:
The per-card flash + out-of-order fetch is fixed in this PR (single `Subject` + `switchMap`, with `customImage` cleared eagerly). The remaining fetch-volume concern (fold a `hasCover` flag into the list/search payload so cover-less cards make zero requests) is tracked as a separate follow-on in #5920.
--
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: notifications-unsubscribe@texera.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] feat(dashboard): add custom cover image for workflow cards [texera]
Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on PR #5704:
URL: https://github.com/apache/texera/pull/5704#issuecomment-4766195119
<!-- texera-reviewer-suggestion -->
### Automated Reviewer Suggestions
Based on the `git blame` history of the changed files, we recommend the following reviewers:
- **Contributors with relevant context:** `@SarahAsad23`, `@kunwp1`
You can notify them by mentioning `@SarahAsad23`, `@kunwp1` in a comment.
--
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: notifications-unsubscribe@texera.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] feat(dashboard): add custom cover image for workflow cards [texera]
Posted by "Ma77Ball (via GitHub)" <gi...@apache.org>.
Ma77Ball commented on PR #5704:
URL: https://github.com/apache/texera/pull/5704#issuecomment-4767477863
@xuang7 and @aglinxinyuan, please review the changes and let me know if they look good.
--
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: notifications-unsubscribe@texera.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] feat(dashboard): add custom cover image for workflow cards [texera]
Posted by "Ma77Ball (via GitHub)" <gi...@apache.org>.
Ma77Ball commented on code in PR #5704:
URL: https://github.com/apache/texera/pull/5704#discussion_r3448038966
##########
amber/src/main/scala/org/apache/texera/web/resource/dashboard/user/workflow/WorkflowResource.scala:
##########
@@ -711,6 +717,73 @@ 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)) {
+ throw new ForbiddenException(s"You do not have permission to modify workflow $wid")
+ }
+ val image = Option(request.image).map(_.trim).getOrElse("")
Review Comment:
Fixed. Now wraps the request entity too (`Option(request).flatMap(r => Option(r.image))`), so a null body returns 400 instead of NPE-ing.
##########
frontend/src/app/dashboard/component/user/list-item/card-item/card-item.component.scss:
##########
@@ -69,6 +69,36 @@
z-index: 10;
}
+// Cover-image controls, revealed on hover.
+.card-image-controls {
+ position: absolute;
+ bottom: 4px;
+ right: 4px;
+ z-index: 11;
+ display: flex;
+ gap: 4px;
+ opacity: 0;
+ transition: opacity 0.2s;
+}
+
+.card-item:hover .card-image-controls {
+ opacity: 1;
+}
Review Comment:
Fixed. Added `pointer-events: none` while hidden and reveal on `:focus-within` for keyboard access.
##########
frontend/src/app/dashboard/component/user/list-item/card-item/card-item.component.html:
##########
@@ -36,6 +36,39 @@
[(ngModel)]="entry.checked"
(ngModelChange)="onCheckboxChange(entry)"></label>
</div>
+ <!-- Cover-image controls -->
+ <div
+ class="card-image-controls"
+ *ngIf="canEditCover"
+ (click)="$event.stopPropagation()">
+ <button
+ nz-button
+ nzType="text"
+ class="image-control-btn"
+ title="Change cover image"
+ (click)="openImagePicker()">
+ <i
+ nz-icon
+ nzType="camera"></i>
+ </button>
+ <button
+ *ngIf="hasCustomImage"
+ nz-button
+ nzType="text"
+ class="image-control-btn"
+ title="Reset to default image"
+ (click)="resetImage()">
+ <i
+ nz-icon
+ nzType="rollback"></i>
+ </button>
Review Comment:
Done. Added `aria-label` and `type=\"button\"` to both buttons.
##########
common/workflow-operator/src/test/scala/org/apache/texera/amber/operator/huggingFace/codegen/ImageTaskCodegenSpec.scala:
##########
@@ -0,0 +1,214 @@
+/*
+ * 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.
+ */
+
+package org.apache.texera.amber.operator.huggingFace.codegen
+
+import org.apache.texera.amber.pybuilder.PyStringTypes.EncodableString
+import org.scalatest.flatspec.AnyFlatSpec
+import org.scalatest.matchers.should.Matchers
+
+class ImageTaskCodegenSpec extends AnyFlatSpec with Matchers {
+
Review Comment:
This branch also includes the HuggingFace image-upload work that this spec covers. I will update the PR description to reflect that.
##########
amber/src/main/scala/org/apache/texera/web/resource/dashboard/user/workflow/WorkflowResource.scala:
##########
@@ -711,6 +717,73 @@ 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)) {
+ throw new ForbiddenException(s"You do not have permission to modify workflow $wid")
+ }
+ val image = Option(request.image).map(_.trim).getOrElse("")
+ if (image.isEmpty) {
+ throw new BadRequestException("Cover image is required")
+ }
+ if (!image.startsWith("data:image/")) {
+ throw new BadRequestException("Cover image must be an image data URL")
+ }
Review Comment:
Fixed. Now requires base64 PNG/JPEG/GIF/WebP via an allowlist (SVG rejected), and tightened the frontend `accept` to match.
--
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: notifications-unsubscribe@texera.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org