yangzhang75 commented on code in PR #8539:
URL: https://github.com/apache/texera/pull/8539#discussion_r4050555560
##########
frontend/src/app/workspace/service/workflow-result-export/workflow-result-export.service.ts:
##########
@@ -198,10 +198,14 @@ export class WorkflowResultExportService {
columnIndex: number,
filename: string,
exportAll: boolean = false, // if the user click export button on the top
bar (a.k.a menu),
Review Comment:
Removed.
##########
frontend/src/app/workspace/service/workflow-result-export/workflow-result-export.service.ts:
##########
@@ -198,10 +198,14 @@ export class WorkflowResultExportService {
columnIndex: number,
filename: string,
exportAll: boolean = false, // if the user click export button on the top
bar (a.k.a menu),
- // we should export all operators, otherwise, only highlighted ones
- // which means export button is selected from context-menu
+ // we should export all operators, otherwise the ones requestedOperatorIds
names below, or
+ // the highlighted ones when it names none, which means the export button
came from the
+ // context-menu
destination: "dataset" | "local" = "dataset", // default to dataset
- unit: DashboardWorkflowComputingUnit | null // computing unit for cluster
setting
+ unit: DashboardWorkflowComputingUnit | null, // computing unit for cluster
setting
+ // The operators to export, for a caller that already knows them. Ignored
when exportAll
+ // asks for everything; left out, the scope falls back to the canvas
selection.
+ requestedOperatorIds: readonly string[] = []
Review Comment:
Renamed to `operatorIds`, default removed.
##########
frontend/src/app/workspace/service/workflow-result-export/workflow-result-export.service.ts:
##########
@@ -198,10 +198,14 @@ export class WorkflowResultExportService {
columnIndex: number,
filename: string,
exportAll: boolean = false, // if the user click export button on the top
bar (a.k.a menu),
- // we should export all operators, otherwise, only highlighted ones
- // which means export button is selected from context-menu
+ // we should export all operators, otherwise the ones requestedOperatorIds
names below, or
+ // the highlighted ones when it names none, which means the export button
came from the
+ // context-menu
destination: "dataset" | "local" = "dataset", // default to dataset
Review Comment:
Removed. `destination` had one too; both are gone.
##########
frontend/src/app/workspace/service/workflow-result-export/workflow-result-export.service.ts:
##########
@@ -262,13 +268,21 @@ export class WorkflowResultExportService {
return;
}
- // Determines operator scope
+ // Determines operator scope. "Everything" wins first: the top menu means
the whole workflow
+ // whatever else it was handed. Otherwise a caller that names its
operators wins over the
+ // canvas selection, which answers a different question -- what the user
has selected. That
+ // is the context menu's scope, but not a result cell's: a cell belongs to
one operator,
+ // whoever is selected. The Form View selects the step the user is
configuring, and until
+ // they click one it selects nothing, so a cell's export there came out
with an empty scope
+ // and returned below without sending a request -- a button that did
nothing at all.
const operatorIds = exportAll
? this.workflowActionService
.getTexeraGraph()
.getAllOperators()
.map(operator => operator.operatorID)
- :
[...this.workflowActionService.getJointGraphWrapper().getCurrentHighlightedOperatorIDs()];
+ : requestedOperatorIds.length > 0
+ ? [...requestedOperatorIds]
+ :
[...this.workflowActionService.getJointGraphWrapper().getCurrentHighlightedOperatorIDs()];
Review Comment:
Simplified to use what it was given. One thing to flag: the three-way
expression also held the `[...]` copy of the selection, and the restriction
analysis is async, so I kept a snapshot at the entry point (`const scope =
[...operatorIds]`) with a test. Without it the export would go out with
whatever is selected when the analysis answers.
--
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]
