-
Notifications
You must be signed in to change notification settings - Fork 303
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Fetch device from IS for Unclaim and GetClaimStatus RPCs #6325
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We should remove that allow_delete_body
from the proto generator as well.
5154d5c
to
b436979
Compare
7106551
to
de8fcb2
Compare
The console part is a "best effort". Let me know what else needs to be changed. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
API and DCS LGTM
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Codeowned LGTM.
Mildly related: #6333, but now we don't use any parameters here (other than route parameters) so it should be fine either way.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
As discussed offline; if the device does not exist, DCS should just proceed attempting to unclaim. |
de8fcb2
to
8ceab0d
Compare
I'll close this PR but keep the branch. I'll adapt some of these changes after #6381 is merged. |
Summary
References; https://github.com/TheThingsIndustries/product-management/issues/6
Changes
Unclaim
. We are doing this since the support for a request body for HTTPDelete
requests is unclear.Unclaim
andGetClaimStatus
.Testing
DCS locally tested. For the console part, I think if the Cypress tests pass, I think this is good.
Regressions
Will be tested on
staging1
since there's currently no support for local TTJS testing.Notes for Reviewers
unclaim
always needed to include the App and Device ID. It's just that we no longer accept a body.Checklist
README.md
for the chosen target branch.CHANGELOG.md
.CONTRIBUTING.md
, there are no fixup commits left.