Fix course identifiers #19
Merged
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
WalkthroughWalkthroughThe updates focus on enhancing the database structure and codebase for course management. The primary key for the Changes
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 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 as PR comments)
Additionally, you can add CodeRabbit Configration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Review Status
Actionable comments generated: 0
Configuration used: CodeRabbit UI
Files selected for processing (7)
- db/migrations/1657379568_create_courses_table.up.sql (2 hunks)
- internal/course/database/course_queries.go (1 hunks)
- internal/course/database/course_repository.go (3 hunks)
- internal/course/database/course_repository_test.go (9 hunks)
- internal/course/domain/course.go (1 hunks)
- internal/course/domain/course_repository.go (1 hunks)
- internal/course/domain/course_service.go (2 hunks)
Additional comments: 17
internal/course/domain/course_repository.go (3)
- 6-6: Renaming the parameter to
courseUUIDin theCoursemethod aligns with the PR's objective of using UUIDs for course identification. This change enhances clarity and consistency across the codebase.- 9-9: The introduction of
UpdateCourseByUUIDmethod in place ofUpdateCourseByIDandUpdateCourseByCodeis a positive change. It simplifies the update mechanism and enforces updates through a more consistent identifier, which is in line with the PR objectives.- 10-10: Changing the parameter to
courseUUIDin theDeleteCoursemethod is consistent with the shift towards using UUIDs for course identification. This change ensures consistency and clarity in the method signature.internal/course/domain/course.go (1)
- 11-20: Removing the
IDfield from theCoursestruct and adjusting the field order to start withUUIDis a significant change that aligns with the PR's objective of using UUIDs for course identification. This modification simplifies the identification mechanism and enhances data integrity.db/migrations/1657379568_create_courses_table.up.sql (3)
- 5-5: Changing the primary key definition to use an
integertype with anIDENTITYproperty instead ofbigserialis a good practice for automating the generation of unique identifiers. This change aligns with modern database design practices and enhances data integrity.- 7-7: Modifying the
codecolumn to no longer beUNIQUEdirectly in the column definition and instead using a unique index oncodeanddeleted_atcolumns supports the implementation of soft deletion patterns. This approach allows for the reuse of course codes while maintaining uniqueness among active records.- 19-20: Adding a comment to the
deleted_atcolumn to explain its purpose for enabling soft deletes is a good practice for code readability and maintainability. It helps future developers understand the design decisions made in the database schema.internal/course/domain/course_service.go (3)
- 10-11: Renaming the parameter to
courseUUIDin theCoursefunction is consistent with the overall PR objectives and enhances clarity in the method signature. This change ensures consistency across the codebase.- 37-37: Refactoring the
UpdateCoursefunction to directly useUpdateCourseByUUIDsimplifies the update mechanism and aligns with the shift towards using UUIDs for course identification. This change improves the maintainability of the code.- 43-44: Changing the parameter to
courseUUIDin theDeleteCoursefunction aligns with the PR's objective and enhances consistency in method signatures across the codebase.internal/course/database/course_queries.go (2)
- 8-17: Adding a constant for returning columns and updating query constants for clarity and consistency is a good practice. It enhances the readability and maintainability of the SQL queries within the codebase.
- 22-36: Refactoring the SQL queries to include returning columns specification and improving the formatting of the queries significantly enhances readability and maintainability. Specifying returning columns is a best practice that ensures only necessary data is retrieved, which can improve performance.
internal/course/database/course_repository.go (3)
- 39-47: Changing the parameter name from
idtocourseUUIDin theCoursemethod is consistent with the PR objectives and enhances clarity in the method signature. This change ensures consistency across the codebase.- 89-91: Renaming
UpdateCourseByIDtoUpdateCourseByUUIDand updating the SQL statement accordingly aligns with the shift towards using UUIDs for course identification. This change simplifies the update mechanism and enhances data integrity.- 114-121: Changing the parameter name from
idtocourseUUIDin theDeleteCoursemethod ensures consistency with the use of UUIDs across the codebase. This change enhances clarity and maintainability of the code.internal/course/database/course_repository_test.go (2)
- 35-38: Adjusting the test data to reflect the use of UUIDs instead of integer IDs is necessary to align with the changes made in the codebase. This ensures that the tests accurately represent the current state of the code and its functionality.
- 208-212: Renaming functions related to updating courses by ID to use UUID instead and adjusting the test data accordingly reflects the shift towards using UUIDs for course identification. This change ensures that the tests remain relevant and accurate.
marianysilva
force-pushed
the
fix-course-identifiers
branch
from
de73f4f to
5109117
Compare
There was a problem hiding this comment.
Review Status
Actionable comments generated: 0
Configuration used: CodeRabbit UI
Files selected for processing (7)
- db/migrations/1657379568_create_courses_table.up.sql (2 hunks)
- internal/course/database/course_queries.go (1 hunks)
- internal/course/database/course_repository.go (3 hunks)
- internal/course/database/course_repository_test.go (9 hunks)
- internal/course/domain/course.go (1 hunks)
- internal/course/domain/course_repository.go (1 hunks)
- internal/course/domain/course_service.go (2 hunks)
Files skipped from review as they are similar to previous changes (7)
- db/migrations/1657379568_create_courses_table.up.sql
- internal/course/database/course_queries.go
- internal/course/database/course_repository.go
- internal/course/database/course_repository_test.go
- internal/course/domain/course.go
- internal/course/domain/course_repository.go
- internal/course/domain/course_service.go
marianysilva
force-pushed
the
fix-course-identifiers
branch
from
5109117 to
54362b2
Compare
Codecov ReportAttention: Patch coverage is
❗ Your organization needs to install the Codecov GitHub app to enable full functionality. Additional details and impacted files@@ Coverage Diff @@
## main #19 +/- ##
==========================================
+ Coverage 43.58% 46.46% +2.88%
==========================================
Files 9 9
Lines 413 396 -17
==========================================
+ Hits 180 184 +4
+ Misses 211 190 -21
Partials 22 22 ☔ View full report in Codecov by Sentry. |
rluders
requested changes
Mar 7, 2024
There was a problem hiding this comment.
Review Status
Actionable comments generated: 0
Configuration used: CodeRabbit UI
Files selected for processing (7)
- db/migrations/1657379568_create_courses_table.up.sql (2 hunks)
- internal/course/database/course_queries.go (1 hunks)
- internal/course/database/course_repository.go (3 hunks)
- internal/course/database/course_repository_test.go (9 hunks)
- internal/course/domain/course.go (1 hunks)
- internal/course/domain/course_repository.go (1 hunks)
- internal/course/domain/course_service.go (2 hunks)
Files skipped from review as they are similar to previous changes (7)
- db/migrations/1657379568_create_courses_table.up.sql
- internal/course/database/course_queries.go
- internal/course/database/course_repository.go
- internal/course/database/course_repository_test.go
- internal/course/domain/course.go
- internal/course/domain/course_repository.go
- internal/course/domain/course_service.go
marianysilva
force-pushed
the
fix-course-identifiers
branch
from
6244cbf to
d2b2ad1
Compare
marianysilva
force-pushed
the
fix-course-identifiers
branch
from
d2b2ad1 to
8fbcd7a
Compare
There was a problem hiding this comment.
Review Status
Actionable comments generated: 0
Configuration used: CodeRabbit UI
Files selected for processing (7)
- db/migrations/1657379568_create_courses_table.up.sql (2 hunks)
- internal/course/database/course_queries.go (1 hunks)
- internal/course/database/course_repository.go (3 hunks)
- internal/course/database/course_repository_test.go (9 hunks)
- internal/course/domain/course.go (1 hunks)
- internal/course/domain/course_repository.go (1 hunks)
- internal/course/domain/course_service.go (2 hunks)
Files skipped from review as they are similar to previous changes (7)
- db/migrations/1657379568_create_courses_table.up.sql
- internal/course/database/course_queries.go
- internal/course/database/course_repository.go
- internal/course/database/course_repository_test.go
- internal/course/domain/course.go
- internal/course/domain/course_repository.go
- internal/course/domain/course_service.go
marianysilva
commented
Mar 8, 2024
arianerocha
requested changes
Mar 8, 2024
arianerocha
requested changes
Mar 8, 2024
arianerocha
reviewed
Mar 8, 2024
arianerocha
reviewed
Mar 8, 2024
There was a problem hiding this comment.
Review Status
Actionable comments generated: 0
Configuration used: CodeRabbit UI
Files selected for processing (1)
- db/migrations/1657379568_create_courses_table.up.sql (2 hunks)
Files skipped from review as they are similar to previous changes (1)
- db/migrations/1657379568_create_courses_table.up.sql
|
@arianerocha since you where involved in the discussions, I'll wait for your approval before squash and merge. :) |
arianerocha
approved these changes
Mar 13, 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.
Description
GENERATED ALWAYS AS IDENTITYidparams to `courseUUID``Type of change
How Has This Been Tested?
go mod vendorgo get ./...make testmake migration-upmake migration-downSummary by CodeRabbit
coursestable structure.