mudhorn/n8n
1
0
Fork 0
mirror of https://github.com/n8n-io/n8n.git synced 2025-02-02 07:01:30 -08:00
Code Issues Actions Packages Projects Releases Wiki Activity

fix(core): revert blocking workflow update on interim change (#4437)

Browse source 
⏪ Revert "feat(core): block workflow update on interim change (#4397)"

This reverts commit cddd012a2f.
This commit is contained in:
Iván Ovejero 2022-10-25 13:45:47 +02:00 committed by GitHub
parent b296fb06f3
commit 07adc2d2dc
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
11 changed files with 10 additions and 208 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

7
packages/cli/src/requests.d.ts vendored
View file

@ -56,12 +56,7 @@ export declare namespace WorkflowRequest {
type Delete = Get;
type Update = AuthenticatedRequest<
{ id: string },
{},
RequestBody & { updatedAt: string },
{ forceSave?: string }
>;
type Update = AuthenticatedRequest<{ id: string }, {}, RequestBody>;
type NewName = AuthenticatedRequest<{}, {}, {}, { name?: string }>;

17
packages/cli/src/workflows/workflows.controller.ts
View file

@ -329,7 +329,6 @@ workflowsController.patch(
`/:id`,
ResponseHelper.send(async (req: WorkflowRequest.Update) => {
const { id: workflowId } = req.params;
const { forceSave } = req.query;
const updateData = new WorkflowEntity();
const { tags, ...rest } = req.body;
@ -356,22 +355,6 @@ workflowsController.patch(
);
}
const lastKnownDate = new Date(req.body.updatedAt).getTime();
const storedDate = new Date(shared.workflow.updatedAt).getTime();
if (!forceSave && lastKnownDate !== storedDate) {
LoggerProxy.info(
'User was blocked from updating a workflow that was changed by another user',
{ workflowId, userId: req.user.id },
);
throw new ResponseHelper.ResponseError(
`Workflow ID ${workflowId} cannot be saved because it was changed by another user.`,
undefined,
400,
);
}
// check credentials for old format
await WorkflowHelpers.replaceInvalidCredentials(updateData);

11
packages/cli/test/integration/shared/utils.ts
View file

@ -706,7 +706,10 @@ export const emptyPackage = () => {
// workflow
// ----------------------------------
export function makeWorkflow(options?: {
export function makeWorkflow({
withPinData,
withCredential,
}: {
withPinData: boolean;
withCredential?: { id: string; name: string };
}) {
@ -721,9 +724,9 @@ export function makeWorkflow(options?: {
position: [740, 240],
};
if (options?.withCredential) {
if (withCredential) {
node.credentials = {
spotifyApi: options.withCredential,
spotifyApi: withCredential,
};
}
@ -732,7 +735,7 @@ export function makeWorkflow(options?: {
workflow.connections = {};
workflow.nodes = [node];
if (options?.withPinData) {
if (withPinData) {
workflow.pinData = MOCK_PINDATA;
}

150
packages/cli/test/integration/workflows.controller.ee.test.ts
View file

@ -295,153 +295,3 @@ describe('POST /workflows', () => {
expect(usedCredentials).toHaveLength(1);
});
});
describe('PATCH /workflows/:id', () => {
it('should block owner update on interim update by member', async () => {
const owner = await testDb.createUser({ globalRole: globalOwnerRole });
const member = await testDb.createUser({ globalRole: globalMemberRole });
// owner creates and shares workflow
const createResponse = await authAgent(owner).post('/workflows').send(makeWorkflow());
const { id, updatedAt: ownerLastKnownDate } = createResponse.body.data;
await authAgent(owner)
.put(`/workflows/${id}/share`)
.send({ shareWithIds: [member.id] });
// member accesses and updates workflow
const memberGetResponse = await authAgent(member).get(`/workflows/${id}`);
const { updatedAt: memberLastKnownDate } = memberGetResponse.body.data;
await authAgent(member)
.patch(`/workflows/${id}`)
.send({ name: 'Update by member', updatedAt: memberLastKnownDate });
// owner blocked from updating workflow
const updateAttemptResponse = await authAgent(owner)
.patch(`/workflows/${id}`)
.send({ name: 'Update attempt by owner', updatedAt: ownerLastKnownDate });
expect(updateAttemptResponse.status).toBe(400);
expect(updateAttemptResponse.body.message).toContain(
'cannot be saved because it was changed by another user',
);
});
it('should block member update on interim update by owner', async () => {
const owner = await testDb.createUser({ globalRole: globalOwnerRole });
const member = await testDb.createUser({ globalRole: globalMemberRole });
// owner creates, updates and shares workflow
const createResponse = await authAgent(owner).post('/workflows').send(makeWorkflow());
const { id, updatedAt: ownerFirstUpdateDate } = createResponse.body.data;
const updateResponse = await authAgent(owner)
.patch(`/workflows/${id}`)
.send({ name: 'Update by owner', updatedAt: ownerFirstUpdateDate });
const { updatedAt: ownerSecondUpdateDate } = updateResponse.body.data;
await authAgent(owner)
.put(`/workflows/${id}/share`)
.send({ shareWithIds: [member.id] });
// member accesses workflow
const memberGetResponse = await authAgent(member).get(`/workflows/${id}`);
const { updatedAt: memberLastKnownDate } = memberGetResponse.body.data;
// owner re-updates workflow
await authAgent(owner)
.patch(`/workflows/${id}`)
.send({ name: 'Owner update again', updatedAt: ownerSecondUpdateDate });
// member blocked from updating workflow
const updateAttemptResponse = await authAgent(member)
.patch(`/workflows/${id}`)
.send({ name: 'Update attempt by member', updatedAt: memberLastKnownDate });
expect(updateAttemptResponse.status).toBe(400);
expect(updateAttemptResponse.body.message).toContain(
'cannot be saved because it was changed by another user',
);
});
it('should block owner activation on interim activation by member', async () => {
const owner = await testDb.createUser({ globalRole: globalOwnerRole });
const member = await testDb.createUser({ globalRole: globalMemberRole });
// owner creates and shares workflow
const createResponse = await authAgent(owner).post('/workflows').send(makeWorkflow());
const { id, updatedAt: ownerLastKnownDate } = createResponse.body.data;
await authAgent(owner)
.put(`/workflows/${id}/share`)
.send({ shareWithIds: [member.id] });
// member accesses and activates workflow
const memberGetResponse = await authAgent(member).get(`/workflows/${id}`);
const { updatedAt: memberLastKnownDate } = memberGetResponse.body.data;
await authAgent(member)
.patch(`/workflows/${id}`)
.send({ active: true, updatedAt: memberLastKnownDate });
// owner blocked from activating workflow
const activationAttemptResponse = await authAgent(owner)
.patch(`/workflows/${id}`)
.send({ active: true, updatedAt: ownerLastKnownDate });
expect(activationAttemptResponse.status).toBe(400);
expect(activationAttemptResponse.body.message).toContain(
'cannot be saved because it was changed by another user',
);
});
it('should block member activation on interim activation by owner', async () => {
const owner = await testDb.createUser({ globalRole: globalOwnerRole });
const member = await testDb.createUser({ globalRole: globalMemberRole });
// owner creates, updates and shares workflow
const createResponse = await authAgent(owner).post('/workflows').send(makeWorkflow());
const { id, updatedAt: ownerFirstUpdateDate } = createResponse.body.data;
const updateResponse = await authAgent(owner)
.patch(`/workflows/${id}`)
.send({ name: 'Update by owner', updatedAt: ownerFirstUpdateDate });
const { updatedAt: ownerSecondUpdateDate } = updateResponse.body.data;
await authAgent(owner)
.put(`/workflows/${id}/share`)
.send({ shareWithIds: [member.id] });
// member accesses workflow
const memberGetResponse = await authAgent(member).get(`/workflows/${id}`);
const { updatedAt: memberLastKnownDate } = memberGetResponse.body.data;
// owner activates workflow
await authAgent(owner)
.patch(`/workflows/${id}`)
.send({ active: true, updatedAt: ownerSecondUpdateDate });
// member blocked from activating workflow
const updateAttemptResponse = await authAgent(member)
.patch(`/workflows/${id}`)
.send({ active: true, updatedAt: memberLastKnownDate });
expect(updateAttemptResponse.status).toBe(400);
expect(updateAttemptResponse.body.message).toContain(
'cannot be saved because it was changed by another user',
);
});
});

2
packages/editor-ui/src/Interface.ts
View file

@ -268,7 +268,6 @@ export interface IWorkflowData {
settings?: IWorkflowSettings;
tags?: string[];
pinData?: IPinData;
updatedAt?: string;
}
export interface IWorkflowDataUpdate {
@ -280,7 +279,6 @@ export interface IWorkflowDataUpdate {
active?: boolean;
tags?: ITag[] | string[]; // string[] when store or requested, ITag[] from API response
pinData?: IPinData;
updatedAt?: string;
}
export interface IWorkflowToShare extends IWorkflowDataUpdate {

10
packages/editor-ui/src/components/WorkflowCard.vue
View file

@ -10,15 +10,7 @@
</template>
<div :class="$style.cardDescription">
<n8n-text color="text-light" size="small">
<span v-show="data">
<span v-if="data.updatedAt === -1">
{{ $locale.baseText('workflows.item.neverUpdated') }} |
</span>
<span v-else>
{{ $locale.baseText('workflows.item.updated') }}
<time-ago :date="data.updatedAt" /> |
</span>
</span>
<span v-show="data">{{$locale.baseText('workflows.item.updated')}} <time-ago :date="data.updatedAt" /> | </span>
<span v-show="data" class="mr-2xs">{{$locale.baseText('workflows.item.created')}} {{ formattedCreatedAtDate }} </span>
<span v-if="areTagsEnabled && data.tags && data.tags.length > 0" v-show="data">
<n8n-tags

3
packages/editor-ui/src/components/WorkflowSettings.vue
View file

@ -519,8 +519,7 @@ export default mixins(
this.isLoading = true;
try {
const workflow = await this.restApi().updateWorkflow(this.$route.params.name, data);
this.$store.commit('setWorkflowUpdatedAt', workflow.updatedAt);
await this.restApi().updateWorkflow(this.$route.params.name, data);
} catch (error) {
this.$showError(
error,

6
packages/editor-ui/src/components/mixins/workflowHelpers.ts
View file

@ -400,7 +400,6 @@ export const workflowHelpers = mixins(
active: this.$store.getters.isActive,
settings: this.$store.getters.workflowSettings,
tags: this.$store.getters.workflowTags,
updatedAt: this.$store.getters.workflowUpdatedAt,
};
const workflowId = this.$store.getters.workflowId;
@ -661,9 +660,6 @@ export const workflowHelpers = mixins(
const isCurrentWorkflow = workflowId === this.$store.getters.workflowId;
if (isCurrentWorkflow) {
data = await this.getWorkflowDataToSave();
} else {
const { updatedAt } = await this.restApi().getWorkflow(workflowId);
data.updatedAt = updatedAt as string;
}
if (active !== undefined) {
@ -671,7 +667,6 @@ export const workflowHelpers = mixins(
}
const workflow = await this.restApi().updateWorkflow(workflowId, data);
this.$store.commit('setWorkflowUpdatedAt', workflow.updatedAt);
if (isCurrentWorkflow) {
this.$store.commit('setActive', !!workflow.active);
@ -719,7 +714,6 @@ export const workflowHelpers = mixins(
this.$store.commit('setStateDirty', false);
this.$store.commit('removeActiveAction', 'workflowSaving');
this.$store.commit('setWorkflowUpdatedAt', workflowData.updatedAt);
this.$externalHooks().run('workflow.afterUpdate', { workflowData });
return true;

1
packages/editor-ui/src/plugins/i18n/locales/en.json
View file

@ -1287,7 +1287,6 @@
"workflows.item.open": "Open",
"workflows.item.duplicate": "Duplicate",
"workflows.item.delete": "Delete",
"workflows.item.neverUpdated": "Never updated",
"workflows.item.updated": "Last updated",
"workflows.item.created": "Created",
"workflows.item.owner": "Owner",

10
packages/editor-ui/src/store.ts
View file

@ -465,13 +465,6 @@ export const store = new Vuex.Store({
state.workflow.id = id;
},
/**
* @warning Do not mutate `updatedAt` in `state.workflow` client-side.
*/
setWorkflowUpdatedAt (state, updatedAt: string) {
state.workflow.updatedAt = updatedAt;
},
// Name
setWorkflowName(state, data) {
if (data.setStateDirty === true) {
@ -1015,9 +1008,6 @@ export const store = new Vuex.Store({
workflowId: (state): string => {
return state.workflow.id;
},
workflowUpdatedAt (state): string | number {
return state.workflow.updatedAt;
},
workflowSettings: (state): IWorkflowSettings => {
if (state.workflow.settings === undefined) {

1
packages/editor-ui/src/views/NodeView.vue
View file

@ -715,7 +715,6 @@ export default mixins(
this.$store.commit('setActive', data.active || false);
this.$store.commit('setWorkflowId', workflowId);
this.$store.commit('setWorkflowUpdatedAt', data.updatedAt);
this.$store.commit('setWorkflowName', { newName: data.name, setStateDirty: false });
this.$store.commit('setWorkflowSettings', data.settings || {});
this.$store.commit('setWorkflowPinData', data.pinData || {});