Skip to content

Commit ba04b0f

Browse files
kevinkim-ogpkevinkim-ogp
authored
chore: fixes and news item (#1326)
## What changed? * Added Collaborators to What's new * Skip check for flow `updatedAt` if its the same user * Change shared Pipe icon to `BiGroup` * Update 'Add collaborator' button to be the same as 'Transfer Pipe' * `assertNotUpdatedSince` for `updateFlow` * Fix flaky test ## Tests - [x] Error still throws when two collaborators attempt to edit the Pipe and clobber each other's changes - [x] Can still add collaborator - [x] What's new shows collaborators
1 parent df565c7 commit ba04b0f

File tree

20 files changed

+158
-103
lines changed

20 files changed

+158
-103
lines changed

‎packages/backend/src/graphql/__tests__/mutations/create-step.itest.ts‎

Lines changed: 19 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -67,6 +67,7 @@ describe('createStep mutation integration tests', async () => {
6767
testFlow = await testUser.$relatedQuery('flows').insertAndFetch({
6868
name: 'Test Flow',
6969
// additional flow properties as needed
70+
updatedBy: owner.id,
7071
})
7172
testFlowTimestampString = String(new Date(testFlow.updatedAt).getTime())
7273

@@ -448,7 +449,7 @@ describe('createStep mutation integration tests', async () => {
448449
expect((newStep as any).connectionId).toBeNull()
449450
})
450451

451-
describe('updatedAt validation', () => {
452+
describe('updatedAt and updatedBy validation', () => {
452453
it('should succeed when input.flow.updatedAt matches flow.updatedAt (timestamp string)', async () => {
453454
const params = {
454455
input: {
@@ -469,7 +470,7 @@ describe('createStep mutation integration tests', async () => {
469470
expect(newStep.key).toBe('newStep')
470471
})
471472

472-
it('should throw when input.flow.updatedAt is different from flow.updatedAt (timestamp string)', async () => {
473+
it('should succeed when input.flow.updatedAt is different from flow.updatedAt for same user', async () => {
473474
const futureTimestamp = (
474475
new Date(testFlow.updatedAt).getTime() + 1000
475476
).toString()
@@ -486,23 +487,22 @@ describe('createStep mutation integration tests', async () => {
486487
},
487488
}
488489

489-
await expect(createStep(null, params, context)).rejects.toThrow(
490-
BadUserInputError,
491-
)
492-
await expect(createStep(null, params, context)).rejects.toThrow(
493-
REFRESH_PIPE_MESSAGE,
494-
)
490+
const newStep = await createStep(null, params, context)
491+
492+
expect(newStep).toBeDefined()
493+
expect(newStep.key).toBe('newStep')
495494
})
496495

497-
it('should throw when input.flow.updatedAt is different from flow.updatedAt (unix timestamp string)', async () => {
498-
const futureUnixTimestamp = Math.floor(
499-
(new Date(testFlow.updatedAt).getTime() + 1000) / 1000,
496+
it('should throw when input.flow.updatedAt is different from flow.updatedAt for different user', async () => {
497+
const futureTimestamp = (
498+
new Date(testFlow.updatedAt).getTime() + 1000
500499
).toString()
500+
context.currentUser = editor
501501
const params = {
502502
input: {
503503
flow: {
504504
id: testFlow.id,
505-
updatedAt: futureUnixTimestamp,
505+
updatedAt: futureTimestamp,
506506
},
507507
previousStep: { id: existingSteps[0].id },
508508
key: 'newStep',
@@ -519,10 +519,11 @@ describe('createStep mutation integration tests', async () => {
519519
)
520520
})
521521

522-
it('should throw when input.flow.updatedAt is older than flow.updatedAt (timestamp string)', async () => {
522+
it('should throw when input.flow.updatedAt is older than flow.updatedAt for different user', async () => {
523523
const oldTimestamp = (
524524
new Date(testFlow.updatedAt).getTime() - 5000
525525
).toString()
526+
context.currentUser = editor
526527
const params = {
527528
input: {
528529
flow: {
@@ -544,7 +545,7 @@ describe('createStep mutation integration tests', async () => {
544545
)
545546
})
546547

547-
it('should throw when input.flow.updatedAt is an invalid timestamp string', async () => {
548+
it('should not throw when input.flow.updatedAt is an invalid timestamp string', async () => {
548549
const params = {
549550
input: {
550551
flow: {
@@ -558,12 +559,10 @@ describe('createStep mutation integration tests', async () => {
558559
},
559560
}
560561

561-
await expect(createStep(null, params, context)).rejects.toThrow(
562-
BadUserInputError,
563-
)
564-
await expect(createStep(null, params, context)).rejects.toThrow(
565-
REFRESH_PIPE_MESSAGE,
566-
)
562+
const newStep = await createStep(null, params, context)
563+
564+
expect(newStep).toBeDefined()
565+
expect(newStep.key).toBe('newStep')
567566
})
568567
})
569568
})

‎packages/backend/src/graphql/__tests__/mutations/update-flow-status.itest.ts‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -332,6 +332,7 @@ describe('updateFlowStatus', () => {
332332
expect(result).toEqual(fakeFlow)
333333
expect(fakeFlow.assertNotUpdatedSince).toHaveBeenCalledWith(
334334
defaultInput.updatedAt,
335+
editor.id,
335336
)
336337
expect(patchSpy).toHaveBeenCalledWith({
337338
active: true,

‎packages/backend/src/graphql/mutations/create-step.ts‎

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -24,7 +24,7 @@ const createStep: MutationResolvers['createStep'] = async (
2424
})
2525
.throwIfNotFound()
2626

27-
flow.assertNotUpdatedSince(input.flow.updatedAt)
27+
flow.assertNotUpdatedSince(input.flow.updatedAt, context.currentUser.id)
2828

2929
// if connectionId is specified, verify that the connection exists
3030
// and the user has the appropriate permissions to use it
@@ -84,7 +84,11 @@ const createStep: MutationResolvers['createStep'] = async (
8484
})
8585
}
8686

87-
const updatedFlow = await flow.patchLastUpdated({ flowId: flow.id, trx })
87+
const updatedFlow = await flow.patchLastUpdated({
88+
flowId: flow.id,
89+
updatedBy: context.currentUser.id,
90+
trx,
91+
})
8892

8993
return {
9094
...step,

‎packages/backend/src/graphql/mutations/delete-step.ts‎

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -38,7 +38,7 @@ const deleteStep: MutationResolvers['deleteStep'] = async (
3838
})
3939

4040
const flow = steps[0].flow
41-
flow.assertNotUpdatedSince(input.flow.updatedAt)
41+
flow.assertNotUpdatedSince(input.flow.updatedAt, context.currentUser.id)
4242

4343
if (!steps.every((step) => step.flowId === steps[0].flowId)) {
4444
throw new Error('All steps to be deleted must be from the same pipe!')
@@ -114,7 +114,11 @@ const deleteStep: MutationResolvers['deleteStep'] = async (
114114
.patch({ position: raw(`position - ${steps.length}`) })
115115
}
116116

117-
await flow.patchLastUpdated({ flowId: flow.id, trx })
117+
await flow.patchLastUpdated({
118+
flowId: flow.id,
119+
updatedBy: context.currentUser.id,
120+
trx,
121+
})
118122

119123
return await flow
120124
.$query(trx)

‎packages/backend/src/graphql/mutations/duplicate-branch.ts‎

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -25,7 +25,7 @@ const duplicateBranch: MutationResolvers['duplicateBranch'] = async (
2525
})
2626
.throwIfNotFound()
2727

28-
flow.assertNotUpdatedSince(input.flow.updatedAt)
28+
flow.assertNotUpdatedSince(input.flow.updatedAt, context.currentUser.id)
2929

3030
// create all the steps in the transaction so that if any fails
3131
// this entire query fails
@@ -90,7 +90,11 @@ const duplicateBranch: MutationResolvers['duplicateBranch'] = async (
9090
// was created in the original branch
9191
}
9292

93-
const updatedFlow = await flow.patchLastUpdated({ flowId: flow.id, trx })
93+
const updatedFlow = await flow.patchLastUpdated({
94+
flowId: flow.id,
95+
updatedBy: context.currentUser.id,
96+
trx,
97+
})
9498

9599
return {
96100
steps: newSteps,

‎packages/backend/src/graphql/mutations/update-flow-status.ts‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -52,7 +52,7 @@ const updateFlowStatus: MutationResolvers['updateFlowStatus'] = async (
5252
return flow
5353
}
5454

55-
flow.assertNotUpdatedSince(params.input.updatedAt)
55+
flow.assertNotUpdatedSince(params.input.updatedAt, context.currentUser.id)
5656

5757
if (params.input.active) {
5858
validateFlowSteps(flow.steps)

‎packages/backend/src/graphql/mutations/update-flow.ts‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -12,8 +12,12 @@ const updateFlow: MutationResolvers['updateFlow'] = async (
1212
})
1313
.throwIfNotFound()
1414

15+
flow.assertNotUpdatedSince(params.input.updatedAt, context.currentUser.id)
16+
1517
return await flow.$query().patchAndFetch({
1618
name: params.input.name,
19+
updatedAt: new Date().toISOString(),
20+
updatedBy: context.currentUser.id,
1721
})
1822
}
1923

‎packages/backend/src/graphql/mutations/update-step-positions.ts‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -50,7 +50,7 @@ const updateStepPositions: MutationResolvers['updateStepPositions'] = async (
5050
}
5151

5252
const flow = steps[0].flow
53-
flow.assertNotUpdatedSince(input.flow.updatedAt)
53+
flow.assertNotUpdatedSince(input.flow.updatedAt, context.currentUser.id)
5454

5555
const foundStepIds = steps.map((step) => step.id)
5656
const missingStepIds = stepIds.filter(

‎packages/backend/src/graphql/mutations/update-step.ts‎

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -28,7 +28,10 @@ const updateStep: MutationResolvers['updateStep'] = async (
2828
throw new BadUserInputError('Step not found')
2929
}
3030

31-
step.flow.assertNotUpdatedSince(input.flow.updatedAt)
31+
step.flow.assertNotUpdatedSince(
32+
input.flow.updatedAt,
33+
context.currentUser.id,
34+
)
3235

3336
if (input.connection.id) {
3437
// if connectionId is specified, verify that the connection exists
@@ -121,11 +124,13 @@ const updateStep: MutationResolvers['updateStep'] = async (
121124
// update the flow's last updated
122125
const updatedFlow = await step.flow.patchLastUpdated({
123126
flowId: step.flowId,
127+
updatedBy: context.currentUser.id,
124128
trx,
125129
})
126130

127131
// do this instead of spreading the updatedStep so that it preserves the Model instance
128132
updatedStep.flow.updatedAt = updatedFlow.updatedAt
133+
updatedStep.flow.updatedBy = context.currentUser.id
129134
return updatedStep
130135
})
131136

‎packages/backend/src/graphql/schema.graphql‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -514,6 +514,7 @@ input CreateTemplatedFlowInput {
514514
input UpdateFlowInput {
515515
id: String!
516516
name: String!
517+
updatedAt: String!
517518
}
518519

519520
input UpdateFlowStatusInput {

0 commit comments

Comments
 (0)