Page MenuHomePhabricator

[keyserver] Create an endpoint that can be used to create or update a public link
ClosedPublic

Authored by tomek on May 19 2023, 9:56 AM.
Tags
None
Referenced Files
Unknown Object (File)
Thu, Jun 20, 7:57 PM
Unknown Object (File)
Thu, Jun 20, 7:02 PM
Unknown Object (File)
Thu, Jun 20, 2:18 AM
Unknown Object (File)
Wed, Jun 19, 9:14 AM
Unknown Object (File)
Tue, Jun 18, 7:11 PM
Unknown Object (File)
Sat, Jun 15, 12:05 AM
Unknown Object (File)
Thu, Jun 13, 6:50 AM
Unknown Object (File)
Tue, Jun 11, 9:46 PM
Subscribers

Details

Summary

If a community doesn't have a public invite link - it will be created. If it has one, it will be updated.

Depends on D7762

Test Plan

Create a new community, create a link for it using manage public link screen (later in the stack) - a link should be created. Update it, using the same screen - it should be updated.

Diff Detail

Repository
rCOMM Comm
Lint
Lint Not Applicable
Unit
Tests Not Applicable

Event Timeline

kamil requested changes to this revision.May 22 2023, 2:36 AM
kamil added inline comments.
keyserver/src/creators/invite-link-creator.js
23–25 ↗(On Diff #26681)

Can this be done via validatior?
(I think tcomb supports regular expressions in built-in validators)

68 ↗(On Diff #26681)

invalid_parameters will always be the case here? I feel like it's more affectedRows = 0 case only.

86 ↗(On Diff #26681)

How about expiration_time, limit_of_uses columns?
this will probably work that way but asking as those do not have DEFAULT clause

91 ↗(On Diff #26681)

I am a bit confused about naming, you are using phrase public and still setting primary = 1.
How is the "public invite link" related to the "primary link", are those the same or different (and if different is the logic here correct)?

This revision now requires changes to proceed.May 22 2023, 2:36 AM
keyserver/src/creators/invite-link-creator.js
23–25 ↗(On Diff #26681)

Makes sense!

68 ↗(On Diff #26681)

In case of an exception we can't talk about affected rows. We can check what is inside the exception and based on that conclude that it is because a unique constraint was violated - we can do that as a part of https://linear.app/comm/issue/ENG-3916/better-error-handling.

86 ↗(On Diff #26681)

They are nullable, so don't need the default value.

91 ↗(On Diff #26681)

Initially, during the design syncs we were using mostly term primary. And the implementation started with that in mind. In the meantime, the copy on UI contains term public. So indeed, these are the same. And public is more recent.

So agree, it should be clarified by replacing primary with public. But that would require significant amount of work - created https://linear.app/comm/issue/ENG-3921/rename-all-the-usages-of-primary-to-public-links to track that.

keyserver/src/creators/invite-link-creator.js
23–25 ↗(On Diff #26681)

I've implemented it and it would require some changes in terms of error handling. Currently, for invalid text, the error message is data isn't of type T - which doesn't make too much sense. So updating this to be a validator should happen as a part of https://linear.app/comm/issue/ENG-3916/better-error-handling.

kamil added inline comments.
keyserver/src/creators/invite-link-creator.js
91 ↗(On Diff #26681)

Thanks for the explanation, makes sense to me

This revision is now accepted and ready to land.May 23 2023, 4:14 PM