[Server Integration tests] Enrich integration GraphQL API tests #2 #7978
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
![gitstart-app[bot]](https://avatars.githubusercontent.com/in/46283?s=60&v=4){kind=small}
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
There was a problem hiding this comment.
PR Summary
This pull request introduces comprehensive integration tests for various GraphQL API resolvers in the Twenty server. Here's a summary of the key changes:
- Added
TIM_USER_IDconstant for consistent user identification in tests - Implemented extensive test suites for API keys, attachments, audit logs, blocklists, calendar channel event associations, and calendar channels
- Each test suite covers CRUD operations, including edge cases and error handling
- Special handling for workspace members, preventing multiple deletions as required
Key points:
- Tests ensure API behaves correctly for single and multiple record operations
- Soft deletion and permanent removal scenarios are covered
- Test suites are designed to be independent and leave no side effects
- Consistent use of utility functions for creating GraphQL operations and making API requests
8 file(s) reviewed, 6 comment(s)
Edit PR Review Bot Settings | Greptile
| expect(destroyApiKeyResponse.body.data.destroyApiKey).toBeTruthy(); | ||
| }); | ||
|
|
||
| it('9. should not find many API keys anymore', async () => { |
There was a problem hiding this comment.
style: Test could be combined with test on line 257 to reduce duplication
Suggested change
| it('9. should not find many API keys anymore', async () => { | |
| it('9. should not find many API keys anymore', async () => { | |
| const graphqlOperation = findManyOperationFactory({ | |
| objectMetadataSingularName: 'apiKey', | |
| objectMetadataPluralName: 'apiKeys', | |
| gqlFields: API_KEY_GQL_FIELDS, | |
| filter: { | |
| id: { | |
| in: [API_KEY_1_ID, API_KEY_2_ID], | |
| }, | |
| not: { | |
| deletedAt: { | |
| is: 'NULL', | |
| }, | |
| }, | |
| }, | |
| }); | |
| const response = await makeGraphqlAPIRequest(graphqlOperation); | |
| expect(response.body.data.apiKeys.edges).toHaveLength(0); | |
| }); |
| expect(createdAttachment).toHaveProperty('opportunityId'); | ||
| }); | ||
|
|
||
| it('2. should find many attachments', async () => { |
There was a problem hiding this comment.
syntax: Duplicate test number. Should be '3.' instead of '2.'
Suggested change
| it('2. should find many attachments', async () => { | |
| it('3. should find many attachments', async () => { |
| }, | ||
| }); | ||
|
|
||
| const findAuditLogsResponse = await makeGraphqlAPIRequest(graphqlOperation); |
There was a problem hiding this comment.
style: Rename variable to 'response' for consistency with other tests
Comment on lines
+373
to
+374
| const destroyAuditLogResponse = | ||
| await makeGraphqlAPIRequest(graphqlOperation); |
There was a problem hiding this comment.
style: Unnecessary variable assignment. Use inline await
Comment on lines
+194
to
+204
| if (data.edges.length > 0) { | ||
| const associations = data.edges[0].node; | ||
|
|
||
| expect(associations).toHaveProperty('eventExternalId'); | ||
| expect(associations).toHaveProperty('id'); | ||
| expect(associations).toHaveProperty('createdAt'); | ||
| expect(associations).toHaveProperty('updatedAt'); | ||
| expect(associations).toHaveProperty('deletedAt'); | ||
| expect(associations).toHaveProperty('calendarChannelId'); | ||
| expect(associations).toHaveProperty('calendarEventId'); | ||
| } |
There was a problem hiding this comment.
style: Check if data exists before asserting properties
| expect(data).toBeDefined(); | ||
| expect(Array.isArray(data.edges)).toBe(true); | ||
|
|
||
| if (data.edges.length > 0) { |
There was a problem hiding this comment.
style: Check if data.edges is empty before accessing
Suggested change
| if (data.edges.length > 0) { | |
| if (data.edges && data.edges.length > 0) { | |
| const calendarChannels = data.edges[0].node; | |
| // ... existing assertions | |
| } |
charlesBochet
approved these changes
Oct 24, 2024
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This PR was created by GitStart to address the requirements from this ticket: TWNTY-7526.
Description
For workspace members, the deletion of multiple members is a special case that is not permitted by the method for regular users. As a result, we ensure that multiple deletions are not processed.
For certain tests, both an account ID and a user ID are required. We are utilizing Tim's account for all testing purposes, as specified by the token in
jest-integration.config.ts. To streamline this process, we have defined a constant to store and reference the account ID and user ID during testing.Refs#7526
Dem