Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: Show actual execution data for production executions even if pin data exists #6302

Merged
merged 6 commits into from
Jun 1, 2023
Merged
Show file tree
Hide file tree
Changes from 4 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
13 changes: 12 additions & 1 deletion packages/editor-ui/src/components/WorkflowPreview.vue
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@ import { useToast } from '@/composables';
import type { IWorkflowDb } from '@/Interface';
import { mapStores } from 'pinia';
import { useRootStore } from '@/stores/n8nRoot.store';
import { useWorkflowsStore } from '@/stores';

export default defineComponent({
name: 'WorkflowPreview',
Expand Down Expand Up @@ -73,7 +74,7 @@ export default defineComponent({
};
},
computed: {
...mapStores(useRootStore),
...mapStores(useRootStore, useWorkflowsStore),
showPreview(): boolean {
return (
!this.loading &&
Expand Down Expand Up @@ -134,6 +135,16 @@ export default defineComponent({
}),
'*',
);

if (this.workflowsStore.activeWorkflowExecution) {
iframeRef.contentWindow.postMessage(
JSON.stringify({
command: 'setActiveExecution',
execution: this.workflowsStore.activeWorkflowExecution,
}),
'*',
);
}
}
} catch (error) {
this.showError(
Expand Down
37 changes: 37 additions & 0 deletions packages/editor-ui/src/mixins/__tests__/workflowHelpers.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,37 @@
import { shouldReplaceInputDataWithPinData } from '../workflowHelpers';
import { createTestingPinia } from '@pinia/testing';
import { useWorkflowsStore } from '@/stores';

let pinia: ReturnType<typeof createTestingPinia>;
beforeAll(() => {
pinia = createTestingPinia();
});

describe('workflowHelpers', () => {
describe('shouldReplaceInputDataWithPinData', () => {
beforeEach(() => {
pinia.state.value = {
workflows: useWorkflowsStore(),
};
});

it('should return true if no active execution is set', () => {
const result = shouldReplaceInputDataWithPinData();
expect(result).toBe(true);
});

it('should return true if active execution is set and mode is manual', async () => {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
it('should return true if active execution is set and mode is manual', async () => {
it('should return true if active execution is set and mode is manual', () => {

pinia.state.value.workflows.activeWorkflowExecution = { mode: 'manual' };

const result = shouldReplaceInputDataWithPinData();
expect(result).toBe(true);
});

it('should return false if active execution is set and mode is not manual', async () => {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
it('should return false if active execution is set and mode is not manual', async () => {
it('should return false if active execution is set and mode is not manual', () => {

pinia.state.value.workflows.activeWorkflowExecution = { mode: 'webhook' };

const result = shouldReplaceInputDataWithPinData();
expect(result).toBe(false);
});
});
});
76 changes: 46 additions & 30 deletions packages/editor-ui/src/mixins/workflowHelpers.ts
Original file line number Diff line number Diff line change
Expand Up @@ -189,6 +189,15 @@ function getNodeTypes(): INodeTypes {
return useWorkflowsStore().getNodeTypes();
}

export function shouldReplaceInputDataWithPinData() {
const workflowsStore = useWorkflowsStore();
return (
!workflowsStore.activeWorkflowExecution ||
(workflowsStore.activeWorkflowExecution &&
workflowsStore.activeWorkflowExecution.mode === 'manual')
);
}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Or shorter:

export function shouldReplaceInputDataWithPinData() {
	const { activeWorkflowExecution } = useWorkflowsStore();
	return !activeWorkflowExecution || activeWorkflowExecution?.mode === 'manual';
}

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We could make this a getter in the workflows store? To avoid adding to mixins.


// Returns connectionInputData to be able to execute an expression.
function connectionInputData(
parentNode: string[],
Expand Down Expand Up @@ -223,32 +232,36 @@ function connectionInputData(
}
}

const parentPinData = parentNode.reduce((acc: INodeExecutionData[], parentNodeName, index) => {
const pinData = useWorkflowsStore().pinDataByNodeName(parentNodeName);
const workflowsStore = useWorkflowsStore();

if (pinData) {
acc.push({
json: pinData[0],
pairedItem: {
item: index,
input: 1,
},
});
}
if (shouldReplaceInputDataWithPinData()) {
const parentPinData = parentNode.reduce((acc: INodeExecutionData[], parentNodeName, index) => {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
const parentPinData = parentNode.reduce((acc: INodeExecutionData[], parentNodeName, index) => {
const parentPinData = parentNode.reduce<INodeExecutionData[]>((acc, parentNodeName, index) => {

const pinData = workflowsStore.pinDataByNodeName(parentNodeName);

return acc;
}, []);
if (pinData) {
acc.push({
json: pinData[0],
pairedItem: {
item: index,
input: 1,
},
});
}

if (parentPinData.length > 0) {
if (connectionInputData && connectionInputData.length > 0) {
parentPinData.forEach((parentPinDataEntry) => {
connectionInputData![0].json = {
...connectionInputData![0].json,
...parentPinDataEntry.json,
};
});
} else {
connectionInputData = parentPinData;
return acc;
}, []);

if (parentPinData.length > 0) {
if (connectionInputData && connectionInputData.length > 0) {
parentPinData.forEach((parentPinDataEntry) => {
connectionInputData![0].json = {
...connectionInputData![0].json,
...parentPinDataEntry.json,
};
});
} else {
connectionInputData = parentPinData;
}
}
}

Expand All @@ -271,21 +284,24 @@ function executeData(
// Add the input data to be able to also resolve the short expression format
// which does not use the node name
const parentNodeName = parentNode[0];
const workflowsStore = useWorkflowsStore();

const parentPinData = useWorkflowsStore().getPinData![parentNodeName];
if (shouldReplaceInputDataWithPinData()) {
const parentPinData = workflowsStore.getPinData![parentNodeName];

// populate `executeData` from `pinData`
// populate `executeData` from `pinData`

if (parentPinData) {
executeData.data = { main: [parentPinData] };
executeData.source = { main: [{ previousNode: parentNodeName }] };
if (parentPinData) {
executeData.data = { main: [parentPinData] };
executeData.source = { main: [{ previousNode: parentNodeName }] };

return executeData;
return executeData;
}
}

// populate `executeData` from `runData`

const workflowRunData = useWorkflowsStore().getWorkflowRunData;
const workflowRunData = workflowsStore.getWorkflowRunData;
if (workflowRunData === null) {
return executeData;
}
Expand Down
2 changes: 2 additions & 0 deletions packages/editor-ui/src/views/NodeView.vue
Original file line number Diff line number Diff line change
Expand Up @@ -3604,6 +3604,8 @@ export default defineComponent({
type: 'error',
});
}
} else if (json && json.command === 'setActiveExecution') {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
} else if (json && json.command === 'setActiveExecution') {
} else if (json?.command === 'setActiveExecution') {

this.workflowsStore.activeWorkflowExecution = json.execution;
}
} catch (e) {}
},
Expand Down