Add seasons table #195
Add seasons table #195
Conversation
WalkthroughThis update introduces a new Changes
Sequence Diagram(s)sequenceDiagram
participant Client
participant Resolver
participant Repository
participant SeasonEntity
participant RelatedEntity
Client->>Resolver: Request (with seasonNumber)
Resolver->>Repository: findSeasonByNumber(seasonNumber)
Repository->>SeasonEntity: Query by seasonNumber
SeasonEntity-->>Repository: Season (with id)
Repository-->>Resolver: Season (with id)
Resolver->>Repository: Operation using seasonId (from Season)
Repository->>RelatedEntity: Query/Update with seasonId
RelatedEntity-->>Repository: Result
Repository-->>Resolver: Result
Resolver-->>Client: Response
Suggested reviewers
Poem
Tip ⚡💬 Agentic Chat (Pro Plan, General Availability)
📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (3)
✅ Files skipped from review due to trivial changes (1)
🚧 Files skipped from review as they are similar to previous changes (1)
🧰 Additional context used🧬 Code Graph Analysis (1)src/resolvers/projectResolver.ts (2)
🔇 Additional comments (3)
✨ Finishing Touches
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
…ges for no active season
…onsTable migration
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🔭 Outside diff range comments (3)
src/repositories/projectUserRecordRepository.test.ts (1)
302-365:⚠️ Potential issueInconsistency: Test appears to use same seasonId when testing different seasons.
This test claims to test "reset individual caps for each season", but both "Season 1" and "Season 2" tests are using the same seasonId (
season.id). If testing different seasons, you should create a second Season entity with a different seasonNumber.// Season 2 donations - should start fresh const season2EaRound = await saveEARoundDirectlyToDb({ roundNumber: generateEARoundNumber(), startDate: new Date('2024-10-01'), endDate: new Date('2024-10-05'), - seasonId: season.id, + seasonId: (await Season.create({ + seasonNumber: 2, + startDate: moment().subtract(1, 'month').toDate(), + endDate: moment().add(1, 'month').toDate(), + }).save()).id, });The same issue exists with the season record queries at lines 349 and 359.
src/repositories/earlyAccessRoundRepository.test.ts (1)
101-139:⚠️ Potential issueCheck date creation in active round test.
In the active round test, there's a potential issue with how dates are created:
startDate: new Date(new Date().setDate(new Date().getDate() - 1)), // yesterday endDate: new Date(new Date().setDate(new Date().getDate() + 1)), // tomorrowAnd for the inactive round:
startDate: new Date(new Date().getDate() + 1), endDate: new Date(new Date().getDate() + 2),The inactive round is using
new Date(number)which creates a date from a timestamp.new Date().getDate()returns the day of month (1-31), which is likely not the intended behavior.const inactiveRoundData = { roundNumber: generateEARoundNumber(), seasonId: season.id, - startDate: new Date(new Date().getDate() + 1), - endDate: new Date(new Date().getDate() + 2), + startDate: new Date(new Date().setDate(new Date().getDate() + 1)), + endDate: new Date(new Date().setDate(new Date().getDate() + 2)), roundPOLCapPerProject: 1000000, roundPOLCapPerUserPerProject: 50000, };src/services/qAccService.ts (1)
28-41: 💡 Verification agent🧩 Analysis chain
Consider adding foreign key validation.
The seasonId parameter is accepted without validation. Consider adding a check to verify that the provided seasonId exists in the database to prevent potential runtime errors with invalid foreign keys.
// Add this at the beginning of getUserProjectSeasonRecord if (seasonId) { const seasonExists = await Season.findOne({ where: { id: seasonId } }); if (!seasonExists) { throw new Error(`Season with ID ${seasonId} not found`); } }This suggestion is based on a general best practice for foreign key validation. Let's check if there's any existing pattern for validation in the codebase:
🏁 Script executed:
#!/bin/bash # Check for similar parameter validation patterns in the codebase rg -A 2 'if \(\w+Id\)' --type tsLength of output: 8387
Action Required: Add Foreign Key Validation for seasonId
We’ve observed that the
getUserProjectSeasonRecordfunction insrc/services/qAccService.tsaccepts aseasonIdparameter without checking its validity in the database. While similar conditional checks (e.g.,if (seasonId)) exist elsewhere (as seen insrc/repositories/projectUserRecordRepository.ts), they only adjust query conditions rather than verify that the related foreign record exists.To prevent possible runtime errors due to an invalid
seasonId, please add a foreign key validation at the beginning of the function. For instance, consider inserting:if (seasonId) { const seasonExists = await Season.findOne({ where: { id: seasonId } }); if (!seasonExists) { throw new Error(`Season with ID ${seasonId} not found`); } }This change makes sure that only a valid
seasonIdis used, aligning with best practices for ensuring data integrity in the codebase.
🧹 Nitpick comments (10)
src/repositories/projectUserRecordRepository.test.ts (1)
45-46: Replace await Promise.all with individual awaits for related deletion operations.You're correctly using
awaitfor the deletion operations in the afterEach hook, which prevents test interference. However, consider usingPromise.allfor parallel execution when deleting related entities:if (eaRound) { - await Donation.delete({ earlyAccessRoundId: eaRound.id }); - await ProjectRoundRecord.delete({ earlyAccessRoundId: eaRound.id }); + await Promise.all([ + Donation.delete({ earlyAccessRoundId: eaRound.id }), + ProjectRoundRecord.delete({ earlyAccessRoundId: eaRound.id }) + ]); await eaRound.remove(); eaRound = null; }Also applies to: 111-162
src/entities/earlyAccessRound.ts (1)
92-94: Query filter updated correctly for season relationship.The query filter in calculateCumulativeCaps has been properly updated to filter by seasonId instead of seasonNumber.
Consider updating the cache key to include seasonId:
- .cache('cumulativeCapEarlyAccessRound-' + this.roundNumber, 300000) + .cache('cumulativeCapEarlyAccessRound-' + this.seasonId + '-' + this.roundNumber, 300000)This would prevent cache collisions if different seasons have rounds with the same number.
src/repositories/seasonRepository.test.ts (2)
72-99: Active season test handles date conditions correctly.The test creates past, current, and future seasons and correctly verifies that only the current season is returned as active.
Consider specifying a fixed date for testing instead of using moment() without parameters:
- const foundActiveSeason = await findActiveSeasonByDate(); + const testDate = moment().toDate(); + const foundActiveSeason = await findActiveSeasonByDate(testDate);This would make the test more deterministic and less dependent on the execution time.
1-147: Consider adding additional test cases.While the test suite covers the basic functionality well, consider adding tests for:
- Overlapping seasons handling
- Season date validation (startDate before endDate)
- Edge cases like seasons with same start/end dates
src/entities/qfRound.ts (1)
171-187: Query filter updated correctly for season relationship.The calculateCumulativeCaps method has been properly updated to check for seasonId and filter by it in the query. This ensures proper isolation of data between seasons.
Consider updating the cache key to include seasonId:
- .cache('cumulativeCapQfRound-' + this.roundNumber, 300000) + .cache('cumulativeCapQfRound-' + this.seasonId + '-' + this.roundNumber, 300000)This would prevent cache collisions if different seasons have rounds with the same number.
src/entities/season.ts (2)
25-32: Date fields are properly defined.The startDate and endDate fields are correctly configured for both database storage and GraphQL exposure.
Consider adding validation logic to ensure startDate is before endDate, either through a check constraint or an entity validation method.
1-48: Consider adding useful helper methods to the Season entity.The Season entity could benefit from additional helper methods, such as:
- A method to check if a given date falls within the season
- A method to calculate the season's duration
Here's an example implementation:
@Entity() @ObjectType() export class Season extends BaseEntity { // existing fields... // Helper method to check if a date falls within this season isDateInSeason(date: Date): boolean { return date >= this.startDate && date <= this.endDate; } // Helper method to get duration in days getDurationInDays(): number { return Math.ceil((this.endDate.getTime() - this.startDate.getTime()) / (1000 * 60 * 60 * 24)); } }src/repositories/seasonRepository.ts (3)
4-8: Simple finder by ID is straightforward.
Returningnullif not found is acceptable. Consider throwing an error or returning a typed result if a not-found state requires specialized handling.
10-16: Finder by season number is clear.
Again, consider validating the result or throwing a custom error if no record is found to reduce null checks in downstream code.
37-49: Season creation logic looks solid.
Parameter destructuring and direct save are straightforward. Consider validating conflicts, e.g., overlapping date ranges with other seasons.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (21)
migration/1744677815994-addSeasonsTable.ts(1 hunks)package.json(1 hunks)src/entities/earlyAccessRound.ts(3 hunks)src/entities/entities.ts(2 hunks)src/entities/project.ts(2 hunks)src/entities/projectUserRecord.ts(2 hunks)src/entities/qfRound.ts(5 hunks)src/entities/season.ts(1 hunks)src/repositories/earlyAccessRoundRepository.test.ts(10 hunks)src/repositories/projectUserRecordRepository.test.ts(28 hunks)src/repositories/projectUserRecordRepository.ts(4 hunks)src/repositories/qfRoundRepository.test.ts(11 hunks)src/repositories/seasonRepository.test.ts(1 hunks)src/repositories/seasonRepository.ts(1 hunks)src/resolvers/donationResolver.test.ts(2 hunks)src/resolvers/donationResolver.ts(1 hunks)src/resolvers/qAccResolver.test.ts(4 hunks)src/resolvers/qAccResolver.ts(2 hunks)src/resolvers/roundsResolver.test.ts(16 hunks)src/services/qAccService.test.ts(8 hunks)src/services/qAccService.ts(5 hunks)
🧰 Additional context used
🧬 Code Graph Analysis (9)
src/resolvers/qAccResolver.test.ts (1)
test/testUtils.ts (1)
generateEARoundNumber(2080-2082)
src/services/qAccService.test.ts (1)
test/testUtils.ts (1)
generateEARoundNumber(2080-2082)
src/repositories/seasonRepository.test.ts (1)
src/repositories/seasonRepository.ts (5)
createSeason(37-49)findSeasonById(4-8)findSeasonByNumber(10-16)findActiveSeasonByDate(18-27)findAllSeasons(29-35)
src/services/qAccService.ts (2)
src/repositories/projectUserRecordRepository.ts (1)
updateOrCreateProjectUserRecord(4-64)src/resolvers/roundsResolver.ts (1)
activeRound(57-69)
src/repositories/earlyAccessRoundRepository.test.ts (1)
test/testUtils.ts (2)
generateEARoundNumber(2080-2082)saveEARoundDirectlyToDb(2068-2073)
src/resolvers/roundsResolver.test.ts (1)
test/testUtils.ts (1)
generateEARoundNumber(2080-2082)
src/resolvers/qAccResolver.ts (2)
src/repositories/seasonRepository.ts (1)
findSeasonByNumber(10-16)src/repositories/projectUserRecordRepository.ts (1)
getProjectUserRecordAmount(71-90)
src/resolvers/donationResolver.test.ts (1)
test/testUtils.ts (1)
generateEARoundNumber(2080-2082)
src/entities/projectUserRecord.ts (1)
src/entities/season.ts (1)
Entity(14-48)
🔇 Additional comments (62)
package.json (1)
1-248: Formatting improvement only—no functional changes.This change only improves readability and does not affect functionality. No further action required.
src/entities/entities.ts (2)
42-42: Season entity import is correct and necessary.The import of the
Seasonentity is required for its registration and use in the ORM.
96-97: Season entity registration is correct.Adding
Seasonto the entities array ensures it is managed by TypeORM and available for migrations and queries.src/resolvers/donationResolver.ts (1)
999-1001: Correct migration to seasonId.The update from
seasonNumbertoseasonIdand the new value source is correct and aligns with the new relational model.src/resolvers/donationResolver.test.ts (3)
71-71: Good addition of Season importThis import brings in the Season entity which is now needed for the test setup. It aligns with the broader schema changes replacing primitive seasonNumber fields with proper relations.
5087-5092: Proper Season setup for testsGood implementation of clearing existing Season records and creating a test Season entity with appropriate date ranges before testing. This ensures the test environment is properly initialized with the new Season entity that early access rounds now depend on.
5095-5095: Correct usage of seasonId in EarlyAccessRoundThe EarlyAccessRound now correctly references the Season entity via seasonId instead of using the previous seasonNumber field. This aligns with the database schema changes.
src/resolvers/qAccResolver.test.ts (5)
39-39: Good addition of Season importAdding the Season import is necessary to support the new entity relationship model where rounds are associated with a Season entity instead of using a primitive seasonNumber field.
152-152: Appropriate Season class variable declarationAdding a class-level Season variable helps maintain consistency by providing a single Season instance that can be referenced by all test rounds.
155-159: Well-implemented Season initializationThe Season setup is properly implemented with appropriate seasonNumber and date ranges. This initialization is essential for the test setup since EarlyAccessRound and QfRound entities now depend on a valid Season entity.
168-168: Consistent Season association across all EarlyAccessRoundsEach EarlyAccessRound entity now correctly references the Season entity through seasonId. This is consistent with the database schema changes and ensures proper relationships are maintained.
Also applies to: 176-176, 184-184, 192-192
202-202: Proper Season association for QfRoundThe QfRound entity now correctly references the Season entity through seasonId, aligning with the database schema changes and maintaining consistency with EarlyAccessRound associations.
src/resolvers/qAccResolver.ts (2)
16-16: Good addition of Season repository importThe findSeasonByNumber import is necessary to support the translation from seasonNumber to seasonId in the resolver.
69-81: Well-implemented seasonNumber to seasonId translationThe implementation properly translates the optional seasonNumber parameter to a seasonId by looking up the corresponding Season entity. It includes appropriate error handling when a Season is not found.
This change maintains backward compatibility by keeping the API signature unchanged while adapting the internal implementation to work with the new relational model.
src/resolvers/roundsResolver.test.ts (5)
19-19: Correctly importing the new Season entityThe import of the new
Seasonentity is properly added which is necessary for the subsequent changes in the test file.
24-24: Season integration in fetchAllRoundsTestCases is well implementedThe test setup now properly creates a Season instance before creating Early Access Rounds and QF Rounds, and cleans up Season data during both setup and teardown phases. This ensures test isolation and provides the necessary test fixture for the relationships established in the refactored entities.
Also applies to: 39-44
68-69: Correctly updated seasonNumber to seasonId in round creationAll references to the previously used
seasonNumber: 1in the creation ofEarlyAccessRoundandQfRoundentities have been properly replaced withseasonId: season.id, explicitly linking rounds to the createdSeasonentity.Also applies to: 77-78, 89-90, 102-103
180-200: Season integration in fetchActiveRoundTestCases is properly implementedSimilar to the first test case, the
fetchActiveRoundTestCasesfunction now correctly creates and manages a Season instance. The test setup follows the same pattern of creating a Season before rounds, and cleaning up all Season data after tests complete.
224-229: Consistent application of seasonId across all test casesThe Season relationship is consistently applied across all test cases in the file. Every instance of round creation now references the Season entity via its ID, maintaining data consistency and relationship integrity.
Also applies to: 236-244, 269-272, 279-287, 311-319, 338-346, 363-366, 373-380
src/repositories/qfRoundRepository.test.ts (4)
27-27: Correctly importing the new Season entityThe import of the
Seasonentity is properly added, which is necessary for the subsequent changes in the test file.
466-467: Minor formatting improvementA blank line was added to improve code readability, which is a good practice to separate logical code blocks.
504-524: Season entity properly integrated in QF Round testsThe test setup now correctly creates a Season instance before creating QF Rounds and properly cleans up Season data during both setup and teardown phases. This ensures test isolation and provides the necessary fixtures for the entity relationships.
568-569: Consistently replaced seasonNumber with seasonId in QF Round creationAll references to the previously used
seasonNumberfield in creatingQfRoundentities have been properly replaced withseasonId: season.id, explicitly linking rounds to the createdSeasonentity.Also applies to: 581-582, 594-595, 620-621, 633-634, 645-646
src/services/qAccService.test.ts (4)
23-23: Correctly importing and cleaning up the new Season entityThe import of the
Seasonentity is properly added, and the test setup now cleans up Season data in thebeforehook, which is essential for test isolation.Also applies to: 30-30
37-38: Well-structured Season setup for different test scenariosThe test now creates two Season instances with different season numbers, allowing tests to verify behavior across different seasons. The setup defines proper date ranges for each season, which is good for comprehensive test coverage.
Also applies to: 61-71
77-78: Early Access Rounds and QF Rounds properly linked to seasonsAll references to
seasonNumberin the creation ofEarlyAccessRoundandQfRoundentities have been correctly replaced withseasonId: season.id, explicitly linking rounds to the createdSeasonentity.Also applies to: 85-86, 93-94, 101-102, 114-115, 128-129
551-552: Consistent usage of both seasons in advanced test casesThe test properly uses both defined seasons (
seasonandseason2) in different test scenarios, providing good test coverage for cross-season behaviors. This is especially valuable for testing the cumulative donation caps across seasons.Also applies to: 687-688, 805-806
src/entities/project.ts (2)
50-50: Correctly importing the new Season entityThe import of the
Seasonentity is properly added, which is necessary for establishing the entity relationship.
500-507: Well-structured Season relationship in Project entityThe changes properly implement:
- A GraphQL field exposing the Season entity
- A many-to-one relationship between Project and Season
- A seasonId column with the RelationId decorator
This is a well-structured implementation following TypeORM and type-graphql best practices, replacing the primitive seasonNumber field with a proper entity relationship.
src/repositories/projectUserRecordRepository.test.ts (1)
29-59: Proper setup and cleanup for Season entity.Good implementation of the Season entity in the test setup and cleanup. Creating the Season in the beforeEach and properly cleaning it up in afterEach ensures test isolation.
src/repositories/earlyAccessRoundRepository.test.ts (2)
3-14: Proper import of moment and Season entity.Good addition of the moment library for date handling and the Season entity. The moment library provides a clean way to define relative dates for season start and end.
20-29: Well-structured test setup with proper cleanup.Good implementation of test setup that cleans existing data and creates a Season entity with appropriate dates. This ensures each test starts with a clean state.
src/services/qAccService.ts (2)
72-107: Good parameter and type updates for seasonId.You've correctly updated the getUserProjectSeasonRecord function to use seasonId instead of seasonNumber, including the function signature, type definitions, and query parameters.
179-183: Consistent seasonId usage across all function calls.All function calls to getUserProjectSeasonRecord have been updated to use seasonId from the respective round entity, maintaining consistency with the new Season model.
Also applies to: 202-206, 270-273
src/repositories/projectUserRecordRepository.ts (3)
4-12: Updated function signature with seasonId parameter.The function signature has been properly updated to use seasonId instead of seasonNumber, maintaining the optional nature of the parameter.
13-50: SQL query updates for seasonId use in joins and conflict resolution.The SQL query has been properly updated to:
- Use seasonId in the INSERT statement
- Join with qf_round and early_access_round tables using seasonId
- Use seasonId in the ON CONFLICT clause
These changes correctly implement the Season entity relationship in the database queries.
71-84: Updated getProjectUserRecordAmount function with seasonId parameter.The getProjectUserRecordAmount function has been properly updated to use seasonId in both its signature and database query.
src/entities/earlyAccessRound.ts (3)
11-16: Dependencies correctly updated to support the new Season relationship.The necessary imports for TypeORM's relationship decorators and the new Season entity have been added appropriately.
30-36: Season relationship properly implemented.The ManyToOne relationship to the Season entity is correctly established, replacing the previous primitive seasonNumber field. The GraphQL field is properly exposed with the correct type.
34-36: RelationId mapping is correctly configured.The RelationId decorator properly maps the seasonId column from the season relationship, maintaining database integrity.
src/repositories/seasonRepository.test.ts (6)
1-11: Test dependencies and imports are correctly set up.All necessary imports for testing the Season repository functionality are properly included.
12-21: Clean testing environment with proper setup and teardown.The test suite correctly cleans up data before and after each test to ensure test isolation.
23-42: Create season test is well-implemented.The test correctly verifies the creation of a new season with proper assertions for all fields.
44-56: Find by ID and number tests are correctly implemented.Both tests properly verify the functionality to find seasons by their unique identifiers.
Also applies to: 58-70
101-127: Order of seasons is correctly tested.The test properly verifies that seasons are returned in descending order by seasonNumber.
129-146: Null case for active season is properly tested.The test correctly handles the scenario where no active season exists for the current date.
src/entities/qfRound.ts (3)
13-18: Dependencies correctly updated to support the new Season relationship.The necessary imports for TypeORM's relationship decorators and the new Season entity have been added appropriately.
32-38: Season relationship properly implemented.The ManyToOne relationship to the Season entity is correctly established, replacing the previous primitive seasonNumber field. The GraphQL field is properly exposed with the correct type.
197-202: Fallback logic for rounds without season is preserved.The fallback logic for rounds without a season association has been maintained and the comment was updated to reflect the new terminology.
src/entities/season.ts (4)
1-13: New Season entity imports are correct.All necessary imports for TypeORM entity definition and GraphQL type generation are included.
33-39: Relationship to Project entities is properly defined.The one-to-many relationship to Project entities is correctly configured with bidirectional mapping.
37-40: Relationship to ProjectUserRecord entities is properly defined.The one-to-many relationship to ProjectUserRecord entities is correctly configured with bidirectional mapping.
41-47: Timestamp fields are properly configured.The createdAt and updatedAt timestamp fields are correctly set up with TypeORM decorators to handle automatic updates.
src/entities/projectUserRecord.ts (3)
13-13: Import of Season entity looks appropriate.
No issues found; this aligns with the new many-to-one relation.
17-17: Verify uniqueness handling for nullable seasonId.
In PostgreSQL, multipleNULLvalues do not violate a unique index, so it’s possible for many records to share aNULLseasonId. Ensure this behavior is desired (i.e., multiple records without a season are allowed).
55-62: Relation to Season entity is consistent with domain model.
BecauseseasonIdis nullable, confirm that existing records can remain seasonless or that the application logic gracefully handles records without a season. If this matches the business rules, it looks good.src/repositories/seasonRepository.ts (3)
1-2: Imports are adequately scoped.
UsingLessThanOrEqualandMoreThanOrEqualfrom TypeORM is fine for date range checks. No concerns.
18-27: Date-based active season retrieval is well-structured.
The usage ofLessThanOrEqual(date)andMoreThanOrEqual(date)is correct for inclusive range checks.
29-35: Fetching all seasons in descending order is correct.
This helps retrieve the most recent season first. No issues found.migration/1744677815994-addSeasonsTable.ts (3)
1-5: Migration class name and structure are appropriate.
No immediate concerns.
6-52: Up migration correctly creates the seasons table and updates foreign keys.
Make sure there is a plan for existing data referencingseasonNumber. If needed, a data-migration step that maps those old values to newly created Season rows should be included so legacy records aren’t orphaned.
54-99: Down migration properly reverses schema changes.
This is consistent with TypeORM’s convention, ensuring a clean rollback if necessary.
| if (seasonId) { | ||
| query += ` AND ( | ||
| (donation."qfRoundId" IS NOT NULL AND EXISTS ( | ||
| SELECT 1 FROM qf_round | ||
| WHERE qf_round.id = donation."qfRoundId" | ||
| AND qf_round."seasonNumber" = $3 | ||
| AND qf_round."seasonId" = $3 | ||
| )) OR | ||
| (donation."earlyAccessRoundId" IS NOT NULL AND EXISTS ( | ||
| SELECT 1 FROM early_access_round | ||
| WHERE early_access_round.id = donation."earlyAccessRoundId" | ||
| AND early_access_round."seasonNumber" = $3 | ||
| AND early_access_round."seasonId" = $3 | ||
| )) | ||
| )`; | ||
| } |
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
Consider adding transaction support for data consistency.
The function performs a complex INSERT with ON CONFLICT resolution, but it's not wrapped in a transaction. Consider adding transaction support to ensure data consistency, especially when this function is called in high-concurrency scenarios.
export async function updateOrCreateProjectUserRecord({
projectId,
userId,
seasonId,
}: {
projectId: number;
userId: number;
seasonId?: number;
}): Promise<ProjectUserRecord> {
+ // Use a transaction to ensure data consistency
+ const queryRunner = ProjectUserRecord.getRepository().manager.connection.createQueryRunner();
+ await queryRunner.connect();
+ await queryRunner.startTransaction();
+
+ try {
let query = `
INSERT INTO project_user_record ("projectId", "userId", "seasonId", "eaTotalDonationAmount", "qfTotalDonationAmount", "totalDonationAmount")
// ... rest of the query ...
`;
// ... query construction ...
- const result = await ProjectUserRecord.query(query, [
+ const result = await queryRunner.query(query, [
projectId,
userId,
seasonId || null,
[
DONATION_STATUS.VERIFIED,
DONATION_STATUS.PENDING,
DONATION_STATUS.SWAP_PENDING,
],
]);
+ await queryRunner.commitTransaction();
return result[0];
+ } catch (error) {
+ await queryRunner.rollbackTransaction();
+ throw error;
+ } finally {
+ await queryRunner.release();
+ }
}Let's check if transactions are used elsewhere in the codebase for similar operations:
🏁 Script executed:
#!/bin/bash
# Check for transaction patterns in the repository
rg -A 2 'startTransaction|createQueryRunner' --type tsLength of output: 1785
Transaction Handling: Ensure Atomicity in Project User Record Updates
The current implementation of the INSERT with ON CONFLICT resolution in src/repositories/projectUserRecordRepository.ts does not use transaction support, which may lead to data inconsistencies under high concurrency. Similar transaction patterns from other parts of the codebase (e.g., in src/resolvers/reactionResolver.ts and test scripts) indicate the preferred approach for managing complex operations.
- Wrap the INSERT operation in a transaction to guarantee atomicity.
- Use a query runner to start the transaction, execute the query, and then either commit or roll back based on the outcome.
- Consider aligning the transaction creation method with that used elsewhere (i.e., using
AppDataSource.getDataSource().createQueryRunner()if applicable) for consistency.
Below is an updated diff snippet that wraps the query within a transaction block:
export async function updateOrCreateProjectUserRecord({
projectId,
userId,
seasonId,
}: {
projectId: number;
userId: number;
seasonId?: number;
}): Promise<ProjectUserRecord> {
- const result = await ProjectUserRecord.query(query, [
+ // Use a transaction to ensure data consistency
+ const queryRunner = ProjectUserRecord.getRepository().manager.connection.createQueryRunner();
+ await queryRunner.connect();
+ await queryRunner.startTransaction();
+
+ try {
+ const result = await queryRunner.query(query, [
projectId,
userId,
seasonId || null,
[
DONATION_STATUS.VERIFIED,
DONATION_STATUS.PENDING,
DONATION_STATUS.SWAP_PENDING,
],
]);
+
+ await queryRunner.commitTransaction();
+ return result[0];
+ } catch (error) {
+ await queryRunner.rollbackTransaction();
+ throw error;
+ } finally {
+ await queryRunner.release();
+ }
}Please adjust the transaction initialization as needed to match the repository’s standard (for example, using AppDataSource.getDataSource().createQueryRunner() if that’s the consistent pattern elsewhere).
| @Entity() | ||
| @ObjectType() | ||
| export class Season extends BaseEntity { | ||
| @Field(_type => ID) | ||
| @PrimaryGeneratedColumn() | ||
| id: number; | ||
|
|
||
| @Field(_type => Int) | ||
| @Column() | ||
| seasonNumber: number; | ||
|
|
There was a problem hiding this comment.
🛠️ Refactor suggestion
Season entity basic structure is well-defined.
The Season entity is properly configured with appropriate decorators for both TypeORM and GraphQL. The primary key and seasonNumber fields are correctly defined.
Add a unique constraint to the seasonNumber field to prevent duplicate season numbers:
@Field(_type => Int)
@Column()
+ @Index({ unique: true })
seasonNumber: number;📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| @Entity() | |
| @ObjectType() | |
| export class Season extends BaseEntity { | |
| @Field(_type => ID) | |
| @PrimaryGeneratedColumn() | |
| id: number; | |
| @Field(_type => Int) | |
| @Column() | |
| seasonNumber: number; | |
| @Entity() | |
| @ObjectType() | |
| export class Season extends BaseEntity { | |
| @Field(_type => ID) | |
| @PrimaryGeneratedColumn() | |
| id: number; | |
| @Field(_type => Int) | |
| @Column() | |
| @Index({ unique: true }) | |
| seasonNumber: number; | |
| } |
1 participant
Summary by CodeRabbit
New Features
Refactor
Tests
Style