[messages] provide content with the state#219
Merged
Conversation
📝 WalkthroughWalkthroughReworks message domain to nested content types, replaces exported DB DTOs with internal models, updates repository/service/cache/handlers/converters/tests to new types, adds optional includeContent query flag and OpenAPI/schema changes, and updates tooling and HTTP example requests. Changes
Sequence Diagram(s)sequenceDiagram
participant Client as Client
participant Controller as 3rdPartyController
participant Service as MessagesService
participant Repo as Repository (DB)
participant Cache as Cache
Client->>Controller: POST /3rdparty/v1/messages (MessageInput)
Controller->>Service: Enqueue(MessageInput)
Service->>Repo: Insert(messageModel)
Repo-->>Service: inserted messageModel
Service->>Cache: Set(userID, id, *MessageState)
Cache-->>Service: OK
Service-->>Controller: MessageState
Controller-->>Client: 202 Created / GetMessageResponse(MessageStateToDTO)
Estimated code review effort🎯 4 (Complex) | ⏱️ ~75 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. 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 |
![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 (2)
internal/sms-gateway/handlers/converters/messages_test.go (1)
90-148: Tests cover the main paths; consider adding a case forDataContentwithincludeContent=true.The current tests verify:
- Text content included when
includeContent=true- Content omitted when
includeContent=falseFor completeness, consider adding a test case that verifies
DataMessageis populated whenincludeContent=trueandDataContentis present.💡 Additional test case suggestion
{ name: "Data content included", input: messages.MessageWithStateOut{ MessageStateOut: messages.MessageStateOut{ MessageStateIn: messages.MessageStateIn{ ID: "msg-3", State: messages.ProcessingStatePending, }, DeviceID: "device-3", }, DataContent: &messages.DataMessageContent{Data: "AQID", Port: 1234}, }, includeContent: true, expected: converters.MessageListItemDTO{ MessageState: smsgateway.MessageState{ ID: "msg-3", DeviceID: "device-3", State: smsgateway.ProcessingStatePending, }, DataMessage: &smsgateway.DataMessage{Data: "AQID", Port: 1234}, }, },🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@internal/sms-gateway/handlers/converters/messages_test.go` around lines 90 - 148, Add a third table-driven test case in TestMessageListItemToDTO that covers DataContent with includeContent=true: create an input messages.MessageWithStateOut whose DataContent is &messages.DataMessageContent{Data: "AQID", Port: 1234}, set includeContent to true, and assert the resulting converters.MessageListItemDTO contains MessageState (ID/device/state) and DataMessage: &smsgateway.DataMessage{Data: "AQID", Port: 1234}; this ensures converters.MessageListItemToDTO correctly maps DataContent to DataMessage when content inclusion is requested.internal/sms-gateway/handlers/messages/3rdparty.go (1)
224-228: Address exhaustruct linter warning by explicitly initializing all fields.The static analysis tool flags that
TextMessageandDataMessagefields are not explicitly initialized. While Go zero-initializes them tonil, explicitly setting them makes the intent clearer and satisfies the linter.🔧 Proposed fix to satisfy the linter
return c.JSON( slices.Map(msgs, func(msg messages.MessageStateOut) converters.MessageListItemDTO { return converters.MessageListItemDTO{ MessageState: converters.MessageStateToDTO(msg), + TextMessage: nil, + DataMessage: nil, } }), )🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@internal/sms-gateway/handlers/messages/3rdparty.go` around lines 224 - 228, The linter complains that TextMessage and DataMessage in the constructed converters.MessageListItemDTO are not explicitly initialized; update the slices.Map call that maps messages.MessageStateOut to converters.MessageListItemDTO (the closure using converters.MessageStateToDTO(msg)) to explicitly set TextMessage: nil and DataMessage: nil in the returned struct literal so all fields are initialized.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@internal/sms-gateway/handlers/converters/messages.go`:
- Around line 58-61: The exhaustruct linter requires every field of
MessageListItemDTO be explicitly set in MessageListItemToDTO; update the
MessageListItemToDTO function to initialize all fields of the MessageListItemDTO
literal (e.g., ID, From, To, Content, CreatedAt, MessageState, etc.)—set Content
conditionally based on includeContent and set other omitted fields to their zero
values explicitly (nil, "" or zero time) so no field is left unspecified; ensure
you reference MessageListItemDTO and MessageStateToDTO(message.MessageStateOut)
when building the struct.
---
Nitpick comments:
In `@internal/sms-gateway/handlers/converters/messages_test.go`:
- Around line 90-148: Add a third table-driven test case in
TestMessageListItemToDTO that covers DataContent with includeContent=true:
create an input messages.MessageWithStateOut whose DataContent is
&messages.DataMessageContent{Data: "AQID", Port: 1234}, set includeContent to
true, and assert the resulting converters.MessageListItemDTO contains
MessageState (ID/device/state) and DataMessage: &smsgateway.DataMessage{Data:
"AQID", Port: 1234}; this ensures converters.MessageListItemToDTO correctly maps
DataContent to DataMessage when content inclusion is requested.
In `@internal/sms-gateway/handlers/messages/3rdparty.go`:
- Around line 224-228: The linter complains that TextMessage and DataMessage in
the constructed converters.MessageListItemDTO are not explicitly initialized;
update the slices.Map call that maps messages.MessageStateOut to
converters.MessageListItemDTO (the closure using
converters.MessageStateToDTO(msg)) to explicitly set TextMessage: nil and
DataMessage: nil in the returned struct literal so all fields are initialized.
🪄 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: ca7478ec-a9e9-4182-b191-1c8c4edc2b74
📒 Files selected for processing (9)
internal/sms-gateway/handlers/converters/messages.gointernal/sms-gateway/handlers/converters/messages_3rdparty.gointernal/sms-gateway/handlers/converters/messages_test.gointernal/sms-gateway/handlers/messages/3rdparty.gointernal/sms-gateway/handlers/messages/params.gointernal/sms-gateway/handlers/messages/responses.gointernal/sms-gateway/modules/messages/domain.gointernal/sms-gateway/modules/messages/service.gointernal/sms-gateway/openapi/docs.go
🤖 Pull request artifacts
|
capcom6
force-pushed
the
codex/add-message-content-access-to-get-/messages
branch
from
e2081ab to
31b1b69
Compare
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (1)
internal/sms-gateway/modules/messages/domain.go (1)
52-59: Consider JSON tag consistency forMessageState.
MessageStateInputhas JSON tags on its fields (lines 46-49), butMessageStateembeds it without tags on the embedding line. When marshaling, Go will flatten embedded fields correctly. However, theDeviceID,IsHashed, andIsEncryptedfields usesnake_caseJSON tags while the embeddedMessageStateInputfields usecamelCase("id","state","recipients","states"). Verify this mixed casing is intentional for API compatibility.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@internal/sms-gateway/modules/messages/domain.go` around lines 52 - 59, The JSON tags across the MessageState type are inconsistent: MessageState embeds MessageStateInput (which uses camelCase tags like "id","state","recipients","states") while DeviceID, IsHashed and IsEncrypted use snake_case tags ("device_id","is_hashed","is_encrypted"); decide on a single convention for the API and make the tags consistent by updating the tags on DeviceID/IsHashed/IsEncrypted to match MessageStateInput's camelCase or alternatively change MessageStateInput tags to snake_case, and ensure MessageState still embeds MessageStateInput unchanged so marshaling flattens fields correctly (update MessageState struct field tags accordingly).
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@internal/sms-gateway/modules/messages/models.go`:
- Around line 135-142: GetHashedContent currently returns a non-nil hashed
payload even when the row was loaded without the Content column, causing
serialized hashedContent.hash="" instead of omitting it; update
messageModel.GetHashedContent to only return a non-nil *smsgateway.HashedMessage
when m.IsHashed is true AND m.Content is non-empty (e.g., check m.Content != ""
or a dedicated flag), otherwise return nil, so toStateDomain() will omit
hashedContent when Content was not loaded.
In `@internal/sms-gateway/modules/messages/service.go`:
- Around line 85-88: The read uses s.messages.get with default SelectOptions
(SelectOptions{}) which omits Content, then you cache that contentless result
and GetState returns cache hits without content; fix by requesting Content when
fetching for caching (e.g., call s.messages.get with SelectOptions that include
Content) or avoid caching reads that lack Content — update the call sites around
s.messages.get/SelectFilter/SelectOptions and the cache write logic (the
conversion and caching block that currently stores the result) so only
contentful payloads are cached (or fetch Content before caching) to prevent
GetState returning contentless cached entries.
---
Nitpick comments:
In `@internal/sms-gateway/modules/messages/domain.go`:
- Around line 52-59: The JSON tags across the MessageState type are
inconsistent: MessageState embeds MessageStateInput (which uses camelCase tags
like "id","state","recipients","states") while DeviceID, IsHashed and
IsEncrypted use snake_case tags ("device_id","is_hashed","is_encrypted"); decide
on a single convention for the API and make the tags consistent by updating the
tags on DeviceID/IsHashed/IsEncrypted to match MessageStateInput's camelCase or
alternatively change MessageStateInput tags to snake_case, and ensure
MessageState still embeds MessageStateInput unchanged so marshaling flattens
fields correctly (update MessageState struct field tags accordingly).
🪄 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: 2b2f51a4-87a5-4fc2-af7a-be9f2229899f
⛔ Files ignored due to path filters (1)
go.sumis excluded by!**/*.sum
📒 Files selected for processing (17)
Makefileapi/local.httpapi/requests.httpgo.modinternal/sms-gateway/handlers/converters/messages.gointernal/sms-gateway/handlers/converters/messages_test.gointernal/sms-gateway/handlers/messages/3rdparty.gointernal/sms-gateway/handlers/messages/mobile.gointernal/sms-gateway/handlers/messages/params.gointernal/sms-gateway/modules/messages/cache.gointernal/sms-gateway/modules/messages/converters.gointernal/sms-gateway/modules/messages/domain.gointernal/sms-gateway/modules/messages/models.gointernal/sms-gateway/modules/messages/repository.gointernal/sms-gateway/modules/messages/repository_filter.gointernal/sms-gateway/modules/messages/service.gointernal/sms-gateway/openapi/docs.go
✅ Files skipped from review due to trivial changes (3)
- api/local.http
- go.mod
- internal/sms-gateway/handlers/converters/messages_test.go
🚧 Files skipped from review as they are similar to previous changes (3)
- internal/sms-gateway/handlers/messages/params.go
- internal/sms-gateway/handlers/converters/messages.go
- internal/sms-gateway/openapi/docs.go
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
internal/sms-gateway/modules/messages/service.go (1)
275-286:⚠️ Potential issue | 🟠 MajorReject requests that set both
textContentanddataContent.Lines 276-283 silently prefer
TextContentand dropDataContent. That turns a malformed request into a different persisted message instead of failing fast.💡 Suggested validation
switch { + case message.TextContent != nil && message.DataContent != nil: + return nil, ValidationError("exactly one of textContent or dataContent must be set") case message.TextContent != nil: if setErr := msg.SetTextContent(*message.TextContent); setErr != nil { return nil, fmt.Errorf("failed to set text content: %w", setErr) } case message.DataContent != nil:🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@internal/sms-gateway/modules/messages/service.go` around lines 275 - 286, The code currently silently prefers message.TextContent over message.DataContent in the switch, mutating msg with SetTextContent and dropping the other; instead validate and reject requests that set both by checking if message.TextContent != nil && message.DataContent != nil and returning a clear error (e.g., ErrAmbiguousContent or reuse ErrNoContent with a new message) before calling msg.SetTextContent or msg.SetDataContent; keep the existing SetTextContent and SetDataContent calls for the mutually exclusive cases and preserve the ErrNoContent return for neither set.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Outside diff comments:
In `@internal/sms-gateway/modules/messages/service.go`:
- Around line 275-286: The code currently silently prefers message.TextContent
over message.DataContent in the switch, mutating msg with SetTextContent and
dropping the other; instead validate and reject requests that set both by
checking if message.TextContent != nil && message.DataContent != nil and
returning a clear error (e.g., ErrAmbiguousContent or reuse ErrNoContent with a
new message) before calling msg.SetTextContent or msg.SetDataContent; keep the
existing SetTextContent and SetDataContent calls for the mutually exclusive
cases and preserve the ErrNoContent return for neither set.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: d51c568f-4d9a-47b4-b761-e17827975a86
📒 Files selected for processing (3)
internal/sms-gateway/modules/messages/domain.gointernal/sms-gateway/modules/messages/models.gointernal/sms-gateway/modules/messages/service.go
capcom6
force-pushed
the
codex/add-message-content-access-to-get-/messages
branch
from
e0a9da0 to
fe74256
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
internal/sms-gateway/modules/messages/service.go (1)
198-217:⚠️ Potential issue | 🟠 MajorDon’t cache the pre-insert state snapshot.
This
stateis built before the row is inserted, so it is derived from amessageModelthat still has no persistedStateshistory. That incomplete snapshot is then both returned fromEnqueue()and written to cache, soGetState()can keep serving a message with empty state history until another refresh happens. Build the response from the inserted model, or seed the initial pending state before converting/caching it.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@internal/sms-gateway/modules/messages/service.go` around lines 198 - 217, The Enqueue function currently builds state from the pre-insert messageModel (via msg.toStateDomain()) and caches/returns that incomplete snapshot; instead, after s.messages.Insert(msg) either retrieve the inserted model (or update msg with the persisted state) and then build the MessageState to return/cache, or explicitly seed the initial pending state into msg before calling msg.toStateDomain(); update Enqueue to call toStateDomain() only after ensuring msg reflects the inserted/persisted state (use the result of s.messages.Insert or mutate msg to include the initial pending state) and then pass that state into s.cache.Set and as the return value.
🧹 Nitpick comments (1)
Makefile (1)
64-64: Avoid hardcoded runtime env inairtarget; make them overridable.Line 64 forces
TZ/DEBUGfor every run, which makes timezone/debug scenario testing harder from shell or.env-driven workflows.Proposed diff
+AIR_TZ ?= UTC +AIR_DEBUG ?= 1 + air: ## Run development server `@command` -v air >/dev/null 2>&1 || { \ echo "Please install air: go install github.com/air-verse/air@latest"; \ exit 1; \ } `@echo` "Starting development server with air..." - TZ=UTC DEBUG=1 air + TZ=$(AIR_TZ) DEBUG=$(AIR_DEBUG) air🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@Makefile` at line 64, The Makefile's air target hardcodes TZ=UTC and DEBUG=1; change it so these are overridable by environment or Makefile variables (e.g., use Makefile variables or shell-parameter defaults) instead of forcing them every run. Update the air target invocation (and/or introduce TZ and DEBUG variables such as TZ ?= UTC and DEBUG ?=1) so callers can override TZ and DEBUG from the shell or .env, while preserving the current defaults if not provided.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@internal/sms-gateway/modules/messages/domain.go`:
- Around line 52-58: The MessageState struct change broke backward compatibility
for cached JSON; update the MessageState type to preserve compatibility by
either restoring the old JSON tag names for DeviceID / IsHashed / IsEncrypted or
implement a custom UnmarshalJSON for MessageState that accepts both the new and
old key names and maps them into DeviceID, IsHashed and IsEncrypted (handle
missing keys by keeping zero-values). Locate the MessageState struct and
implement the chosen fix so cached entries from the previous release still
deserialize correctly (use MessageState.DeviceID, MessageState.IsHashed,
MessageState.IsEncrypted as the target fields).
---
Outside diff comments:
In `@internal/sms-gateway/modules/messages/service.go`:
- Around line 198-217: The Enqueue function currently builds state from the
pre-insert messageModel (via msg.toStateDomain()) and caches/returns that
incomplete snapshot; instead, after s.messages.Insert(msg) either retrieve the
inserted model (or update msg with the persisted state) and then build the
MessageState to return/cache, or explicitly seed the initial pending state into
msg before calling msg.toStateDomain(); update Enqueue to call toStateDomain()
only after ensuring msg reflects the inserted/persisted state (use the result of
s.messages.Insert or mutate msg to include the initial pending state) and then
pass that state into s.cache.Set and as the return value.
---
Nitpick comments:
In `@Makefile`:
- Line 64: The Makefile's air target hardcodes TZ=UTC and DEBUG=1; change it so
these are overridable by environment or Makefile variables (e.g., use Makefile
variables or shell-parameter defaults) instead of forcing them every run. Update
the air target invocation (and/or introduce TZ and DEBUG variables such as TZ ?=
UTC and DEBUG ?=1) so callers can override TZ and DEBUG from the shell or .env,
while preserving the current defaults if not provided.
🪄 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: 769d65e9-a009-41e1-bab7-e640abf2e212
⛔ Files ignored due to path filters (1)
go.sumis excluded by!**/*.sum
📒 Files selected for processing (18)
Makefileapi/local.httpapi/requests.httpgo.modinternal/sms-gateway/handlers/converters/messages.gointernal/sms-gateway/handlers/converters/messages_test.gointernal/sms-gateway/handlers/messages/3rdparty.gointernal/sms-gateway/handlers/messages/mobile.gointernal/sms-gateway/handlers/messages/params.gointernal/sms-gateway/handlers/webhooks/3rdparty.gointernal/sms-gateway/modules/messages/cache.gointernal/sms-gateway/modules/messages/converters.gointernal/sms-gateway/modules/messages/domain.gointernal/sms-gateway/modules/messages/models.gointernal/sms-gateway/modules/messages/repository.gointernal/sms-gateway/modules/messages/repository_filter.gointernal/sms-gateway/modules/messages/service.gointernal/sms-gateway/openapi/docs.go
✅ Files skipped from review due to trivial changes (3)
- internal/sms-gateway/handlers/webhooks/3rdparty.go
- go.mod
- internal/sms-gateway/handlers/converters/messages_test.go
🚧 Files skipped from review as they are similar to previous changes (10)
- api/local.http
- internal/sms-gateway/handlers/messages/mobile.go
- internal/sms-gateway/modules/messages/repository_filter.go
- internal/sms-gateway/handlers/messages/3rdparty.go
- api/requests.http
- internal/sms-gateway/modules/messages/cache.go
- internal/sms-gateway/openapi/docs.go
- internal/sms-gateway/handlers/messages/params.go
- internal/sms-gateway/modules/messages/converters.go
- internal/sms-gateway/modules/messages/repository.go
capcom6
changed the title
includeContent support to 3rd-party messages list and include message content in responses
capcom6
force-pushed
the
codex/add-message-content-access-to-get-/messages
branch
from
397c19e to
eb21004
Compare
capcom6
force-pushed
the
codex/add-message-content-access-to-get-/messages
branch
from
a36bfc9 to
de0360c
Compare
There was a problem hiding this comment.
🧹 Nitpick comments (1)
api/requests.http (1)
38-39: Optional docs clarity: explain auth mode preference once.Since both Basic and Bearer variants are shown, add a short one-line note near the top clarifying when to use each to avoid ambiguity for API consumers.
Also applies to: 90-91, 115-116
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@api/requests.http` around lines 38 - 39, Add a single-line clarifying note at the top of api/requests.http that explains which auth mode to prefer and when (e.g., "Use Basic {{credentials}} for initial API access/credential exchange; use Bearer {{jwtToken}} after obtaining a JWT for authenticated requests"), and insert the same one-line preference note adjacent to each pair of Authorization header examples (the "Authorization: Basic {{credentials}}" and "Authorization: Bearer {{jwtToken}}" occurrences) so consumers understand when to use Basic vs Bearer across all examples.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@api/requests.http`:
- Around line 38-39: Add a single-line clarifying note at the top of
api/requests.http that explains which auth mode to prefer and when (e.g., "Use
Basic {{credentials}} for initial API access/credential exchange; use Bearer
{{jwtToken}} after obtaining a JWT for authenticated requests"), and insert the
same one-line preference note adjacent to each pair of Authorization header
examples (the "Authorization: Basic {{credentials}}" and "Authorization: Bearer
{{jwtToken}}" occurrences) so consumers understand when to use Basic vs Bearer
across all examples.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 048f627e-01dd-4d3b-a2a0-8b7031ff89a7
⛔ Files ignored due to path filters (1)
go.sumis excluded by!**/*.sum
📒 Files selected for processing (11)
Makefileapi/local.httpapi/requests.httpgo.modinternal/sms-gateway/handlers/converters/messages.gointernal/sms-gateway/handlers/converters/messages_test.gointernal/sms-gateway/handlers/messages/3rdparty.gointernal/sms-gateway/handlers/messages/mobile.gointernal/sms-gateway/handlers/messages/params.gointernal/sms-gateway/handlers/webhooks/3rdparty.gointernal/sms-gateway/openapi/docs.go
✅ Files skipped from review due to trivial changes (3)
- go.mod
- api/local.http
- internal/sms-gateway/handlers/webhooks/3rdparty.go
🚧 Files skipped from review as they are similar to previous changes (6)
- internal/sms-gateway/handlers/messages/mobile.go
- internal/sms-gateway/handlers/messages/params.go
- internal/sms-gateway/handlers/messages/3rdparty.go
- Makefile
- internal/sms-gateway/openapi/docs.go
- internal/sms-gateway/handlers/converters/messages.go
capcom6
force-pushed
the
codex/add-message-content-access-to-get-/messages
branch
from
de0360c to
4f1efb9
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
internal/sms-gateway/modules/messages/domain.go (1)
56-58:⚠️ Potential issue | 🟠 MajorPreserve cache deserialization compatibility for renamed JSON keys.
Line 56–58 uses new key names; cached payloads written with legacy keys can deserialize with zero values on cache hit. Add backward-compatible unmarshal handling (or equivalent migration strategy).
🔧 Backward-compatible unmarshal approach
import ( + "encoding/json" "time" "github.com/android-sms-gateway/client-go/smsgateway" ) @@ type MessageState struct { MessageStateInput MessageStateContent DeviceID string `json:"deviceId"` // Device ID IsHashed bool `json:"isHashed"` // Hashed IsEncrypted bool `json:"isEncrypted"` // Encrypted } + +func (m *MessageState) UnmarshalJSON(data []byte) error { + type alias MessageState + aux := struct { + alias + LegacyDeviceID string `json:"device_id"` + LegacyIsHashed *bool `json:"is_hashed"` + LegacyIsEncrypted *bool `json:"is_encrypted"` + }{} + + if err := json.Unmarshal(data, &aux); err != nil { + return err + } + + *m = MessageState(aux.alias) + if m.DeviceID == "" { + m.DeviceID = aux.LegacyDeviceID + } + if !m.IsHashed && aux.LegacyIsHashed != nil { + m.IsHashed = *aux.LegacyIsHashed + } + if !m.IsEncrypted && aux.LegacyIsEncrypted != nil { + m.IsEncrypted = *aux.LegacyIsEncrypted + } + return nil +}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@internal/sms-gateway/modules/messages/domain.go` around lines 56 - 58, The struct in domain.go that declares DeviceID, IsHashed and IsEncrypted must support deserializing both the new JSON keys and legacy keys; add a custom UnmarshalJSON on that struct which first attempts normal decoding into an alias type and then falls back to reading the raw map[string]json.RawMessage to populate fields from legacy names (e.g. "device_id" -> DeviceID, "is_hashed" -> IsHashed, "is_encrypted" -> IsEncrypted) so cached payloads with old keys still populate the struct, and add/update tests to cover both key formats.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@internal/sms-gateway/openapi/docs.go`:
- Around line 1330-1334: Update the OpenAPI schema descriptions for the
hashedMessage field so they match actual behavior: clarify that hashedMessage
(referenced as smsgateway.HashedMessage) is provided when isHashed=true and the
original content is included, but may be omitted when content is not included;
update both places where hashedMessage is described (the schema block
referencing smsgateway.HashedMessage) to use this wording so docs reflect the
conditional visibility.
---
Duplicate comments:
In `@internal/sms-gateway/modules/messages/domain.go`:
- Around line 56-58: The struct in domain.go that declares DeviceID, IsHashed
and IsEncrypted must support deserializing both the new JSON keys and legacy
keys; add a custom UnmarshalJSON on that struct which first attempts normal
decoding into an alias type and then falls back to reading the raw
map[string]json.RawMessage to populate fields from legacy names (e.g.
"device_id" -> DeviceID, "is_hashed" -> IsHashed, "is_encrypted" -> IsEncrypted)
so cached payloads with old keys still populate the struct, and add/update tests
to cover both key formats.
🪄 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: 16341f0a-0390-4a68-a3c3-bc42ba2c0c68
⛔ Files ignored due to path filters (1)
go.sumis excluded by!**/*.sum
📒 Files selected for processing (18)
Makefileapi/local.httpapi/requests.httpgo.modinternal/sms-gateway/handlers/converters/messages.gointernal/sms-gateway/handlers/converters/messages_test.gointernal/sms-gateway/handlers/messages/3rdparty.gointernal/sms-gateway/handlers/messages/mobile.gointernal/sms-gateway/handlers/messages/params.gointernal/sms-gateway/handlers/webhooks/3rdparty.gointernal/sms-gateway/modules/messages/cache.gointernal/sms-gateway/modules/messages/converters.gointernal/sms-gateway/modules/messages/domain.gointernal/sms-gateway/modules/messages/models.gointernal/sms-gateway/modules/messages/repository.gointernal/sms-gateway/modules/messages/repository_filter.gointernal/sms-gateway/modules/messages/service.gointernal/sms-gateway/openapi/docs.go
✅ Files skipped from review due to trivial changes (4)
- api/local.http
- go.mod
- internal/sms-gateway/handlers/webhooks/3rdparty.go
- internal/sms-gateway/modules/messages/repository.go
🚧 Files skipped from review as they are similar to previous changes (7)
- internal/sms-gateway/handlers/messages/mobile.go
- internal/sms-gateway/modules/messages/repository_filter.go
- Makefile
- internal/sms-gateway/handlers/messages/params.go
- api/requests.http
- internal/sms-gateway/modules/messages/cache.go
- internal/sms-gateway/handlers/messages/3rdparty.go
capcom6
force-pushed
the
codex/add-message-content-access-to-get-/messages
branch
from
4f1efb9 to
1296986
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@api/local.http`:
- Around line 10-11: Update the example HTTP request in api/local.http to use
the canonical plural endpoint by replacing the GET path "/device" with
"/devices" (i.e., change the line "GET {{localUrl}}/device HTTP/1.1" to "GET
{{localUrl}}/devices HTTP/1.1"); keep the Authorization header and other request
parts intact so the example continues to work with the current API.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Motivation
includeContentquery parameter and a richer response schema.Description
MessageWithStateOutto the messages domain and implementmodelToMessageWithStateto decode text and data content from stored messages.SelectWithContenttoServiceto return messages with decoded content and map repository models toMessageWithStateOutusingslices.MapOrError.converters.MessageListItemDTOandMessageListItemToDTOto represent and build list items that optionally includetextMessageanddataMessagefields.includeContentquery param (thirdPartyGetQueryParams.IncludeContent) and to branch betweenSelectWithContent(when true) andSelectStates(when false), returning the appropriate DTOs and settingX-Total-Count.GetMessagesResponsealias type for the new response shape and update OpenAPIdocs.goto document theincludeContentparam and theconverters.MessageListItemDTOresponse schema.MessageListItemToDTOand extend existing message converter tests to cover the new behavior.Testing
go test ./internal/sms-gateway/handlers/converters -run TestMessageListItemToDTOand the new test passed.go test ./...and they completed successfully.Codex Task
Summary by CodeRabbit