[messages] add get sent messages endpoint #159
Conversation
WalkthroughThis change introduces a new GET endpoint to retrieve message states with filtering and pagination, updates conversion logic for message and message state DTOs, expands repository and service methods to support flexible querying, and updates API documentation accordingly. Supporting changes include new query parameter structs, test updates, and dependency version bump. Changes
Sequence Diagram(s)sequenceDiagram
participant Client
participant API
participant Auth
participant Service
participant Repository
Client->>API: GET /3rdparty/v1/messages?[filters]
API->>Auth: Validate user credentials
Auth-->>API: Authenticated user
API->>Service: SelectStates(user, filter, options)
Service->>Repository: Select(filter, options)
Repository-->>Service: Messages[], totalCount
Service-->>API: MessageStateOut[], totalCount
API-->>Client: 200 OK (JSON array of MessageState DTOs, X-Total-Count header)
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Note ⚡️ Unit Test Generation is now available in beta!Learn more here, or try it out under "Finishing Touches" below. ✨ Finishing Touches
🧪 Generate unit tests
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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![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
🧹 Nitpick comments (1)
pkg/swagger/docs/swagger.json (1)
1703-1759: Consider consolidating duplicate schema definitions.The
MessageStatedefinition appears to be identical to the existingGetMessageResponsedefinition. This duplication could lead to maintenance issues if the schemas need to evolve differently over time.Consider either:
- Using
GetMessageResponsedirectly in the endpoint response if they serve the same purpose- Adding a comment explaining why separate definitions are needed if they're intended to diverge in the future
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
go.sumis excluded by!**/*.sum
📒 Files selected for processing (13)
go.mod(1 hunks)internal/sms-gateway/handlers/converters/messages.go(2 hunks)internal/sms-gateway/handlers/converters/messages_test.go(1 hunks)internal/sms-gateway/handlers/messages/3rdparty.go(4 hunks)internal/sms-gateway/handlers/messages/params.go(1 hunks)internal/sms-gateway/handlers/mobile.go(1 hunks)internal/sms-gateway/modules/messages/repository.go(2 hunks)internal/sms-gateway/modules/messages/repository_filter.go(1 hunks)internal/sms-gateway/modules/messages/service.go(2 hunks)pkg/swagger/docs/mobile.http(1 hunks)pkg/swagger/docs/requests.http(1 hunks)pkg/swagger/docs/swagger.json(2 hunks)pkg/swagger/docs/swagger.yaml(2 hunks)
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: in the sse service send method (internal/sms-gateway/modules/sse/service.go), blocking on full chann...
Learnt from: capcom6
PR: android-sms-gateway/server#154
File: internal/sms-gateway/modules/sse/service.go:67-79
Timestamp: 2025-07-22T00:31:28.796Z
Learning: In the SSE service Send method (internal/sms-gateway/modules/sse/service.go), blocking on full channels is intentional and acceptable. Timeout control is handled at the calling code level through context, not within the Send method itself. The user prefers this approach over adding a default case that would drop messages when channels are full.
Applied to files:
internal/sms-gateway/handlers/messages/3rdparty.go
🧬 Code Graph Analysis (5)
internal/sms-gateway/handlers/converters/messages_test.go (1)
internal/sms-gateway/handlers/converters/messages.go (1)
MessageToMobileDTO(8-43)
internal/sms-gateway/handlers/converters/messages.go (1)
internal/sms-gateway/modules/messages/domain.go (2)
MessageOut(25-29)MessageStateOut(42-51)
internal/sms-gateway/modules/messages/repository_filter.go (1)
internal/sms-gateway/modules/messages/models.go (1)
ProcessingState(12-12)
internal/sms-gateway/handlers/messages/params.go (2)
internal/sms-gateway/modules/messages/repository_filter.go (2)
MessagesSelectFilter(5-12)MessagesSelectOptions(14-21)internal/sms-gateway/modules/messages/models.go (1)
ProcessingState(12-12)
internal/sms-gateway/handlers/messages/3rdparty.go (3)
internal/sms-gateway/models/models.go (1)
User(17-23)internal/sms-gateway/handlers/converters/messages.go (1)
MessageStateToDTO(45-55)internal/sms-gateway/handlers/middlewares/userauth/userauth.go (1)
WithUser(121-125)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (4)
- GitHub Check: lint
- GitHub Check: E2E
- GitHub Check: Test
- GitHub Check: Analyze (go)
🔇 Additional comments (28)
go.mod (1)
9-9: Approve pseudo-version update for client-goThe upstream’s latest tagged release is v1.7.0 and doesn’t include the commit that adds the
GetMessagesResponsetype required by our new endpoint. Using the pseudo-versionv1.9.2-0.20250805133721-9ebe535d038ais therefore justified to pull in that change.• File: go.mod (line 9)
• Dependency:github.com/android-sms-gateway/client-go v1.9.2-0.20250805133721-9ebe535d038aConsider reverting to an official tagged release (e.g., v1.9.2) once it’s published to improve stability.
pkg/swagger/docs/mobile.http (1)
17-17: Confirm/deviceauth requirement
I wasn’t able to find any authentication checks for the POST/devicehandler in the codebase. Please double-check whether this endpoint should remain unsecured (and the docs updated accordingly) or if Basic auth is still required to avoid misleading consumers.internal/sms-gateway/handlers/converters/messages_test.go (1)
82-82: LGTM!The function call correctly uses the renamed
MessageToMobileDTOfunction, maintaining consistency with the refactoring changes.pkg/swagger/docs/requests.http (1)
77-84: LGTM!The new HTTP request examples properly demonstrate the new GET messages endpoint with both basic usage and comprehensive query parameter filtering. The parameter formats and values are appropriate.
internal/sms-gateway/handlers/mobile.go (1)
175-175: LGTM!The function call correctly uses the renamed
MessageToMobileDTOfunction, maintaining consistency with the converter refactoring across the codebase.internal/sms-gateway/handlers/converters/messages.go (2)
8-8: Good naming clarification.The renaming from
MessageToDTOtoMessageToMobileDTObetter clarifies the intended usage context for mobile endpoints.
45-55: Well-implemented converter function.The
MessageStateToDTOfunction correctly maps all fields from the domain model to the API DTO, including proper type conversion for theStatefield.internal/sms-gateway/modules/messages/repository_filter.go (3)
3-3: Appropriate import addition.The
timeimport is correctly added to support the new date filtering fields.
5-12: Well-designed filter expansion.The additional filter fields provide comprehensive filtering capabilities with appropriate types for each field. The structure supports the new message listing requirements effectively.
19-21: Proper pagination support.The
LimitandOffsetfields correctly extend the options struct to support pagination functionality.internal/sms-gateway/modules/messages/service.go (3)
105-105: Consistent filter usage.The
UpdateStatemethod correctly uses the newMessagesSelectFilterstruct withExtIDandDeviceIDfields, maintaining the same filtering logic with improved structure.
135-144: Well-implemented message state selection.The
SelectStatesmethod properly:
- Sets the user scope by assigning
filter.UserID = user.ID- Delegates to the repository's
Selectmethod- Maps results using the existing
modelToMessageStatefunction- Returns both results and total count for pagination support
- Includes appropriate error handling
148-148: Improved user scoping.The
GetStatemethod now correctly includesUserIDin the filter, ensuring proper user-based access control at the query level rather than relying on post-query validation.internal/sms-gateway/handlers/messages/params.go (4)
15-22: Comprehensive query parameter validation.The
getQueryParamsstruct includes appropriate validation tags for all fields:
- Date format validation with RFC3339
- State enumeration validation
- Device ID length validation (21 chars)
- Pagination bounds validation
24-30: Good date range validation.The
Validatemethod correctly ensures that the start date is before the end date when both are provided.
32-56: Robust filter conversion.The
ToFiltermethod properly handles optional fields and correctly parses RFC3339 dates while gracefully handling parsing errors by skipping invalid values.
58-75: Well-structured options conversion.The
ToOptionsmethod correctly sets default options forWithRecipientsandWithStatestotrue, and handles pagination parameters appropriately (aside from the limit assignment issue noted above).pkg/swagger/docs/swagger.yaml (2)
312-353: Complete API schema definition.The
smsgateway.MessageStatedefinition correctly mirrors the converter function output with:
- All required fields properly marked
- Appropriate field types and constraints
- Consistent property names and descriptions
- Proper references to other schema definitions
964-1021: Comprehensive endpoint documentation.The GET
/3rdparty/v1/messagesendpoint is well-documented with:
- All query parameters from the params struct
- Appropriate parameter types and descriptions
- Default values for pagination parameters
- Proper response schema reference
- Standard error responses (400, 401, 500)
- Correct security configuration
pkg/swagger/docs/swagger.json (1)
267-354: Well-structured API endpoint definition.The new GET endpoint for message retrieval is properly documented with comprehensive query parameters, authentication, and error responses. The parameter validation constraints and descriptions are appropriate.
internal/sms-gateway/handlers/messages/3rdparty.go (4)
6-6: LGTM!The new imports are necessary for the added functionality -
strconvfor header conversion andconvertersfor DTO transformation.Also applies to: 11-11
174-208: Well-implemented message listing endpoint.The implementation follows established patterns with proper error handling, logging, and response formatting. The X-Total-Count header is a good practice for paginated APIs.
Note: The Swagger documentation references
smsgateway.GetMessagesResponsebut the actual response is an array ofsmsgateway.MessageState. This appears to be a documentation inconsistency that should be addressed in the Swagger file.
235-235: Excellent refactor for consistency.Using the converter function eliminates code duplication and ensures consistent response formatting between the get and list endpoints.
275-275: LGTM!The route registration correctly maps the new list endpoint and adds the necessary route name for URL generation.
Also applies to: 277-277
internal/sms-gateway/modules/messages/repository.go (4)
19-19: LGTM!The new error type improves error handling specificity in the Get method.
25-92: Well-designed unified query method.The Select method effectively consolidates query logic with comprehensive filtering, proper pagination, and conditional preloading. The implementation follows best practices:
- Executes count query before pagination for accurate totals
- Uses proper joins for user filtering
- Conditional preloading for performance optimization
- Appropriate error handling and logging
94-104: Good refactor using unified query approach.The SelectPending method now leverages the unified Select method, eliminating code duplication while maintaining the same functionality.
106-121: Excellent refactor with improved error handling.The Get method now uses the unified Select approach and provides better error specificity by distinguishing between "not found" and "multiple found" scenarios.
capcom6
force-pushed
the
feature/get-sent-messages
branch
2 times, most recently
from
52eab03 to
ce885bd
Compare
capcom6
force-pushed
the
feature/get-sent-messages
branch
from
512a7e2 to
80c5c60
Compare
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
internal/sms-gateway/modules/messages/repository.go (1)
25-92: Comprehensive Select method with robust filtering and pagination.The implementation correctly applies filters in a logical sequence, handles pagination, and provides flexible preloading options. The query building is sound and the error handling is appropriate.
Minor optimization suggestion: Consider using
query.Session(&gorm.Session{})to avoid counting on the same query object that will be modified for pagination.func (r *repository) Select(filter MessagesSelectFilter, options MessagesSelectOptions) ([]Message, int64, error) { - query := r.db.Model(&Message{}) + baseQuery := r.db.Model(&Message{}) // Apply date range filter if !filter.StartDate.IsZero() { - query = query.Where("messages.created_at >= ?", filter.StartDate) + baseQuery = baseQuery.Where("messages.created_at >= ?", filter.StartDate) } if !filter.EndDate.IsZero() { - query = query.Where("messages.created_at < ?", filter.EndDate) + baseQuery = baseQuery.Where("messages.created_at < ?", filter.EndDate) } // Apply ID filter if filter.ExtID != "" { - query = query.Where("messages.ext_id = ?", filter.ExtID) + baseQuery = baseQuery.Where("messages.ext_id = ?", filter.ExtID) } // Apply user filter if filter.UserID != "" { - query = query. + baseQuery = baseQuery. Joins("JOIN devices ON messages.device_id = devices.id"). Where("devices.user_id = ?", filter.UserID) } // Apply state filter if filter.State != "" { - query = query.Where("messages.state = ?", filter.State) + baseQuery = baseQuery.Where("messages.state = ?", filter.State) } // Apply device filter if filter.DeviceID != "" { - query = query.Where("messages.device_id = ?", filter.DeviceID) + baseQuery = baseQuery.Where("messages.device_id = ?", filter.DeviceID) } // Get total count var total int64 - if err := query.Count(&total).Error; err != nil { + if err := baseQuery.Count(&total).Error; err != nil { return nil, 0, err } + query := baseQuery // Apply pagination if options.Limit > 0 { query = query.Limit(options.Limit) }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
go.sumis excluded by!**/*.sum
📒 Files selected for processing (13)
go.mod(1 hunks)internal/sms-gateway/handlers/converters/messages.go(2 hunks)internal/sms-gateway/handlers/converters/messages_test.go(1 hunks)internal/sms-gateway/handlers/messages/3rdparty.go(4 hunks)internal/sms-gateway/handlers/messages/params.go(1 hunks)internal/sms-gateway/handlers/mobile.go(1 hunks)internal/sms-gateway/modules/messages/repository.go(2 hunks)internal/sms-gateway/modules/messages/repository_filter.go(1 hunks)internal/sms-gateway/modules/messages/service.go(2 hunks)pkg/swagger/docs/mobile.http(1 hunks)pkg/swagger/docs/requests.http(1 hunks)pkg/swagger/docs/swagger.json(2 hunks)pkg/swagger/docs/swagger.yaml(2 hunks)
✅ Files skipped from review due to trivial changes (3)
- internal/sms-gateway/handlers/converters/messages_test.go
- go.mod
- pkg/swagger/docs/mobile.http
🚧 Files skipped from review as they are similar to previous changes (8)
- pkg/swagger/docs/requests.http
- internal/sms-gateway/modules/messages/service.go
- pkg/swagger/docs/swagger.yaml
- internal/sms-gateway/handlers/converters/messages.go
- internal/sms-gateway/modules/messages/repository_filter.go
- internal/sms-gateway/handlers/messages/3rdparty.go
- internal/sms-gateway/handlers/messages/params.go
- internal/sms-gateway/handlers/mobile.go
🧰 Additional context used
🧬 Code Graph Analysis (1)
internal/sms-gateway/modules/messages/repository.go (2)
internal/sms-gateway/modules/messages/repository_filter.go (2)
MessagesSelectFilter(5-12)MessagesSelectOptions(14-21)internal/sms-gateway/modules/messages/models.go (2)
Message(35-55)ProcessingStatePending(16-16)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (3)
- GitHub Check: E2E
- GitHub Check: lint
- GitHub Check: Analyze (go)
🔇 Additional comments (6)
pkg/swagger/docs/swagger.json (2)
267-354: LGTM! Well-structured endpoint with comprehensive filtering and pagination support.The new GET endpoint follows established API patterns with proper authentication, consistent parameter naming, and comprehensive error handling. The filtering options (date range, state, deviceId) and pagination (limit/offset with sensible defaults) provide flexible query capabilities.
1703-1759: New MessageState schema is well-defined and consistent.The schema properly defines required fields (
deviceId,id,recipients,state) and optional fields (isEncrypted,isHashed,states). The field types, constraints, and examples are consistent with existing schemas in the API.internal/sms-gateway/modules/messages/repository.go (4)
7-7: Import addition is appropriate.The
fmtpackage is now used for error wrapping in the updated methods.
19-19: New error constant is well-defined.The
ErrMultipleMessagesFounderror provides clear semantics for the Get method's uniqueness enforcement.
94-104: SelectPending wrapper maintains backward compatibility.The method correctly uses the new Select method with appropriate filters and options, maintaining the existing interface while benefiting from the consolidated query logic.
106-121: Get method properly enforces uniqueness.The implementation correctly handles the cases of no results (ErrMessageNotFound) and multiple results (ErrMultipleMessagesFound), which is important for maintaining data consistency expectations.
|
Staging... |
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
test/e2e/messages_test.go (1)
115-147: Appropriate date range filtering validation.The date range test uses realistic ISO 8601 timestamps and validates the response structure, though it could benefit from more specific date validation.
Consider adding validation to ensure returned messages actually fall within the specified date range:
// Verify response structure for _, msg := range result { if msg.ID == "" { t.Error("message ID is empty") } if msg.DeviceID == "" { t.Error("device ID is empty") } + // TODO: Add timestamp validation once message timestamps are exposed in the API }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
test/e2e/device_selection_test.go(1 hunks)test/e2e/messages_test.go(1 hunks)test/e2e/priority_test.go(1 hunks)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (3)
- GitHub Check: E2E
- GitHub Check: lint
- GitHub Check: Analyze (go)
🔇 Additional comments (15)
test/e2e/device_selection_test.go (1)
12-12: Excellent practice for test isolation!Cloning the shared client before setting authentication prevents side effects and ensures test isolation. This is a good defensive programming practice that maintains the integrity of shared test resources.
test/e2e/priority_test.go (1)
50-50: Consistent application of client isolation pattern.The same client cloning approach as in
device_selection_test.goensures consistent test isolation across the test suite. This systematic improvement prevents potential authentication state leakage between tests.test/e2e/messages_test.go (13)
10-27: Well-designed test data structures.The data structures properly model the expected API responses with appropriate JSON tags. The nested
statestruct and error response handling provide good test coverage for different response scenarios.
29-31: Consistent client isolation pattern.Following the same client cloning pattern as the other test files ensures proper test isolation and prevents authentication state leakage.
40-78: Comprehensive validation for successful retrieval test.The test properly validates response structure, status code, headers, and JSON unmarshalling. Good defensive checks for empty fields that could indicate API issues.
80-113: Thorough pagination testing.The pagination test correctly validates both the limit parameter and the
X-Total-Countheader, ensuring proper API contract compliance.
149-175: Effective state filtering validation.The state filter test properly validates that all returned messages have the expected state, ensuring the filtering functionality works correctly.
177-206: Good device ID filtering test structure.The device ID filter test uses the actual registered device ID, which is more realistic than hardcoded values.
208-231: Proper error handling validation for invalid date format.The test correctly validates both the HTTP status code and the presence of an error message in the response body.
233-256: Appropriate validation for invalid state values.Testing with an invalid state value ensures proper input validation on the server side.
258-281: Good device ID validation testing.Using an invalid device ID length tests the server's input validation for device ID format requirements.
283-305: Essential authentication testing.Testing missing authentication ensures the endpoint is properly secured and returns appropriate HTTP status codes.
307-329: Comprehensive invalid credentials testing.Testing with invalid credentials complements the missing authentication test and ensures proper authentication handling.
332-350: Well-structured test execution logic.The table-driven test execution properly handles setup, request execution, status code validation, and custom validation functions. Good error handling with descriptive failure messages.
1-351: Excellent comprehensive test suite.This e2e test file provides thorough coverage of the new messages endpoint with:
- Proper test isolation using client cloning
- Comprehensive test cases covering happy paths, edge cases, and error scenarios
- Good validation of response structure, headers, and error messages
- Realistic test data and scenarios
- Clear, maintainable table-driven test structure
The test suite effectively validates the new GET messages endpoint functionality and ensures API contract compliance.
Summary by CodeRabbit
New Features
Bug Fixes
Documentation