[keyserver] Create an endpoint that allows disabling links
ClosedPublic
Actions

Authored by tomek on Mon, May 22, 6:25 AM.

Details

Reviewers

Commits

Summary

At this point calling the endpoint results in link being deleted. In the future we can consider having a list of disabled links.

Depends on D7885

Test Plan

Tested in combination with the rest of the stack. Confirming disabling a link results in link being deleted.

Repository

rCOMM Comm

Lint

Lint Not Applicable

Unit

Tests Not Applicable

tomek requested review of this revision.Mon, May 22, 6:42 AM

kamil requested changes to this revision.Mon, May 22, 10:24 AM

kamil added inline comments.

keyserver/src/deleters/link-deleters.js
14 ↗	(On Diff #26773)
24–27 ↗	(On Diff #26773)	shouldn't we throw `ServerError('invalid_parameters');` when no rows was deleted? or maybe another solution will be returning deleted invite link to inform the client about the result?

This revision now requires changes to proceed.Mon, May 22, 10:24 AM

keyserver/src/responders/link-responders.js
93 ↗	(On Diff #26773)	Should this be `mixed` now?

keyserver/src/deleters/link-deleters.js
24–27 ↗	(On Diff #26773)	Why should we throw? It is generally a good idea to have more idempotent API: in this case, the intention of a user was for a link to not exist. If that's the case, then great - it doesn't matter if a link wasn't existing before. A possible scenario where it could happen is when two users try to disable a link at the same time - it's ok that one of the request will finish earlier. A different corner case is when a link was renamed by one user. Then it's hard to tell how we should handle that. There are a couple of solutions. But that would require some analysis (probably not worth it in the near future).

Specify return type and use mixed as input type

keyserver/src/deleters/link-deleters.js
14 ↗	(On Diff #26773)	Don't think that it matters - otherwise Flow would complain as this function is exported.
keyserver/src/responders/link-responders.js
93 ↗	(On Diff #26773)	Yes, we should. Created D7944 where the other endpoint from this file is also updated.

kamil added inline comments.

keyserver/src/deleters/link-deleters.js
24–27 ↗	(On Diff #26773)	Your explanation makes sense. My intention was to point out that when a user calls this endpoint and nothing changes we probably shouldn't allow the user to call it (as this is something unexpected), but from the scenario you brought it looks like a regular situation, sorry for the confusion