Fix OAuth client authorizations delete request

mjamescompton commented 1 week ago

Summary

Close #7350

Changes

Move attachPromise inside dispatch

Testing

Steps

Open the console at /console/user-settings/authorizations Click on one of the clients and click Revoke authorization

Results

Regressions

...

Notes for Reviewers

https://github.com/user-attachments/assets/e9ddda7c-a475-4f6a-9d40-f581327ce425

Checklist

[ ] Scope: The referenced issue is addressed, there are no unrelated changes.
[ ] Compatibility: The changes are backwards compatible with existing API, storage, configuration and CLI, according to the compatibility commitments in README.md for the chosen target branch.
[ ] Documentation: Relevant documentation is added or updated.
[ ] Testing: The steps/process to test this feature are clearly explained including testing for regressions.
[ ] Infrastructure: If infrastructural changes (e.g., new RPC, configuration) are needed, a separate issue is created in the infrastructural repositories.
[ ] Changelog: Significant features, behavior changes, deprecations and fixes are added to CHANGELOG.md.
[ ] Commits: Commit messages follow guidelines in CONTRIBUTING.md, there are no fixup commits left.

KrishnaIyer commented 1 week ago

Nice. Is this behavior more generic across the Console? I've noticed it while deleting devices I think (don't have a screenshot for now).

ryaplots commented 1 week ago

@KrishnaIyer This particular issue only happened in this case. The device delete might be a different issue

KrishnaIyer commented 1 week ago

Isn't the promise logic the same? Basically do a DELETE and then a GET (list)?

mjamescompton commented 1 week ago

I search the repo for any other examples of attachPromise(dispatch and did not find any. Similar issues might be something else.

ryaplots commented 1 week ago

yes, but in this case the issue was that we were attaching the promise in the wrong place: attachPromise(dispatch(request)), when it should be dispatch(attachPromise(request)). The promise should be attached to the request and not the dispatch. I did a global search in the code base for places where we attach the promise in the wrong place and this was the only place.

ryaplots commented 1 week ago

@mjamescompton Please also the check the ci before merging

TheThingsNetwork / lorawan-stack