-
Notifications
You must be signed in to change notification settings - Fork 0
API Methods names clarification #105
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
- made the query methods of the users.service return null when the data is not found - error handling (throwing trpc errors is handled in the auth.service)
- the query methods return the data or null instead of throwing an error - renamed the findRangeDailyData method to simply findRange
- as for daily-data and users, renamed the methods of the languages.service
- renamed the projects methods with less verbose names and updated all affected files
- renamed the methods of the files.service to make them less verbose and updated all the affected files
📝 WalkthroughWalkthroughThis PR systematically refactors service method names across the API layer, replacing verbose domain-specific names with standardized generic CRUD operations (create, findOne, findAll, findRange, update). All corresponding call sites, type definitions, DTOs, and error handling are updated to align with the new API. Additionally, user retrieval in auth and users services is enhanced with improved null-checking and error handling. Changes
Estimated code review effort🎯 4 (Complex) | ⏱️ ~50 minutes Possibly related PRs
Poem
Pre-merge checks and finishing touches✅ Passed checks (3 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
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. Comment |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
🧹 Nitpick comments (2)
apps/api/src/projects/projects.dto.ts (1)
50-54: Consolidate or documentGetProjectFilesOnPeriodDtoduplication.Two definitions of
GetProjectFilesOnPeriodDtoexist with inconsistent validation rules:
- projects.dto.ts (lines 50–54, 82–84):
amount: z.number().optional()andlanguages: z.array(z.string()).optional()- files-stats.dto.ts (lines 54–61):
amount: z.number().int().nonnegative().optional()andlanguages: z.array(z.string().min(1)).optional()The files-stats version also wraps with
refineSchemaand explicitly definesname: z.string().min(1), while projects uses spread operators with different base schemas. If the differences are intentional, document the rationale; otherwise, consolidate to a single definition with appropriate strictness levels.apps/api/src/projects/projects-crud.service.ts (1)
24-123: LGTM! Method renames follow standard CRUD pattern.All methods renamed consistently (create, findOne, checkExists, findRange, update) with corresponding parameter updates. Internal logic unchanged.
Optional: Remove extra blank line
return timeSpentPerProject; } - async update(updateProjectDto: UpdateProjectDtoType) {
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (24)
apps/api/src/auth/auth.service.tsapps/api/src/coding-stats/coding-stats-dashboard.service.tsapps/api/src/coding-stats/coding-stats-extension.service.tsapps/api/src/coding-stats/utils/getDaysOfPeriodStatsGroupByMonths.tsapps/api/src/coding-stats/utils/getDaysOfPeriodStatsGroupByWeeks.tsapps/api/src/coding-stats/utils/getGeneralStatsOnPeriodGroupByMonths.tsapps/api/src/coding-stats/utils/getGeneralStatsOnPeriodGroupByWeeks.tsapps/api/src/coding-stats/utils/getPeriodLanguagesGroupByMonths.tsapps/api/src/coding-stats/utils/getPeriodLanguagesGroupByWeeks.tsapps/api/src/daily-data/daily-data.dto.tsapps/api/src/daily-data/daily-data.service.tsapps/api/src/files-stats/files-stats-dashboard.service.tsapps/api/src/files-stats/files-stats-extension.service.tsapps/api/src/files-stats/utils/getProjectLanguageGroupByMonths.tsapps/api/src/files-stats/utils/getProjectLanguagesGroupByWeeks.tsapps/api/src/files-stats/utils/getProjectPerDayOfPeriodGroupByMonths.tsapps/api/src/files-stats/utils/getProjectPerDayOfPeriodGroupByWeeks.tsapps/api/src/files/files.service.tsapps/api/src/languages/languages.service.tsapps/api/src/projects/projects-analytics.service.tsapps/api/src/projects/projects-crud.service.tsapps/api/src/projects/projects.dto.tsapps/api/src/projects/projects.service.tsapps/api/src/users/users.service.ts
🧰 Additional context used
🧬 Code graph analysis (10)
apps/api/src/projects/projects.dto.ts (1)
apps/api/src/files-stats/files-stats.dto.ts (2)
GetProjectFilesOnPeriodDto(55-62)GetProjectFilesOnPeriodDtoType(95-98)
apps/api/src/files-stats/utils/getProjectLanguagesGroupByWeeks.ts (1)
packages/common/src/types-schemas.ts (1)
PeriodResolution(81-81)
apps/api/src/projects/projects-analytics.service.ts (1)
apps/api/src/projects/projects.dto.ts (1)
GetProjectFilesOnPeriodDtoType(82-84)
apps/api/src/files/files.service.ts (1)
apps/api/src/files/files.dto.ts (4)
CreateFileDtoType(30-30)FindOneFileDtoType(34-34)FindAllFilesOnDayDtoType(36-36)UpdateFileDtoType(32-32)
apps/api/src/users/users.service.ts (1)
apps/api/src/users/users.dto.ts (1)
FindByIdDtoType(56-56)
apps/api/src/projects/projects-crud.service.ts (2)
apps/api/src/projects/projects.dto.ts (5)
CreateProjectDtoType(56-56)FindProjectDtoType(60-60)CheckProjectExistsDtoType(66-66)FindRangeProjectsDtoType(68-68)UpdateProjectDtoType(58-58)apps/api/src/files-stats/files-stats.dto.ts (2)
checkProjectExistsDto(34-36)CheckProjectExistsDtoType(71-72)
apps/api/src/daily-data/daily-data.service.ts (2)
apps/api/src/daily-data/daily-data.dto.ts (4)
CreateDailyDataDtoType(24-24)FindOneDailyDataDtoType(28-28)FindRangeDailyDataDtoType(30-30)UpdateDailyDataDtoType(26-26)apps/api/src/drizzle/schema/dailyData.ts (1)
dailyData(7-26)
apps/api/src/files-stats/files-stats-dashboard.service.ts (1)
apps/api/src/files-stats/files-stats.dto.ts (1)
GetPeriodProjectsDtoType(74-75)
apps/api/src/projects/projects.service.ts (2)
apps/api/src/projects/projects.dto.ts (12)
CreateProjectDtoType(56-56)FindProjectDtoType(60-60)CheckProjectExistsDtoType(66-66)FindRangeProjectsDtoType(68-68)UpdateProjectDtoType(58-58)GroupAndAggregateProjectByNameDtoType(70-72)FindProjectByNameOnRangeDtoType(62-64)getProjectLanguagesTimeOnPeriodDto(45-45)GetProjectLanguagesTimeOnPeriodDtoType(74-76)getProjectLanguagesTimePerDayOfPeriodDto(47-48)GetProjectLanguagesTimePerDayOfPeriodDtoType(78-80)GetProjectFilesOnPeriodDtoType(82-84)apps/api/src/files-stats/files-stats.dto.ts (3)
checkProjectExistsDto(34-36)CheckProjectExistsDtoType(71-72)GetProjectFilesOnPeriodDtoType(95-98)
apps/api/src/auth/auth.service.ts (1)
packages/common/src/constants.ts (1)
USER_NOT_FOUND_MESSAGE(3-3)
🔇 Additional comments (41)
apps/api/src/files-stats/utils/getProjectLanguageGroupByMonths.ts (1)
8-8: LGTM! Type reference correctly updated to match renamed service method.The type parameter update from
findProjectByNameOnRangetofindByNameOnRangealigns with the PR's objective of using shorter, less verbose method names. Removing the redundant "Project" prefix improves consistency since the method is already scoped toProjectsService.apps/api/src/auth/auth.service.ts (3)
64-71: Proper null handling for user lookup.The null check correctly handles the case where
findByEmailreturnsnull, throwing an appropriateNOT_FOUNDerror. This aligns with the PR objective to move authentication error handling to the auth service.
178-183: Proper null handling added.The null check correctly handles the case where
findByEmailreturnsnull, consistent with the refactoring objective.
214-235: Excellent error handling and proper use of renamed method.The
getUsermethod now has improved layered error handling:
- Early check for missing
ctx.user(lines 215-220)- Use of the renamed
findByIdmethod (line 223)- Proper null handling with appropriate error (lines 227-232)
This implementation correctly adapts to the users service changes and follows good defensive programming practices.
apps/api/src/users/users.service.ts (3)
189-191: Null-return behavior is consistent with other find methods.This change aligns with the pattern established by
findByEmailandfindById, which both returnnullwhen a user is not found. The single caller inauth.service.ts(line 329) properly handles the null case with anif (existingUser)check.
134-152: Null-return behavior is properly handled by the single caller.The rename to
findByIdimproves clarity, and returningnullwhen no user is found aligns with the PR objectives. The auth service correctly handles thenullreturn value at line 223 by checkingif (!user)and throwing aTRPCErrorwithNOT_FOUNDcode, moving error handling to the auth service layer as intended.
169-171: Null-return behavior is consistent with the refactoring. This change aligns with the new pattern established infindById. All callers inauth.service.tsproperly handle thenullreturn with appropriate error throwing.apps/api/src/daily-data/daily-data.service.ts (4)
23-40: LGTM! Clean rename to generic CRUD naming.The method rename from
createDailyDatatocreatefollows standard CRUD conventions and improves API consistency. The logic and return behavior remain unchanged.
42-53: LGTM! Consistent null-handling behavior.The rename from
findOneDailyDatatofindOneis clean, and the method correctly returnsnullwhen no data is found (line 50), aligning with the PR's stated objective of having query methods return null instead of throwing errors.
55-90: LGTM! Good refactor with minor improvement.The rename from
findRangeDailyDatatofindRangeis consistent with the broader refactoring. Storing the mapped result in a localrangevariable (lines 78-89) before returning is a minor improvement that aids debugging.
92-110: LGTM! Update method renamed consistently.The rename from
updateDailyDatatoupdatecompletes the consistent CRUD naming pattern across all service methods.apps/api/src/daily-data/daily-data.dto.ts (1)
11-14: LGTM! Improved naming convention.Renaming the schema from
findOneDailyDataDtotoFindOneDailyDataDto(PascalCase) is more conventional for exported Zod schemas and aligns with TypeScript naming best practices. The corresponding type reference is correctly updated.Also applies to: 28-28
apps/api/src/coding-stats/utils/getDaysOfPeriodStatsGroupByWeeks.ts (1)
9-11: LGTM! Type reference correctly updated.The parameter type is properly updated to reference the renamed
DailyDataService["findRange"]method, maintaining type safety across the refactoring.apps/api/src/coding-stats/utils/getGeneralStatsOnPeriodGroupByMonths.ts (1)
12-19: LGTM! Parameter type correctly updated.The
dailyDataForPeriodparameter type is properly updated to referenceDailyDataService["findRange"], consistent with the service method renaming.apps/api/src/coding-stats/utils/getDaysOfPeriodStatsGroupByMonths.ts (1)
8-10: LGTM! Type reference updated consistently.The parameter type correctly references the renamed
DailyDataService["findRange"]method, maintaining consistency with the refactoring pattern.apps/api/src/files-stats/utils/getProjectPerDayOfPeriodGroupByMonths.ts (1)
8-10: LGTM! Type reference updated for ProjectsService refactoring.The parameter type is correctly updated to reference
ProjectsService["findByNameOnRange"], aligning with the same refactoring pattern applied to ProjectsService methods.apps/api/src/coding-stats/utils/getGeneralStatsOnPeriodGroupByWeeks.ts (1)
13-21: LGTM! Parameter type correctly updated.The
dailyDataForPeriodparameter type properly referencesDailyDataService["findRange"], maintaining type safety across the refactoring.apps/api/src/files-stats/utils/getProjectPerDayOfPeriodGroupByWeeks.ts (1)
9-12: LGTM! Type reference updated consistently.The parameter type correctly references
ProjectsService["findByNameOnRange"], completing the consistent type updates across all files-stats utilities.apps/api/src/coding-stats/utils/getPeriodLanguagesGroupByWeeks.ts (1)
9-11: Method and type signature updates look correct.The type signature and method call updates align with the PR's goal of standardizing service method names to shorter CRUD-style conventions (
findRange,findAll).Also applies to: 33-35
apps/api/src/coding-stats/coding-stats-extension.service.ts (3)
23-36: Service method renames are consistent.The
getDailyStatsForExtensionmethod correctly uses the renamed service methods (findOne,findAll) while maintaining the same null-check logic.
51-84: Upsert flow for daily data updated correctly.The create/update logic for daily data properly uses the renamed methods (
findOne,create,update) while preserving the conditional upsert behavior.
86-117: Language data upsert updated consistently.The language processing loop correctly uses renamed methods (
findOne,create,update) with unchanged business logic.apps/api/src/files-stats/utils/getProjectLanguagesGroupByWeeks.ts (1)
8-12: Type signature update aligned with service method rename.The parameter type correctly references the renamed
findByNameOnRangemethod.apps/api/src/files-stats/files-stats-dashboard.service.ts (3)
25-27: ProjectsService method renames applied correctly.The
checkExistsandfindRangemethod calls align with the new naming convention while maintaining proper parameter passing.Also applies to: 29-36
59-70: Project query methods updated consistently.Both
groupAndAggregateByNameandfindByNameOnRangecalls are properly updated with correct parameter object wrapping.Also applies to: 72-111
113-159: Remaining service method renames are consistent.All language time aggregation methods (
getLanguagesTimeOnPeriod,getLanguagesTimePerDayOfPeriod) and file retrieval (getFilesOnPeriod) correctly use the renamed methods.Also applies to: 161-210, 212-220
apps/api/src/coding-stats/utils/getPeriodLanguagesGroupByMonths.ts (1)
8-11: Type and method updates consistent with the weeks variant.The parameter type and
findAllcall mirror the changes ingetPeriodLanguagesGroupByWeeks.ts, maintaining consistency across the codebase.Also applies to: 30-37
apps/api/src/projects/projects.dto.ts (1)
37-41: DTO rename fromFindAllRangeProjectsDtotoFindRangeProjectsDtois consistent.Removing "All" from the name aligns with the standardized naming convention across the codebase.
Also applies to: 68-68
apps/api/src/files-stats/files-stats-extension.service.ts (3)
21-40:getDailyFilesStatsForExtensionmethod updated correctly.Service calls to
dailyDataService.findOneandfilesService.findAllOnDayalign with the new naming convention.
42-107: Project upsert logic uses renamed methods consistently.The
upsertmethod correctly usesdailyDataService.findOne,filesService.findAllOnDay,projectsService.findOne,projectsService.create, andprojectsService.updatewith unchanged business logic.
109-171: File processing uses renamed service methods correctly.The file creation/update flow properly uses
languagesService.findOne,filesService.findOne,filesService.create, andfilesService.updatewith preserved conditional logic.apps/api/src/files/files.service.ts (4)
24-43:createmethod (formerlycreateFile) renamed correctly.The implementation is unchanged; only the method name has been shortened for consistency.
45-67:findOnemethod (formerlyfindOneFile) renamed correctly.Method signature and null-returning behavior preserved.
69-92:findAllOnDaymethod (formerlyfindAllFilesOnDay) renamed correctly.The join query and data transformation logic remain unchanged.
94-117:updatemethod (formerlyupdateFile) renamed correctly.Update logic and return structure preserved.
apps/api/src/languages/languages.service.ts (1)
21-106: LGTM! Clean refactoring to standard CRUD names.The method renames follow a consistent pattern (create, findAll, findOne, update) and improve API clarity. Internal logic remains unchanged.
apps/api/src/coding-stats/coding-stats-dashboard.service.ts (1)
39-282: LGTM! Call sites correctly updated.All method calls to
dailyDataServiceandlanguagesServicehave been updated to use the new concise names. Arguments and logic remain unchanged.apps/api/src/projects/projects-crud.service.ts (1)
13-13: LGTM! DTO type import updated correctly.The import change from
FindAllRangeProjectsDtoTypetoFindRangeProjectsDtoTypealigns with the shorter naming convention.apps/api/src/projects/projects-analytics.service.ts (2)
16-16: LGTM! DTO type import updated correctly.The import change from
GetAllProjectFilesOnPeriodDtoTypetoGetProjectFilesOnPeriodDtoTypealigns with the concise naming convention.
29-237: LGTM! Analytics methods renamed consistently.Method names shortened appropriately while maintaining clarity. The "get" prefix for analytics methods (getLanguagesTimeOnPeriod, getFilesOnPeriod) effectively distinguishes aggregation operations from simple CRUD retrieval.
apps/api/src/projects/projects.service.ts (1)
8-9: LGTM! DTO type imports updated correctly.Both import changes align with the new concise naming convention.
Commits
fix: users.service
fix: daily data
fix: langages
feat: projects
chore: files
Summary by CodeRabbit
Release Notes
Bug Fixes
Refactor
✏️ Tip: You can customize this high-level summary in your review settings.