Conversation
There was a problem hiding this comment.
Pull request overview
This PR adds a comprehensive Activity API endpoint that allows querying activity data across all models (todos, projects, milestones, events, thoughts, resources, journal entries) for any time period. The endpoint is designed specifically for AI agents to summarize productivity.
Changes:
- Adds ActivityService with support for named periods (this_week, last_month, etc.) and custom date ranges
- Adds new API endpoint GET /api/v1/activity with filtering, period selection, and summary statistics
- Updates API documentation with detailed examples and usage patterns, including URL updates from placeholder domains to today.travserve.net
Reviewed changes
Copilot reviewed 12 out of 12 changed files in this pull request and generated 10 comments.
Show a summary per file
| File | Description |
|---|---|
| app/services/activity_service.rb | New service implementing activity aggregation logic with period validation, date range handling, and data formatting across all entity types |
| app/controllers/api/v1/activity_controller.rb | New controller exposing the activity endpoint with proper authentication (read scope required) and error handling |
| app/models/todo.rb | Adds created_in_range and completed_in_range scopes for date-based filtering |
| app/models/project.rb | Adds created_in_range, completed_in_range, and updated_in_range scopes |
| app/models/milestone.rb | Adds created_in_range and completed_in_range scopes |
| app/models/thought.rb | Adds created_in_range scope |
| app/models/resource.rb | Adds created_in_range scope |
| app/models/journal_entry.rb | Adds created_in_range scope |
| test/services/activity_service_test.rb | Comprehensive service tests covering period validation, date range validation, data filtering, cross-references, and edge cases |
| test/controllers/api/v1/activity_controller_test.rb | Controller tests verifying authentication, authorization, parameter handling, response structure, and data filtering |
| docs/API.md | Extensive documentation of the new endpoint with examples, parameter descriptions, and response format; also updates example URLs throughout |
| config/routes.rb | Adds the new activity route to the v1 API namespace |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
app/services/activity_service.rb
Outdated
| raise InvalidDateRangeError, "start_date is required for custom date range" if custom_start.blank? | ||
| raise InvalidDateRangeError, "end_date is required for custom date range" if custom_end.blank? | ||
|
|
||
| if (date_range.end.to_date - date_range.begin.to_date).to_i > MAX_RANGE_DAYS |
There was a problem hiding this comment.
The validation doesn't check if end_date is before start_date. When end_date is before start_date, the date range calculation will succeed but produce an invalid range (where range.end < range.begin). This could lead to unexpected behavior when querying the database, as the WHERE clause would search for records where created_at is in a backwards range. Consider adding explicit validation to ensure start_date is before or equal to end_date.
| if (date_range.end.to_date - date_range.begin.to_date).to_i > MAX_RANGE_DAYS | |
| start_time = parse_date(custom_start) | |
| end_time = parse_date(custom_end) | |
| if end_time < start_time | |
| raise InvalidDateRangeError, "end_date cannot be before start_date" | |
| end | |
| if (end_time.to_date - start_time.to_date).to_i > MAX_RANGE_DAYS |
app/models/project.rb
Outdated
|
|
||
| scope :created_in_range, ->(range) { where(created_at: range) } | ||
| scope :completed_in_range, ->(range) { where(completed_at: range) } | ||
| scope :updated_in_range, ->(range) { where(updated_at: range) } |
There was a problem hiding this comment.
The updated_in_range scope is defined but not used anywhere in the codebase. This scope is not referenced in ActivityService or any other controller/service. Consider removing it to keep the codebase clean, or if it's intended for future use, consider adding it when it's actually needed.
| scope :updated_in_range, ->(range) { where(updated_at: range) } |
|
|
||
| test "returns todos created in range" do | ||
| # Create a todo within this week | ||
| todo = @user.todos.create!( |
| end | ||
|
|
||
| test "returns projects created in range" do | ||
| project = @user.projects.create!( |
|
|
||
| test "returns milestones created in range" do | ||
| project = projects(:one) | ||
| milestone = project.milestones.create!( |
|
|
||
| test "thought includes project cross-reference" do | ||
| project = projects(:one) | ||
| thought = project.thoughts.create!(content: "Test thought") |
|
|
||
| test "returns resources created in range" do | ||
| project = projects(:one) | ||
| resource = project.resources.create!(url: "https://example.com") |
|
|
||
| test "returns journal entries created in range" do | ||
| project = projects(:one) | ||
| entry = project.journal_entries.create!(content: "Test journal entry") |
|
|
||
| test "excludes other users data" do | ||
| other_user = users(:two) | ||
| other_todo = other_user.todos.create!( |
There was a problem hiding this comment.
This assignment to other_todo is useless, since its value is never read.
| other_todo = other_user.todos.create!( | |
| other_user.todos.create!( |
1 participant
No description provided.