Add swap service - #173
Add swap service#173watersRand wants to merge 6 commits into
Conversation
RafaelJohn9
left a comment
There was a problem hiding this comment.
Hello @watersRand , thanks for the awesome PR, a few changes to consider 🤝
There was a problem hiding this comment.
Apologies for the late review. you can always tag me for a review, no worries 🤝 ,
Thanks for the changes and we have a few things to do before we become ready to merge.
- Remove the
environmentparameter across the Swap module since this is only used insidehttp_clientmodule. - We then update the stubs correctly.
- Then we wire in this service to client and async client see (https://github.com/Byte-Barn/mpesakit/blob/master/mpesakit/mpesa_client.py)
Then we will be good to go 👍
|
|
||
| http_client: HttpClient | ||
| token_manager: TokenManager | ||
| environment: Literal["sandbox", "production"] = "sandbox" |
There was a problem hiding this comment.
We can deprecate the environment parameter that we have across the different swap files, since this is handled by our http client file.
RafaelJohn9
left a comment
There was a problem hiding this comment.
Thanks for addressing the earlier feedback — the lastSwapDate doc note, using the shared HTTP client, and dropping environment from Swap/AsyncSwap all look good.
One loose end: environment was removed from Swap/AsyncSwap but is still threaded through the SwapService/AsyncSwapService facades, the docs examples, and the E2E tests. Since Swap/AsyncSwap are Pydantic models without extra="forbid", passing environment=... to them is silently dropped rather than erroring, so this isn't caught by tests — but it's dead/misleading code. Left inline comments on where this needs cleanup.
| self._swap = Swap( | ||
| http_client=self.http_client, | ||
| token_manager=self.token_manager, | ||
| environment=self.environment, |
There was a problem hiding this comment.
SwapService still takes an environment param (default "sandbox") and forwards it here to Swap(...), but Swap no longer has an environment field — it's silently dropped by Pydantic. mpesa_client.py doesn't even pass environment when constructing SwapService, so this is dead state at every call site. Please remove environment from this facade too, matching the cleanup already done on Swap/AsyncSwap.
| self._swap = AsyncSwap( | ||
| http_client=self.http_client, | ||
| token_manager=self.token_manager, | ||
| environment=self.environment, |
There was a problem hiding this comment.
Same issue as SwapService above — environment is forwarded to AsyncSwap(...), which no longer accepts it.
| swap_service = Swap( | ||
| http_client=http_client, | ||
| token_manager=token_mgr, | ||
| environment=env, |
There was a problem hiding this comment.
This example still passes environment=env to Swap(...), which no longer has that parameter (it's now a no-op field that gets dropped). Please update the example to match the current constructor signature (http_client + token_manager only).
| swap_service = AsyncSwap( | ||
| http_client=http_client, | ||
| token_manager=token_mgr, | ||
| environment=env, |
There was a problem hiding this comment.
Same stale environment=env kwarg here for AsyncSwap(...).
| return Swap( | ||
| http_client=http_client, | ||
| token_manager=token_manager, | ||
| environment=env, |
There was a problem hiding this comment.
This fixture still passes environment=env into Swap(...), which no longer accepts it — worth dropping so the test reflects the real constructor.
| return AsyncSwap( | ||
| http_client=http_client, | ||
| token_manager=token_manager, | ||
| environment=env, |
There was a problem hiding this comment.
Same stale environment=env kwarg for AsyncSwap(...) here.
There was a problem hiding this comment.
@RafaelJohn9 Thanks for the thorough review. I didn't realize I had missed these
Description
This PR introduces the SIM Swap Detection Service (
/imsi/v2/checkATI) to support checking when a subscriber's SIM card was last swapped. This helps detect and mitigate account takeover and fraud risks prior to completing high-value transactions.Key changes included:
SwapServiceandAsyncSwapServicewithswap_query(customer_number)andswap_request(request)methods.SwapRequest(with MSISDN auto-normalization) andSwapResponsePydantic models.is_successfulandis_recently_swappedproperties toSwapResponseto handle Safaricom's sentinel non-swapped date (01-01-1900)..pyitype stubs, updateddocs/swap.md, and updated landing page status/feature data.Fixes #160
Type of Change
How Has This Been Tested?
SwapRequestphone number auto-normalization,SwapResponse.is_recently_swappedsentinel date evaluation (01-01-1900vs active timestamp), and sync/async service call execution.ruff checkand type assertions with.pyistub files to verify clean linting across the package.Checklist
Screenshots (if applicable)
N/A (Backend API addition & documentation update)
Additional Context
checkATI) is a paid API product on Safaricom Daraja. Documentation and docstrings reflect onboarding guidance for sandbox and production.