Skip to content
Open
Show file tree
Hide file tree
Changes from all 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
41 changes: 39 additions & 2 deletions .workback/workback.md
Original file line number Diff line number Diff line change
@@ -1,2 +1,39 @@
- Always use `pnpm` to install dependencies and run scripts
- Use the Node.js version defined in package.json file under "engines". Check the version with `node -v` and change it with `nvm use` if needed.
- ALWAYS use `pnpm` run scripts
- You can assume that the server is running and the database is up at `localhost:5050` and running at `localhost:3000`
- Here are the steps I ran to start the development environment, you can use them as a reference:

```bash
nvm use 22.14
cp .env.example .env
pnpm i
# Start docker manually
pnpm db:start
pnpm relay:build
pnpn dev
```

We gave ourselves super user permissions to play around in http://localhost:3000/admin/graphql with this script:

```
pnpm node ./scripts/toolbox/assignSURole.js --add you@example.com
```

You can run GraphQL mutations in `localhost:3000/admin/graphql`:

```graphql
mutation upgradetoEnterprise {
draftEnterpriseInvoice(
orgId: "XXXXX",
quantity: <number>,
email: "exampeuser@email.com",
apEmail: "invoice@email.com",
plan: "stripe_api_price_id") {
organization {
tier
}
error {
message
}
}
}
```
84 changes: 84 additions & 0 deletions packages/server/__tests__/teamAgendaItemsResolver.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,84 @@
import Team from '../graphql/types/Team'
import {isTeamMember} from '../utils/authorization'

// Mock the authorization module
jest.mock('../utils/authorization', () => ({
isTeamMember: jest.fn()
}))

describe('Team.agendaItems resolver', () => {
// Extract the agendaItems resolver from the Team type
const agendaItemsResolver = Team.getFields().agendaItems?.resolve
if (!agendaItemsResolver) {
throw new Error('agendaItems resolver is not defined')
}

test('should return null for non-team members, causing GraphQL error', async () => {
// Set up mocks
const teamId = 'team123'
const mockTeam = {id: teamId}
const mockAuthToken = {sub: 'user123', tms: ['otherTeam123']}
const mockDataLoader = {
get: jest.fn().mockReturnValue({
load: jest.fn().mockResolvedValue([])
})
}
const mockContext = {authToken: mockAuthToken, dataLoader: mockDataLoader}

// Mock the authorization check to simulate a non-team member
const mockedIsTeamMember = isTeamMember as jest.Mock
mockedIsTeamMember.mockReturnValue(false)

// Call the resolver
const result = await agendaItemsResolver(mockTeam, {}, mockContext, {} as any)

// Verify the resolver returns null for non-team members
expect(result).toBeNull()
expect(isTeamMember).toHaveBeenCalledWith(mockAuthToken, teamId)

console.log(`
TEST CONFIRMS BUG: Team.agendaItems resolver returns null for non-team members.

This causes the GraphQL error: "Cannot return null for non-nullable field Team.agendaItems"
because the field is defined as non-nullable in the schema:

type: new GraphQLNonNull(new GraphQLList(new GraphQLNonNull(AgendaItem)))

FIX: Change the resolver implementation from:
if (!isTeamMember(authToken, teamId)) return null

To:
if (!isTeamMember(authToken, teamId)) return []
`)
})

test('should return an empty array for non-team members after fix', async () => {
// Set up mocks
const teamId = 'team123'
const mockTeam = {id: teamId}
const mockAuthToken = {sub: 'user123', tms: ['otherTeam123']}
const mockDataLoader = {
get: jest.fn().mockReturnValue({
load: jest.fn().mockResolvedValue([])
})
}
const mockContext = {authToken: mockAuthToken, dataLoader: mockDataLoader}

// Mock the authorization check to simulate a non-team member
const mockedIsTeamMember = isTeamMember as jest.Mock
mockedIsTeamMember.mockReturnValue(false)

// Call the resolver
const result = await agendaItemsResolver(mockTeam, {}, mockContext, {} as any)

// IMPORTANT: This test will fail until you fix the resolver!
// After the fix, it should return an empty array instead of null
expect(Array.isArray(result)).toBe(true)
expect(result).toEqual([])

console.log(`
After the fix, this test should pass, confirming that the resolver now returns
an empty array for non-team members, which satisfies the GraphQL schema requirements.
`)
})
})
2 changes: 1 addition & 1 deletion packages/server/graphql/types/Team.ts
Original file line number Diff line number Diff line change
Expand Up @@ -272,7 +272,7 @@ const Team: GraphQLObjectType = new GraphQLObjectType<ITeam, GQLContext>({
_args: unknown,
{authToken, dataLoader}: GQLContext
) {
if (!isTeamMember(authToken, teamId)) return null
if (!isTeamMember(authToken, teamId)) return []

Choose a reason for hiding this comment

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

I don't think this is the right call. While it fixes the symptom, it would be good to know why the client is requesting agendaItems for non-teammembers as it probably points to a (slightly) broader issue

Copy link
Member Author

@arthurgousset arthurgousset May 23, 2025

Choose a reason for hiding this comment

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

Interesting, good point @Dschoordsch. For simplicity, I'll consider that a new (broader) issue #5 and consider this (smaller) issue reproduced. Should we land a fix for the symptom in the mean time? If so, I can clean this up and contribute the small fix in the mean time.

@priyankc will run WorkBack with your feedback and check if it can reproduce the bug #5 more upstream to find a broader issue. We'll open a separate PR for that to keep things clean.

Choose a reason for hiding this comment

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

I would prefer if we look for the real fix first. If we just make this error case not an error anymore, we will just hide the potential logic issue.
The backtrace shows it's happening during accepting a team invitation. So I think the user in question might in fact be a team member, but the subscription is evaluated before the auth token of that user was updated.

Copy link
Member Author

Choose a reason for hiding this comment

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

Great feedback and thanks for the pointers. @priyankc will take ownership of this issue from here 👍

return dataLoader.get('agendaItemsByTeamId').load(teamId)
}
},
Expand Down