feat(dar): implement Daily Accomplishment Report backend#55
Conversation
|
Warning Review limit reached
Next review available in: 32 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (12)
📝 WalkthroughWalkthroughAdds a complete daily reports feature, including EF Core entities and migrations, request/response DTOs, validation, service-layer CRUD and query operations, supervisor remarks, authentication and permissions, and REST endpoints. ChangesDaily reports
Sequence Diagram(s)sequenceDiagram
participant Client
participant DailyReportsController
participant DailyReportsService
participant AppDbContext
Client->>DailyReportsController: POST api/daily-reports
DailyReportsController->>DailyReportsService: Create request
DailyReportsService->>AppDbContext: Save report and tasks
AppDbContext-->>DailyReportsService: Persisted entities
DailyReportsService-->>DailyReportsController: DailyReportDto
DailyReportsController-->>Client: 201 Created
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 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 |
Zekiroh
added
documentation
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 8
🧹 Nitpick comments (1)
apps/api/HRIS.Api/Data/Configurations/DailyReportTaskConfiguration.cs (1)
23-24: 🗄️ Data Integrity & Integration | 🔵 Trivial | ⚡ Quick winAdd a unique constraint for
TaskNumberwithin aDailyReport.While the service layer validates that task numbers are unique for a report, adding a composite unique index ensures data integrity at the database level against concurrent inserts or manual data entry.
💡 Proposed fix
builder.Property(x => x.BlockedByRemarks).HasMaxLength(500); + + builder.HasIndex(x => new { x.DailyReportId, x.TaskNumber }).IsUnique(); } }(Note: You will need to generate a migration to apply this constraint to the database).
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@apps/api/HRIS.Api/Data/Configurations/DailyReportTaskConfiguration.cs` around lines 23 - 24, Update DailyReportTaskConfiguration to add a composite unique index for DailyReport and TaskNumber, ensuring each task number is unique within its report. Then generate the corresponding database migration to apply the constraint.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@apps/api/HRIS.Api/Features/Attendance/Controllers/DailyReportsController.cs`:
- Around line 18-22: The Create method’s CreatedAtAction response points to
GetById, which some report creators cannot access because of the ATTENDANCE/View
restriction. Update the authorization for GetById to allow report owners to read
their own reports, or change Create to reference an existing owner-accessible
route while preserving access restrictions for other users.
In `@apps/api/HRIS.Api/Features/Attendance/DTOs/CreateDailyReportRequest.cs`:
- Line 13: Update CreateDailyReportRequest.BreakDurationMinutes to enforce a
valid 0–1440 minute range using the appropriate validation attribute, while
preserving its default value of 60.
In `@apps/api/HRIS.Api/Features/Attendance/DTOs/SupervisorRemarksRequest.cs`:
- Around line 10-11: Remove ReviewedBy and DateReviewed from
SupervisorRemarksRequest so audit metadata cannot be caller-controlled. Update
IDailyReportsService and its DailyReportsService implementation to accept
ClaimsPrincipal, derive the authenticated reviewer and current server date in
the review persistence flow, and update DailyReportsController to pass User;
apply these changes in
apps/api/HRIS.Api/Features/Attendance/DTOs/SupervisorRemarksRequest.cs lines
10-11, apps/api/HRIS.Api/Features/Attendance/Services/IDailyReportsService.cs
line 14, apps/api/HRIS.Api/Features/Attendance/Services/DailyReportsService.cs
lines 151-175, and
apps/api/HRIS.Api/Features/Attendance/Controllers/DailyReportsController.cs
lines 32-36.
In `@apps/api/HRIS.Api/Features/Attendance/DTOs/UpdateDailyReportRequest.cs`:
- Line 22: Support omitted-versus-explicit-null PATCH semantics: update
ExpectedTimeIn in
apps/api/HRIS.Api/Features/Attendance/DTOs/UpdateDailyReportRequest.cs:22-22 and
ReviewDate in
apps/api/HRIS.Api/Features/Attendance/DTOs/SupervisorRemarksRequest.cs:8-8 to
use an optional-field wrapper or clear flag, then update
DailyReportsService.cs:138-144 and :159-169 to apply assignments when fields are
explicitly cleared while preserving existing values when omitted.
In `@apps/api/HRIS.Api/Features/Attendance/Services/DailyReportsService.cs`:
- Around line 218-222: Update the query ordering in the reports retrieval flow
to add ThenByDescending(r => r.Id) after OrderByDescending(r => r.ReportDate)
and before Skip, ensuring stable pagination when multiple reports share the same
ReportDate.
- Around line 50-54: Update the daily-report creation flow in
DailyReportsService to catch DbUpdateException caused by the
EmployeeId/ReportDate unique constraint and translate it into ApiException with
StatusCodes.Status409Conflict, while preserving propagation of unrelated
database errors and the existing pre-insert duplicate check.
- Around line 296-313: Order report.Tasks by TaskNumber before projecting to
DailyReportTaskDto, preserving all existing field mappings and materialization
behavior. Update only the Tasks projection in the surrounding report-mapping
method.
In `@apps/api/HRIS.Api/Models/DailyReport.cs`:
- Around line 16-17: Update DailyReport.SubmittedToId to nullable long and
update incoming request DTOs such as CreateDailyReportRequest to use the same
property name and type. In DailyReportConfiguration, explicitly map SubmittedTo
to SubmittedToId with HasForeignKey alongside the Employee relationship.
Regenerate the migration and model snapshots:
apps/api/HRIS.Api/Migrations/20260601170756_AddDailyReports.cs:32-33,
apps/api/HRIS.Api/Migrations/20260601170756_AddDailyReports.Designer.cs:207-212,
and apps/api/HRIS.Api/Migrations/AppDbContextModelSnapshot.cs:541-546, removing
the duplicate integer/shadow foreign-key column.
---
Nitpick comments:
In `@apps/api/HRIS.Api/Data/Configurations/DailyReportTaskConfiguration.cs`:
- Around line 23-24: Update DailyReportTaskConfiguration to add a composite
unique index for DailyReport and TaskNumber, ensuring each task number is unique
within its report. Then generate the corresponding database migration to apply
the constraint.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 1ca3692e-9a9d-4fe9-97c2-0ec52adba12f
📒 Files selected for processing (19)
apps/api/HRIS.Api/Data/AppDbContext.csapps/api/HRIS.Api/Data/Configurations/DailyReportConfiguration.csapps/api/HRIS.Api/Data/Configurations/DailyReportTaskConfiguration.csapps/api/HRIS.Api/Features/Attendance/Controllers/DailyReportsController.csapps/api/HRIS.Api/Features/Attendance/DTOs/CreateDailyReportRequest.csapps/api/HRIS.Api/Features/Attendance/DTOs/DailyReportDto.csapps/api/HRIS.Api/Features/Attendance/DTOs/GetDailyReportsQuery.csapps/api/HRIS.Api/Features/Attendance/DTOs/SupervisorRemarksRequest.csapps/api/HRIS.Api/Features/Attendance/DTOs/UpdateDailyReportRequest.csapps/api/HRIS.Api/Features/Attendance/Services/DailyReportsService.csapps/api/HRIS.Api/Features/Attendance/Services/IDailyReportsService.csapps/api/HRIS.Api/Migrations/20260601170756_AddDailyReports.Designer.csapps/api/HRIS.Api/Migrations/20260601170756_AddDailyReports.csapps/api/HRIS.Api/Migrations/20260712212707_AddDailyReportSections4to8.Designer.csapps/api/HRIS.Api/Migrations/20260712212707_AddDailyReportSections4to8.csapps/api/HRIS.Api/Migrations/AppDbContextModelSnapshot.csapps/api/HRIS.Api/Models/DailyReport.csapps/api/HRIS.Api/Models/DailyReportTask.csapps/api/HRIS.Api/Program.cs
| [HttpPost] | ||
| public async Task<IActionResult> Create([FromBody] CreateDailyReportRequest request) | ||
| { | ||
| var result = await _service.CreateAsync(User, request); | ||
| return CreatedAtAction(nameof(GetById), new { id = result.Id }, result); |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
rg -n -C3 'PermissionAuthorize|ATTENDANCE.*View|CreatedAtAction' apps/api/HRIS.ApiRepository: Zekiroh/hris-system
Length of output: 29166
🏁 Script executed:
ast-grep outline apps/api/HRIS.Api/Features/Attendance/Controllers/DailyReportsController.cs --view expandedRepository: Zekiroh/hris-system
Length of output: 546
🏁 Script executed:
fd -a 'DailyReports*' apps/api/HRIS.Api/Features/AttendanceRepository: Zekiroh/hris-system
Length of output: 777
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '\n## DailyReportsController\n'
sed -n '1,120p' apps/api/HRIS.Api/Features/Attendance/Controllers/DailyReportsController.cs | cat -n
printf '\n## DailyReportsService\n'
sed -n '1,240p' apps/api/HRIS.Api/Features/Attendance/Services/DailyReportsService.cs | cat -n
printf '\n## PermissionAuthorizeAttribute\n'
sed -n '1,220p' apps/api/HRIS.Api/Features/IAM/Controllers/PermissionAuthorizeAttribute.cs | cat -nRepository: Zekiroh/hris-system
Length of output: 16419
Make the returned Location reachable by the creator. In apps/api/HRIS.Api/Features/Attendance/Controllers/DailyReportsController.cs:18-22, 47-52, Create returns CreatedAtAction(nameof(GetById)), but GetById is gated by ATTENDANCE/View, so some creators get a 201 with a Location they can’t follow. Let report owners read GetById, or point the response at an owner-accessible route instead.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@apps/api/HRIS.Api/Features/Attendance/Controllers/DailyReportsController.cs`
around lines 18 - 22, The Create method’s CreatedAtAction response points to
GetById, which some report creators cannot access because of the ATTENDANCE/View
restriction. Update the authorization for GetById to allow report owners to read
their own reports, or change Create to reference an existing owner-accessible
route while preserving access restrictions for other users.
3 participants
Summary by CodeRabbit