fix(core): revert blocking workflow updates on interim changes (no-changelog) (#4396)

 Revert "feat(core): block workflow update on interim change (#4374)"

This reverts commit e83b9bd983.
This commit is contained in:
Iván Ovejero 2022-10-20 17:55:21 +02:00 committed by GitHub
parent ef1198324d
commit 69848b54db
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
8 changed files with 8 additions and 116 deletions

View file

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

View file

@ -329,7 +329,6 @@ workflowsController.patch(
`/:id`, `/:id`,
ResponseHelper.send(async (req: WorkflowRequest.Update) => { ResponseHelper.send(async (req: WorkflowRequest.Update) => {
const { id: workflowId } = req.params; const { id: workflowId } = req.params;
const { forceSave } = req.query;
const updateData = new WorkflowEntity(); const updateData = new WorkflowEntity();
const { tags, ...rest } = req.body; 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 // check credentials for old format
await WorkflowHelpers.replaceInvalidCredentials(updateData); await WorkflowHelpers.replaceInvalidCredentials(updateData);

View file

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

View file

@ -294,78 +294,4 @@ describe('POST /workflows', () => {
const usedCredentials = await testDb.getCredentialUsageInWorkflow(response.body.data.id); const usedCredentials = await testDb.getCredentialUsageInWorkflow(response.body.data.id);
expect(usedCredentials).toHaveLength(1); expect(usedCredentials).toHaveLength(1);
}); });
it('PATCH /workflows/:id should be blocked on interim change - owner blocked', 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('PATCH /workflows/:id should be blocked on interim change - member blocked', 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',
);
});
}); });

View file

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

View file

@ -400,7 +400,6 @@ export const workflowHelpers = mixins(
active: this.$store.getters.isActive, active: this.$store.getters.isActive,
settings: this.$store.getters.workflowSettings, settings: this.$store.getters.workflowSettings,
tags: this.$store.getters.workflowTags, tags: this.$store.getters.workflowTags,
updatedAt: this.$store.getters.workflowUpdatedAt,
}; };
const workflowId = this.$store.getters.workflowId; const workflowId = this.$store.getters.workflowId;
@ -679,8 +678,6 @@ export const workflowHelpers = mixins(
} else { } else {
this.$store.commit('setWorkflowInactive', workflowId); this.$store.commit('setWorkflowInactive', workflowId);
} }
this.$store.commit('setWorkflowUpdatedAt', data.updatedAt);
}, },
async saveCurrentWorkflow({name, tags}: {name?: string, tags?: string[]} = {}, redirect = true): Promise<boolean> { async saveCurrentWorkflow({name, tags}: {name?: string, tags?: string[]} = {}, redirect = true): Promise<boolean> {
@ -717,7 +714,6 @@ export const workflowHelpers = mixins(
this.$store.commit('setStateDirty', false); this.$store.commit('setStateDirty', false);
this.$store.commit('removeActiveAction', 'workflowSaving'); this.$store.commit('removeActiveAction', 'workflowSaving');
this.$store.commit('setWorkflowUpdatedAt', workflowData.updatedAt);
this.$externalHooks().run('workflow.afterUpdate', { workflowData }); this.$externalHooks().run('workflow.afterUpdate', { workflowData });
return true; return true;

View file

@ -464,11 +464,6 @@ export const store = new Vuex.Store({
state.workflow.id = id; state.workflow.id = id;
}, },
// updatedAt
setWorkflowUpdatedAt (state, updatedAt: string) {
state.workflow.updatedAt = updatedAt;
},
// Name // Name
setWorkflowName(state, data) { setWorkflowName(state, data) {
if (data.setStateDirty === true) { if (data.setStateDirty === true) {
@ -1014,9 +1009,6 @@ export const store = new Vuex.Store({
workflowId: (state): string => { workflowId: (state): string => {
return state.workflow.id; return state.workflow.id;
}, },
workflowUpdatedAt (state): string | number {
return state.workflow.updatedAt;
},
workflowSettings: (state): IWorkflowSettings => { workflowSettings: (state): IWorkflowSettings => {
if (state.workflow.settings === undefined) { if (state.workflow.settings === undefined) {

View file

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