Skip to content

Commit

Permalink
Bug: flaky test 460 (#476)
Browse files Browse the repository at this point in the history
* refine promise chains

* cleanup
  • Loading branch information
greg-adams authored Oct 4, 2024
1 parent 8a09735 commit c79b821
Show file tree
Hide file tree
Showing 4 changed files with 84 additions and 97 deletions.
1 change: 1 addition & 0 deletions api/src/graphql/users.sdl.ts
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@ export const schema = gql`
agency: Agency!
role: RoleEnum!
isActive: Boolean!
passageId: String
uploaded: [Upload]!
certifiedReportingPeriods: [ReportingPeriodCertification]!
}
Expand Down
1 change: 0 additions & 1 deletion api/src/services/uploads/uploads.scenarios.ts
Original file line number Diff line number Diff line change
Expand Up @@ -292,7 +292,6 @@ export const uploadCheck = defineScenario<
},
agency: {
one: (scenario) => {
console.log(scenario)
return {
data: {
name: 'Agency1',
Expand Down
161 changes: 75 additions & 86 deletions api/src/services/users/users.test.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,3 @@
import type { User } from '@prisma/client'

import { EmailValidationError } from '@redwoodjs/api'

import { ROLES } from 'src/lib/constants'
Expand Down Expand Up @@ -172,7 +170,7 @@ describe('user queries', () => {
id: scenario.user.one.id,
email: scenario.user.one.email,
roles: ['USDR_ORGANIZATION_STAFF'],
agency: {},
agency: scenario.agency.three,
})
const result = await usersByOrganization({
organizationId: scenario.organization.one.id,
Expand Down Expand Up @@ -246,9 +244,9 @@ describe('user writes', () => {
email: scenario.user.one.email,
roles: ['USDR_ADMIN'],
})
const original = (await deleteUser({
const original = await deleteUser({
id: scenario.user.one.id,
})) as User
})
const result = await user({ id: original.id })

expect(result).toEqual(null)
Expand Down Expand Up @@ -363,30 +361,27 @@ describe('user writes', () => {

describe('general write validations', () => {
scenario('general validations, fails on invalid email', async () => {
expect(
async () =>
await runGeneralCreateOrUpdateValidations({
email: 'iamnotarealemail',
})
await expect(
runGeneralCreateOrUpdateValidations({
email: 'iamnotarealemail',
})
).rejects.toThrow(EmailValidationError)
})

scenario('general validations, fails on missing name', async () => {
expect(
async () =>
await runGeneralCreateOrUpdateValidations({
email: '[email protected]',
})
await expect(
runGeneralCreateOrUpdateValidations({
email: '[email protected]',
})
).rejects.toThrow('Please provide a name')
})

scenario('general validations, fails on missing role', async () => {
expect(
async () =>
await runGeneralCreateOrUpdateValidations({
email: '[email protected]',
name: 'FDR',
})
await expect(
runGeneralCreateOrUpdateValidations({
email: '[email protected]',
name: 'FDR',
})
).rejects.toThrow('Please provide a role')
})

Expand All @@ -398,9 +393,10 @@ describe('general write validations', () => {
email: scenario.user.one.email,
roles: ['USDR_ADMIN'],
})
expect(
async () => await runPermissionsCreateOrUpdateValidations({})
).not.toThrowError()

await expect(
runPermissionsCreateOrUpdateValidations({})
).resolves.not.toThrow()
}
)

Expand All @@ -412,33 +408,32 @@ describe('general write validations', () => {
email: scenario.user.one.email,
roles: ['ORGANIZATION_STAFF'],
})
expect(
async () => await runPermissionsCreateOrUpdateValidations({})
).rejects.toThrow("You don't have permission to do that")

await expect(runPermissionsCreateOrUpdateValidations({})).rejects.toThrow(
"You don't have permission to do that"
)
}
)

scenario('general validations, fails on bad role', async () => {
expect(
async () =>
await runGeneralCreateOrUpdateValidations({
email: '[email protected]',
name: 'FDR',
role: 'FAKE_ROLE',
})
await expect(
runGeneralCreateOrUpdateValidations({
email: '[email protected]',
name: 'FDR',
role: 'FAKE_ROLE',
})
).rejects.toThrow('Please select a recognized role')
})

scenario('general validations, fails on bad agency id', async () => {
expect(
async () =>
await runGeneralCreateOrUpdateValidations({
email: '[email protected]',
name: 'FDR',
role: 'ORGANIZATION_STAFF',
agencyId: 4598,
})
).rejects.toThrowError('No Agency found')
await expect(
runGeneralCreateOrUpdateValidations({
email: '[email protected]',
name: 'FDR',
role: 'ORGANIZATION_STAFF',
agencyId: 4598,
})
).rejects.toThrow('No Agency found')
})
})

Expand All @@ -451,11 +446,10 @@ describe('permissions write validations', () => {
email: scenario.user.one.email,
roles: ['ORGANIZATION_ADMIN'],
})
expect(
async () =>
await runPermissionsCreateOrUpdateValidations({
role: ROLES.USDR_ADMIN,
})
await expect(
runPermissionsCreateOrUpdateValidations({
role: ROLES.USDR_ADMIN,
})
).rejects.toThrow("You don't have permission to update that role")
}
)
Expand All @@ -469,12 +463,11 @@ describe('permissions write validations', () => {
roles: ['ORGANIZATION_ADMIN'],
agency: scenario.agency.one,
})
expect(
async () =>
await runPermissionsCreateOrUpdateValidations({
role: ROLES.ORGANIZATION_STAFF,
agencyId: scenario.agency.three.id,
})
await expect(
runPermissionsCreateOrUpdateValidations({
role: ROLES.ORGANIZATION_STAFF,
agencyId: scenario.agency.three.id,
})
).rejects.toThrow("You don't have permission to do that")
}
)
Expand All @@ -487,12 +480,11 @@ describe('permissions write validations', () => {
email: scenario.user.two.email,
roles: ['ORGANIZATION_ADMIN'],
})
expect(
async () =>
await runPermissionsCreateOrUpdateValidations({
role: ROLES.ORGANIZATION_STAFF,
agencyId: scenario.agency.one.id,
})
await expect(
runPermissionsCreateOrUpdateValidations({
role: ROLES.ORGANIZATION_STAFF,
agencyId: scenario.agency.one.id,
})
).rejects.toThrow("You don't have permission to do that")
}
)
Expand All @@ -508,15 +500,14 @@ describe('update specific validations', () => {
roles: ['USDR_ADMIN'],
})
await expect(
async () =>
await runUpdateSpecificValidations(
{
role: ROLES.ORGANIZATION_STAFF,
agencyId: scenario.agency.one.id,
},
scenario.user.one.id
)
).not.toThrowError()
runUpdateSpecificValidations(
{
role: ROLES.ORGANIZATION_STAFF,
agencyId: scenario.agency.one.id,
},
scenario.user.one.id
)
).resolves.not.toThrow()
}
)

Expand All @@ -529,15 +520,14 @@ describe('update specific validations', () => {
roles: ['ORGANIZATION_ADMIN'],
})
await expect(
async () =>
await runUpdateSpecificValidations(
{
role: ROLES.ORGANIZATION_STAFF,
agencyId: scenario.agency.one.id,
},
scenario.user.three.id
)
).not.toThrowError()
runUpdateSpecificValidations(
{
role: ROLES.ORGANIZATION_STAFF,
agencyId: scenario.agency.one.id,
},
scenario.user.three.id
)
).resolves.not.toThrow()
}
)

Expand All @@ -550,14 +540,13 @@ describe('update specific validations', () => {
roles: ['ORGANIZATION_ADMIN'],
})
await expect(
async () =>
await runUpdateSpecificValidations(
{
role: ROLES.ORGANIZATION_STAFF,
agencyId: scenario.agency.one.id,
},
scenario.user.four.id
)
runUpdateSpecificValidations(
{
role: ROLES.ORGANIZATION_STAFF,
agencyId: scenario.agency.one.id,
},
scenario.user.four.id
)
).rejects.toThrow('agencyId is invalid or unavailable to this user')
}
)
Expand Down
18 changes: 8 additions & 10 deletions api/src/services/users/users.ts
Original file line number Diff line number Diff line change
Expand Up @@ -99,21 +99,19 @@ export const runPermissionsCreateOrUpdateValidations = async (input) => {
/**
* Validations specific to updating a user
*/
export const runUpdateSpecificValidations = async (input, id) => {
export const runUpdateSpecificValidations = async (input, userId) => {
// If not USDR admin, changes to agencyId are only allowed if the new agency ID is in the same organization
// as the user's current agency (e.g., you can't swap a user between organizations)
await validateWith(async () => {
if (!currentUserIsUSDRAdmin()) {
const currentAgency = (
await db.user.findUniqueOrThrow({
where: { id },
select: {
agency: {
select: { id: true, organizationId: true },
},
const { agency: currentAgency } = await db.user.findUniqueOrThrow({
where: { id: userId },
select: {
agency: {
select: { id: true, organizationId: true },
},
})
).agency
},
})

if (input.agencyId === currentAgency.id) {
return
Expand Down

0 comments on commit c79b821

Please sign in to comment.