[device] sim cards list#367
Merged
Merged
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (8)
WalkthroughCollect active SIM metadata, model it as SimCard, include simCards in gateway register/patch payloads (redacted), thread Context through callers, and expose simCards from the local /device endpoint. ChangesSIM Card Tracking
Sequence Diagram(s)sequenceDiagram
participant Caller
participant GatewayService
participant SubscriptionsHelper
participant GatewayAPI
Caller->>GatewayService: registerDevice(context, pushToken, mode)
GatewayService->>SubscriptionsHelper: getActiveSimCards(context)
SubscriptionsHelper-->>GatewayService: List<SimCard>
GatewayService->>GatewayService: String?.redact() phone/carrier/iccid
GatewayService->>GatewayAPI: DeviceRegisterRequest{simCards=[...]}
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Suggested labels
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
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: 3
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@app/src/main/java/me/capcom/smsgateway/helpers/SubscriptionsHelper.kt`:
- Around line 125-127: The unguarded access to
subscriptionManager.activeSubscriptionInfoList can throw SecurityException when
READ_PHONE_STATE is denied; wrap each access (including the one at lines
130-141) in a try-catch(SecurityException) block inside the SubscriptionsHelper
method (e.g., the function that reads active subscriptions) so that on catch you
return an emptyList or otherwise safely handle the absence of permissions,
optionally logging the exception; do not rely on
`@SuppressLint`("MissingPermission") to prevent runtime crashes.
In `@app/src/main/java/me/capcom/smsgateway/modules/gateway/GatewayApi.kt`:
- Around line 167-168: DevicePatchRequest currently types simCards as
List<SimCard>? using the wrong domain SimCard; change DevicePatchRequest to use
the same nested DTO SimCard used by DeviceRegisterRequest (the redacted DTO) and
update GatewayService.updateDevice to map
SubscriptionsHelper.getActiveSimCards() into that DTO (applying the same
redaction of phone number/ICCID as registerDevice does) so serialization and
sensitive-field handling match; locate DevicePatchRequest.simCards,
DeviceRegisterRequest.SimCard, GatewayService.updateDevice, and
SubscriptionsHelper.getActiveSimCards() to implement the type swap and
mapping/redaction.
In `@app/src/main/java/me/capcom/smsgateway/modules/gateway/GatewayService.kt`:
- Around line 176-184: The updateDevice call is sending raw simCards from
SubscriptionsHelper.getActiveSimCards(context) without redaction; modify the
code that builds GatewayApi.DevicePatchRequest in GatewayService.updateDevice to
map each sim card into GatewayApi.DeviceRegisterRequest.SimCard and call
redact() (same mapping used in registerDevice), so only redacted phone/ICCID are
sent via api.devicePatch; also update the DevicePatchRequest.simCards type in
GatewayApi.kt to List<DeviceRegisterRequest.SimCard>? to match the patched
payload.
🪄 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: 925ded56-d36f-4324-b05b-0582648dbc36
📒 Files selected for processing (8)
app/src/main/java/me/capcom/smsgateway/helpers/SubscriptionsHelper.ktapp/src/main/java/me/capcom/smsgateway/modules/gateway/GatewayApi.ktapp/src/main/java/me/capcom/smsgateway/modules/gateway/GatewayService.ktapp/src/main/java/me/capcom/smsgateway/modules/gateway/workers/RegistrationWorker.ktapp/src/main/java/me/capcom/smsgateway/modules/localserver/WebService.ktapp/src/main/java/me/capcom/smsgateway/modules/localserver/domain/Device.ktapp/src/main/java/me/capcom/smsgateway/modules/localserver/domain/SimCard.ktapp/src/main/java/me/capcom/smsgateway/ui/HomeFragment.kt
Contributor
🤖 Pull request artifacts
|
capcom6
force-pushed
the
feature/sim-cards-list
branch
from
189106a to
bb935b8
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@app/src/main/java/me/capcom/smsgateway/modules/gateway/GatewayApi.kt`:
- Around line 146-147: Remove the empty class body for DeviceRegisterRequest to
satisfy detekt.empty-blocks.EmptyClassBlock; replace the trailing "{ }" after
the constructor/parameter list with nothing so the declaration is just the class
header (i.e., close the declaration immediately after the parameter list),
keeping the class signature (DeviceRegisterRequest) intact and ensuring no
behavior or annotations are removed.
🪄 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: f38c0a53-d9f4-455b-9c5c-017d834494ae
📒 Files selected for processing (8)
app/src/main/java/me/capcom/smsgateway/helpers/SubscriptionsHelper.ktapp/src/main/java/me/capcom/smsgateway/modules/gateway/GatewayApi.ktapp/src/main/java/me/capcom/smsgateway/modules/gateway/GatewayService.ktapp/src/main/java/me/capcom/smsgateway/modules/gateway/workers/RegistrationWorker.ktapp/src/main/java/me/capcom/smsgateway/modules/localserver/WebService.ktapp/src/main/java/me/capcom/smsgateway/modules/localserver/domain/Device.ktapp/src/main/java/me/capcom/smsgateway/modules/localserver/domain/SimCard.ktapp/src/main/java/me/capcom/smsgateway/ui/HomeFragment.kt
🚧 Files skipped from review as they are similar to previous changes (5)
- app/src/main/java/me/capcom/smsgateway/helpers/SubscriptionsHelper.kt
- app/src/main/java/me/capcom/smsgateway/modules/localserver/domain/SimCard.kt
- app/src/main/java/me/capcom/smsgateway/modules/gateway/workers/RegistrationWorker.kt
- app/src/main/java/me/capcom/smsgateway/ui/HomeFragment.kt
- app/src/main/java/me/capcom/smsgateway/modules/gateway/GatewayService.kt
capcom6
force-pushed
the
feature/sim-cards-list
branch
from
1443eb1 to
6649db8
Compare
There was a problem hiding this comment.
🧹 Nitpick comments (1)
app/src/main/java/me/capcom/smsgateway/modules/gateway/GatewayService.kt (1)
135-143: ⚡ Quick winExtract SIM-card mapping/redaction into one helper.
The same mapping logic is duplicated in register and update paths; centralizing it will prevent payload drift in future edits.
Proposed refactor
+ private fun mapSimCards(simCards: List<me.capcom.smsgateway.modules.localserver.domain.SimCard>): List<GatewayApi.SimCard> { + return simCards.map { + GatewayApi.SimCard( + it.slotIndex, + it.simNumber, + it.phoneNumber.redact(), + it.carrierName.redact(), + it.iccid.redact(), + ) + } + } @@ - simCards.map { - GatewayApi.SimCard( - it.slotIndex, - it.simNumber, - it.phoneNumber.redact(), - it.carrierName.redact(), - it.iccid.redact(), - ) - } + mapSimCards(simCards) @@ - simCards.map { - GatewayApi.SimCard( - it.slotIndex, - it.simNumber, - it.phoneNumber.redact(), - it.carrierName.redact(), - it.iccid.redact(), - ) - } + mapSimCards(simCards)Also applies to: 188-196
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@app/src/main/java/me/capcom/smsgateway/modules/gateway/GatewayService.kt` around lines 135 - 143, Duplicate mapping of sim card entities to GatewayApi.SimCard needs consolidation: add a single helper (e.g., a private function mapSimCardsToApi(simCards: List<...>) or an extension List<...>.toGatewaySimCards()) that returns List<GatewayApi.SimCard> and performs the exact mapping and redact() calls (preserve slotIndex and simNumber, redact phoneNumber, carrierName, iccid). Replace the inline map blocks that construct GatewayApi.SimCard (the one using simCards.map { GatewayApi.SimCard(...) } in both the register and update code paths) with a call to this helper so both paths share the same mapping logic.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In `@app/src/main/java/me/capcom/smsgateway/modules/gateway/GatewayService.kt`:
- Around line 135-143: Duplicate mapping of sim card entities to
GatewayApi.SimCard needs consolidation: add a single helper (e.g., a private
function mapSimCardsToApi(simCards: List<...>) or an extension
List<...>.toGatewaySimCards()) that returns List<GatewayApi.SimCard> and
performs the exact mapping and redact() calls (preserve slotIndex and simNumber,
redact phoneNumber, carrierName, iccid). Replace the inline map blocks that
construct GatewayApi.SimCard (the one using simCards.map {
GatewayApi.SimCard(...) } in both the register and update code paths) with a
call to this helper so both paths share the same mapping logic.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: c6ae6ff6-ab7e-485a-9745-881669c68920
📒 Files selected for processing (8)
app/src/main/java/me/capcom/smsgateway/helpers/SubscriptionsHelper.ktapp/src/main/java/me/capcom/smsgateway/modules/gateway/GatewayApi.ktapp/src/main/java/me/capcom/smsgateway/modules/gateway/GatewayService.ktapp/src/main/java/me/capcom/smsgateway/modules/gateway/workers/RegistrationWorker.ktapp/src/main/java/me/capcom/smsgateway/modules/localserver/WebService.ktapp/src/main/java/me/capcom/smsgateway/modules/localserver/domain/Device.ktapp/src/main/java/me/capcom/smsgateway/modules/localserver/domain/SimCard.ktapp/src/main/java/me/capcom/smsgateway/ui/HomeFragment.kt
capcom6
force-pushed
the
feature/sim-cards-list
branch
from
ad096ff to
55f90ac
Compare
capcom6
force-pushed
the
feature/sim-cards-list
branch
from
55f90ac to
a1e0e51
Compare
capcom6
force-pushed
the
feature/sim-cards-list
branch
from
a1e0e51 to
ac26b79
Compare
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.
Summary by CodeRabbit
New Features
Privacy