fix: make unregister idempotent - #25
Conversation
Made-with: Cursor
WalkthroughThe pull request updates the unregister functionality to be idempotent. The API documentation is revised to explicitly note that DELETE /unregister returns 200 OK even when no matching device exists. The corresponding implementation changes remove error returns for "no rows deleted" scenarios in three unregistration methods. Changes
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
There was a problem hiding this comment.
🧹 Nitpick comments (1)
storage/database.go (1)
102-103: Add regression tests for no-row delete paths across all unregister variants.Given this semantic change, add tests that assert success when nothing matches for
UnregisterDevice,UnregisterDeviceByParty, andUnregisterDeviceByPartyAndToken.Also applies to: 123-124, 148-149
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@storage/database.go` around lines 102 - 103, Add regression tests that exercise the "no-row" delete paths for UnregisterDevice, UnregisterDeviceByParty, and UnregisterDeviceByPartyAndToken: for each function call it against a non-existent device/party/token (ensure DB pre-state has no matching row) and assert the call returns success (no error) and leaves DB unchanged; reuse existing test helpers/fixtures to create a clean DB or transaction, call UnregisterDevice/UnregisterDeviceByParty/UnregisterDeviceByPartyAndToken with values that won't match any row, and add assertions that error == nil and row counts remain the same to prevent regressions.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@storage/database.go`:
- Around line 102-103: Add regression tests that exercise the "no-row" delete
paths for UnregisterDevice, UnregisterDeviceByParty, and
UnregisterDeviceByPartyAndToken: for each function call it against a
non-existent device/party/token (ensure DB pre-state has no matching row) and
assert the call returns success (no error) and leaves DB unchanged; reuse
existing test helpers/fixtures to create a clean DB or transaction, call
UnregisterDevice/UnregisterDeviceByParty/UnregisterDeviceByPartyAndToken with
values that won't match any row, and add assertions that error == nil and row
counts remain the same to prevent regressions.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: ac9d665d-3f86-4668-afd3-96b917133a7c
📒 Files selected for processing (2)
README.mdstorage/database.go
Summary
200 OKin API).200 OKfor unregister in README.Summary by CodeRabbit
Bug Fixes
Documentation