Add docstrings throughout codebase #11
Conversation
Backend: - Document service interfaces (IActionService, IContextService, IInboxService, IProcessingService, IProjectService) with method summaries and param/return docs - Document DTOs (ActionFilterDto, ProcessingResultDto, AttachmentDto, CreateAttachmentDto) with class and property-level docs Frontend: - Document all enums in enums.ts with enum and member descriptions - Add JSDoc to Zod schemas in schemas.ts (23 schemas) - Document exported functions in recurrenceUtils.ts - Document DatabaseStats and SyncStatusInfo interfaces in useAdmin.ts Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
📝 WalkthroughWalkthroughThis PR introduces comprehensive type definitions, enums, utility functions, and service interface extensions to support task dependencies, recurrence patterns, and inbox item processing workflows. Changes span both frontend TypeScript definitions and backend C# service interfaces, with extensive documentation additions. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 inconclusive)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@src/OpenGTD/Features/Attachments/IAttachmentService.cs`:
- Around line 119-134: The DTO currently allowing multiple or zero parent IDs
must enforce exactly one parent ID; implement IValidatableObject.Validate() on
the attachment DTO (the class containing InboxItemId, ActionItemId, ProjectId,
ReferenceItemId, SomedayMaybeItemId) to count non-null IDs and return a
validation error unless exactly one is set, then ensure calls that create/update
attachments use model validation by invoking/propagating that validation in
CreateAsync, CreateFromSyncAsync, and UpdateFromSyncAsync paths so the DTO-level
rule is applied consistently across the upload (/upload) and PowerSync
endpoints.
🧹 Nitpick comments (1)
src/OpenGTD/Features/Attachments/IAttachmentService.cs (1)
80-82: StandardizeTypeacross attachment DTOs to avoid mapping drift.
AttachmentDto.Typeis astringwhileCreateAttachmentDto.TypeusesAttachmentTypeand sync DTOs useint. This increases conversion/mapping risk and makes contracts harder to reason about. Consider unifying onAttachmentType(or a shared wire format) across DTOs and handling any required serialization explicitly.♻️ Possible alignment (if API contract allows enum output)
- /// <summary>Attachment type as a string (File, Link, Image, etc.).</summary> - public string Type { get; set; } = string.Empty; + /// <summary>Attachment type.</summary> + public AttachmentType Type { get; set; }
| // One of these must be set | ||
|
|
||
| /// <summary>Parent inbox item ID (if attaching to an inbox item).</summary> | ||
| public Guid? InboxItemId { get; set; } | ||
|
|
||
| /// <summary>Parent action item ID (if attaching to an action).</summary> | ||
| public Guid? ActionItemId { get; set; } | ||
|
|
||
| /// <summary>Parent project ID (if attaching to a project).</summary> | ||
| public Guid? ProjectId { get; set; } | ||
|
|
||
| /// <summary>Parent reference item ID (if attaching to reference material).</summary> | ||
| public Guid? ReferenceItemId { get; set; } | ||
|
|
||
| /// <summary>Parent someday/maybe item ID (if attaching to a someday/maybe item).</summary> | ||
| public Guid? SomedayMaybeItemId { get; set; } |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# First, find the CreateAttachmentDto class definition
find . -type f -name "*.cs" | xargs rg -l "CreateAttachmentDto" | head -20Repository: trevwilson/GTDApp
Length of output: 349
🏁 Script executed:
# Also search for the file mentioned in the review
find . -type f -name "IAttachmentService.cs"Repository: trevwilson/GTDApp
Length of output: 117
🏁 Script executed:
# Look for validation on parent IDs in services and controllers
rg -n --type=cs "InboxItemId|ActionItemId|ProjectId|ReferenceItemId|SomedayMaybeItemId" -A 2 -B 2 | head -100Repository: trevwilson/GTDApp
Length of output: 9309
🏁 Script executed:
# Read the IAttachmentService.cs file to find the DTO definition
cat -n ./src/OpenGTD/Features/Attachments/IAttachmentService.csRepository: trevwilson/GTDApp
Length of output: 7148
🏁 Script executed:
# Read AttachmentService.cs to check for validation
wc -l ./src/OpenGTD/Features/Attachments/AttachmentService.csRepository: trevwilson/GTDApp
Length of output: 120
🏁 Script executed:
# Read AttachmentEndpoints.cs to check for validation
wc -l ./src/OpenGTD/Features/Attachments/AttachmentEndpoints.csRepository: trevwilson/GTDApp
Length of output: 122
🏁 Script executed:
# Read AttachmentService.cs to check for validation in CreateAsync
cat -n ./src/OpenGTD/Features/Attachments/AttachmentService.csRepository: trevwilson/GTDApp
Length of output: 11540
🏁 Script executed:
# Read AttachmentEndpoints.cs to check for validation
cat -n ./src/OpenGTD/Features/Attachments/AttachmentEndpoints.csRepository: trevwilson/GTDApp
Length of output: 9163
Add validation to enforce exactly one parent ID.
The DTO includes a comment "One of these must be set" but provides no enforcement. The /upload endpoint checks for at least one parent ID (lines 74-78), but:
- It does not prevent multiple parent IDs being set simultaneously
- The PowerSync sync endpoints (
POST /andPUT /{id:guid}) have no parent ID validation at all - The service layer performs no validation
This allows orphan attachments (zero parent IDs) and multi-parent links (multiple IDs). Add DTO-level validation using IValidatableObject.Validate() to ensure exactly one parent ID is set, then apply it across all creation paths (CreateAsync, CreateFromSyncAsync, and UpdateFromSyncAsync).
🤖 Prompt for AI Agents
In `@src/OpenGTD/Features/Attachments/IAttachmentService.cs` around lines 119 -
134, The DTO currently allowing multiple or zero parent IDs must enforce exactly
one parent ID; implement IValidatableObject.Validate() on the attachment DTO
(the class containing InboxItemId, ActionItemId, ProjectId, ReferenceItemId,
SomedayMaybeItemId) to count non-null IDs and return a validation error unless
exactly one is set, then ensure calls that create/update attachments use model
validation by invoking/propagating that validation in CreateAsync,
CreateFromSyncAsync, and UpdateFromSyncAsync paths so the DTO-level rule is
applied consistently across the upload (/upload) and PowerSync endpoints.
Summary
Test plan
dotnet buildpasses with 0 warnings, 0 errorspnpm exec tsc --noEmitpasses🤖 Generated with Claude Code
Summary by CodeRabbit
Release Notes
New Features
✏️ Tip: You can customize this high-level summary in your review settings.