Copilot commented on code in PR #8551:
URL: https://github.com/apache/texera/pull/8551#discussion_r4030366745
##########
frontend/src/app/workspace/component/power-button/computing-unit-selection.component.ts:
##########
@@ -153,9 +158,23 @@ export class ComputingUnitSelectionComponent implements
OnInit {
selectedComputingUnit: DashboardWorkflowComputingUnit | null = null;
allComputingUnits: DashboardWorkflowComputingUnit[] = [];
+ // Per-user warehouse picker (#7817): shown whenever the deployment reports
+ // the feature enabled — with zero warehouses it still offers the create
+ // entry, and the Run button leads there too.
+ warehouseEnabled: boolean = false;
+ warehouses: DashboardWarehouse[] = [];
+ selectedWarehouseId?: number;
+ // The latest execution's warehouse; the warehouse list and the latest
+ // execution are fetched concurrently, so preselection re-runs after
+ // whichever response lands last.
+ private lastExecutionWhid?: number;
Review Comment:
`lastExecutionWhid` is component-global but is never reset when `workflowId`
changes. If workflow A sets this to warehouse 2 and workflow B has no execution
history, either error handler calls `applyWarehousePreselect()` with A's value
and permanently selects warehouse 2 instead of B's required first-warehouse
fallback; the old service selection is also usable while B's lookup is pending.
Clear both this field and the selected warehouse immediately on a workflow-ID
transition before starting the new lookup.
This issue also appears on line 474 of the same file.
##########
frontend/src/app/workspace/component/menu/menu.component.ts:
##########
@@ -408,6 +408,18 @@ export class MenuComponent implements OnInit, OnDestroy {
};
}
+ // Per-user warehouses enabled but none to write to (#7817): mirror the
+ // Connect state above — name the fixing action, and runWorkflow() routes
+ // the click into the create-warehouse modal.
+ if (this.computingUnitSelectionComponent?.warehouseRequiredButMissing) {
Review Comment:
The Run button stores a snapshot of this behavior, but `MenuComponent` only
recomputes it for execution, validation, and computing-unit status changes.
Warehouse status/selection changes do not trigger `applyRunButtonBehavior`, so
after the asynchronous warehouse load the button can still display “Run” with
no warehouse (or remain “Create Warehouse” after creation). Subscribe the menu
to warehouse readiness changes or emit readiness from the child and recompute
the behavior.
##########
frontend/src/app/workspace/component/power-button/computing-unit-selection.component.ts:
##########
@@ -409,6 +450,123 @@ export class ComputingUnitSelectionComponent implements
OnInit {
}
}
+ /**
+ * Fetches the warehouse list, on init and on every dropdown open (mirroring
+ * onDropdownVisibilityChange). Preselection re-runs only when the current
+ * pick is gone (first load, or the picked warehouse was deleted), so a
+ * routine refresh cannot override a manual pick.
+ */
+ private refreshWarehouses(): void {
+ this.warehouseService
+ .getStatus()
+ .pipe(untilDestroyed(this))
+ .subscribe({
Review Comment:
These independent subscriptions can complete out of order. For example, an
initial/dropdown refresh can return after the post-delete refresh and restore
the deleted row, or an older failed request can clear a selection established
by a newer successful request. Route refreshes through a `switchMap` request
stream so only the latest response mutates state, as `UserWarehouseComponent`
already does.
##########
frontend/src/app/workspace/component/power-button/computing-unit-selection.component.html:
##########
@@ -58,6 +58,92 @@
</div>
</div>
+ <button
+ *ngIf="warehouseEnabled"
+ nz-button
+ nz-dropdown
+ nzTrigger="click"
+ [nzDropdownMenu]="warehouseMenu"
+ [nzPlacement]="'bottomRight'"
+ (nzVisibleChange)="onWarehouseDropdownVisibilityChange($event)"
+ class="warehouse-dropdown-button"
+ nz-tooltip
+ nzTooltipTitle="Warehouse this execution writes to">
+ <div class="button-content">
+ <texera-user-avatar
+ *ngIf="selectedWarehouse as selected"
+ [avatar]="selected.ownerAvatar || ''"
+ userColor="grey"
+ [userName]="selected.ownerName || ''"
+ [style.transform]="'scale(0.65)'"
+ [style.opacity]="0.7"
+ [style.padding-right.px]="2">
+ </texera-user-avatar>
+ <i
+ nz-icon
+ nzType="cloud-server"></i>
+ <span class="warehouse-name-text">{{ getWarehouseButtonText() }}</span>
+ <i
+ nz-icon
+ nzType="down"></i>
+ </div>
+ </button>
+
+ <nz-dropdown-menu #warehouseMenu="nzDropdownMenu">
+ <ul
+ nz-menu
+ class="warehouses-dropdown">
+ <li
+ nz-menu-item
+ *ngFor="let warehouse of warehouses; trackBy: trackByWhid"
+ id="warehouse-option"
+ class="warehouse-option"
Review Comment:
This `id` is emitted once per warehouse, producing duplicate DOM IDs
whenever the user has multiple warehouses. IDs must be unique; use a class/test
selector or include `warehouse.whid` in the bound ID.
This issue also appears on line 125 of the same file.
--
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]
