[webhooks] add deviceId field to the API for compatibility with Clo…#224
[webhooks] add deviceId field to the API for compatibility with Clo…#224
deviceId field to the API for compatibility with Clo…#224Conversation
|
""" WalkthroughA new nullable Changes
Sequence Diagram(s)sequenceDiagram
participant Client
participant WebhooksRoutes
participant LocalServerSettings
participant WebHooksService
Client->>WebhooksRoutes: POST /webhook (WebHookDTO)
WebhooksRoutes->>LocalServerSettings: get deviceId
alt WebHookDTO.deviceId != null and != localServerSettings.deviceId
WebhooksRoutes-->>Client: throw IllegalArgumentException("Device ID mismatch")
else
WebhooksRoutes->>WebHooksService: replace(WebHookDTO)
WebHooksService-->>WebhooksRoutes: WebHookDTO (deviceId = null)
WebhooksRoutes-->>Client: WebHookDTO response
end
Possibly related PRs
📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (6)
🚧 Files skipped from review as they are similar to previous changes (6)
⏰ Context from checks skipped due to timeout of 90000ms (1)
✨ Finishing Touches
🧪 Generate Unit Tests
🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. 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: 0
🧹 Nitpick comments (1)
app/src/main/java/me/capcom/smsgateway/modules/webhooks/WebHooksService.kt (1)
50-59: Consider adding deviceId validation in sync method.The
syncmethod doesn't have the same deviceId validation as thereplacemethod. For consistency in webhook handling, consider validating deviceId in the sync method as well.fun sync(source: EntitySource, webHooks: List<WebHookDTO>) { + // Validate device IDs if present + webHooks.forEach { webHook -> + if (webHook.deviceId != null && webHook.deviceId != localServerSettings.deviceId) { + throw IllegalArgumentException("Device ID mismatch") + } + } + webHooksDao.replaceAll(source, webHooks.map { WebHook( id = requireNotNull(it.id) { "ID is required for sync" },
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
app/src/main/java/me/capcom/smsgateway/modules/gateway/workers/WebhooksUpdateWorker.kt(1 hunks)app/src/main/java/me/capcom/smsgateway/modules/webhooks/WebHooksService.kt(3 hunks)app/src/main/java/me/capcom/smsgateway/modules/webhooks/domain/WebHookDTO.kt(1 hunks)docs/api/swagger.json(1 hunks)
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: Build / build-apk
🔇 Additional comments (7)
app/src/main/java/me/capcom/smsgateway/modules/webhooks/domain/WebHookDTO.kt (1)
7-7: Property addition looks good.The addition of the nullable
deviceIdfield to the data class is appropriate for supporting the cloud server compatibility requirement while maintaining backward compatibility.app/src/main/java/me/capcom/smsgateway/modules/gateway/workers/WebhooksUpdateWorker.kt (1)
41-41: Appropriate null handling for cloud webhooks.Setting
deviceIdto null for cloud webhooks is consistent with the design intent, as device identifiers are managed separately for cloud communication.app/src/main/java/me/capcom/smsgateway/modules/webhooks/WebHooksService.kt (3)
42-42: Consistent null handling in select method.Setting
deviceIdto null in the returned DTOs is consistent with the approach in other parts of the codebase.
73-77: Good validation for device ID consistency.The validation ensures that webhooks cannot be associated with incorrect devices, which is crucial for proper routing and security.
89-89: Consistent null handling in replace method.Setting
deviceIdto null in the returned DTOs maintains consistency with the pattern established in the select method.docs/api/swagger.json (2)
652-657: API documentation properly updated.The OpenAPI specification has been properly updated to include the new
deviceIdfield with appropriate nullability and description.
647-651: Logical property ordering.Moving the
idproperty to the top of the properties list improves readability by emphasizing the primary identifier first.
🤖 Pull request artifacts
|
capcom6
force-pushed
the
issue/215-webhook-deviceid-api
branch
from
c5e9a43 to
3014df0
Compare
1 participant
…ud server
Summary by CodeRabbit
New Features
Bug Fixes
Documentation