[WIP] Listing Crate Owner Invitations

[sunjay]

DO NOT MERGE YET

This is the first deployable chunk of #924: Listing Invitations

I'm only going to be working on the backend stuff for each of these chunks. Anyone else is welcome to take on the frontend work. 😄 I will go through and finish as much of the backend in each deployable chunk as I can, one chunk at a time.

So far I've just got the migration for the table in which every invitation will be stored. I followed the examples of other migrations in order to create this. The trigger is based on the one we have for reserved crate names and ensures the integrity of our data by making sure you can't invite the same user to the same crate more than once.

I've manually tested this in postgres and it works very well. You can update the same row but you cannot have more than once pending invitation for the same user and crate. Multiple declined invitations are fine, but only a single pending one will validate.

@carols10cents Could you skim this over and let me know if it's alright? I don't want to take too much of your time. We'll do a full review once I've made more progress. I want to know if I'm on the right track so far since I've never added a migration for this codebase before.

To Do

• Migration for crate_owner_invitations table
• Make an API route that can list all of the currently logged in user's pending invitations (and perhaps the declined ones too, if we want to support "undecline"?)

[sunjay]

@carols10cents Should I add something to the trigger for checking if a user is already an owner of a crate? It would not be difficult to do so.

[carols10cents]

Hm what if instead of a trigger, we create a unique index on invited_user_id, invited_by, and crate_id?

Also could we make invited_by be invited_by_id to be more consistent? Or maybe inviter_id/invitee_id? Idk, they just feel not consistent right now to me.

I like the ON DELETE CASCADE so that if a crate is deleted, invitations to that crate will also be deleted-- what about the same on the two users, so that if either of the users are deleted, the invitation will also be deleted?

Have you run disesel migration run and diesel migration revert (or diesel migration redo) to make sure everything works as you'd expect?

[sunjay]

I'll answer your questions in reverse order.

Yup. The migration works as expected in diesel.

On delete cascade is actually required by the tests. All crate_id fields are required to do that and yes I believe the effect will be as you described. I think it makes sense to add it for the other fields too.

I tried to avoid using invitor and invitee because I know I personally have to lookup or remember the difference all the time so someone who isn't a native English speaker may have even more trouble. I figured the extra few characters were worth the explicitness.

I can add _id to the field. I agree that that's more consistent. Would you also like me to add _at to the created field? I saw that used elsewhere but I wasn't sure if it was by convention.

Finally, regarding the unique index, would that cover multiple users inviting to the same crate and multiple declined invitations being allowed but only a single pending invitation allowed? I think the conditions required are a bit more complicated than what an index allows for. I'm not an expert though so I'm open to being corrected. :)

Thanks for all your feedback! I look forward to more!

[carols10cents]

Finally, regarding the unique index, would that cover multiple users inviting to the same crate

Let's think through the cases! If the unique index is (person sending invite + person receiving invite + crate), then we can have:

(1, 2, 3): User 1 sends an invite to user 2 to collaborate on crate 3
(1, 4, 3): User 1 sends an invite to user 4 to collaborate on crate 3 (user 1 can send multiple invites to the same crate)
(4, 2, 5): User 4 sends an invite to user 2 to collaborate on crate 5 (user 2 can receive multiple invites to different crates)
(4, 2, 3): User 4 sends an invite to user 2 to collaborate on crate 3 (user 2 can receive multiple invites to the same crate)

So perhaps it should just be (person receiving invite + crate), since there's no reason to need multiple invitations to the same crate? So then it would be:

(2, 3): User 2 has an invitation to collaborate on crate 3
(4, 3): User 4 has an invitation to collaborate on crate 3 (there can be multiple invitations of different users to the same crate)
(2, 5): User 2 has an invitation to collaborate on crate 5 (there can be multiple invitations of the same user to different crates)

But in this case since user 2 already has an invitation to collaborate on crate 3, no one could create another invitation to the same crate. That seems right, wdyt? Can you think of any other cases? What cases were you handling with the trigger?

multiple declined invitations being allowed but only a single pending invitation allowed?

Reading this and this comment of yours and this comment of yours

I don't think you should be able to undecline. If you accidentally decline, the other person should have to add you again.

Regarding this, I think it's alright for now to just make it so you can send someone an invitation again after it is declined. We're not sending emails right now so there isn't actually much abuse that can happen. I think a cargo owner --block command would be a more robust solution.

I disagree, and I thought we said once we have emails we want to send emails for invitations. I think, to minimize harassment opportunities, once you decline an invitation, that person can't keep inviting you over and over again and bothering you. They could invite you to different crates, but they'd have to keep creating different crates, and we should also implement user blocking for that case, but I don't think we should force users to block other users in order to permanently decline a particular invitation.

I tried to avoid using invitor and invitee because I know I personally have to lookup or remember the difference all the time so someone who isn't a native English speaker may have even more trouble. I figured the extra few characters were worth the explicitness.

I can add _id to the field. I agree that that's more consistent. Would you also like me to add _at to the created field? I saw that used elsewhere but I wasn't sure if it was by convention.

Yup, consistency is good!

[isislovecruft]

Regarding this, I think it's alright for now to just make it so you can send someone an invitation again after it is declined. We're not sending emails right now so there isn't actually much abuse that can happen. I think a cargo owner --block command would be a more robust solution.

I disagree, and I thought we said once we have emails we want to send emails for invitations. I think, to minimize harassment opportunities, once you decline an invitation, that person can't keep inviting you over and over again and bothering you. They could invite you to different crates, but they'd have to keep creating different crates, and we should also implement user blocking for that case, but I don't think we should force users to block other users in order to permanently decline a particular invitation.

Hi, I tried to think a bit about the best and simplest state machine here that would prevent abuse, and I agree with @carols10cents that someone shouldn't be able to just keep adding you over and over once you've declined. However, this also raises the issue of: "What if my coworker invites me to something and I don't understand that it's a real thing and I mistake it for spam and decline?" Perhaps something like the following might satisfy the constraints (numbered states, state numbers they can transition to in parentheses, bullet points of actions they can take underneath):

1. OWNER (8)
   • INVITE_USER (2)
   • REMOVE_OWNER (8)
2. INVITE_PENDING (3,4,6)
   • [user] ACCEPT_INVITE (3)
   • [user] REJECT_INVITE (4)
   • [owner] CANCEL_INVITE (6)
3. INVITE_ACCEPTED (1)
4. INVITE_REJECTED (5,6)
   • [user] UNDO_REJECT_INVITE (5)
   • [owner] CANCEL_INVITE (6)
5. INVITE_REJECTED_UNDO (3→1)
6. INVITE_CANCELLED (8)
   • [owner] (no new invites allowed after this?)
7. USER_BLOCKED (?) ¹
8. NON_OWNER

¹ Might be a separate issue to the invite state machine? We should probably check this first, of course, i.e. "Is the user we're trying to invite blocking us? If so, bail."

@sunjay Are you planning to work more on this? I'm happy to help, but I don't want to step on anyone's toes.

(Also sorry for the cargo canoe)

[sunjay]

Hi @isislovecruft,
I like your suggestions. Let's see what @carols10cents says about them too. I am planning to work more on this. Hopefully I'll get a chance this weekend to finish this part off. See the original issue for the list of parts. Once this is done, someone else will need to work on the frontend aspect of this part. Then I'll start on the remaining parts. The frontend and any of the remaining parts are all open for your help. I appreciate you joining in on this. :)

Regarding your suggestions, if I understand what you're saying, you're suggesting that we have a way to undo rejected invites. Please correct me if I'm mistaken, but everything else in that flow is already planned. Owners will be able to invite and cancel their invitations. Invited users will be able to accept and reject invitations. The addition is that they won't be able to be reinvited until the undo their rejection.

I'm on board with all of this.

My question is about terminology. It sounds to me like the behavior of rejecting an invitation here is the same as the behavior I imagined that block would have. Maybe it would make sense to make the two options for handling an invitation: accept or block. Then having an unblock command would be natural. Unblock would do the same thing as "undo reject", just with more familiar terminology.

What do you all think of this?

[sunjay]

When I think about this stuff, I like to consider the things the user will actually see in addition to the high level flow. So for this feature I would imagine something like this:

Owner Scenarios

• Owner of coolcrate runs cargo owner --add sunjay
  Message is displayed:

  The user sunjay has been invited to become an owner of coolcrate. They will be able to publish, yank and invite other users to become owners of coolcrate.

  To accept your invitation, sunjay will need to go to the following URL and click "Accept":
  https://crates.io/awesome_invite_endpoint/sunjay/coolcrate/example_url

  If they select "Block" instead, they will need to unblock requests from this crate at that same URL before they are able to be re-added as an owner.

• Owner otherowneruser attempts to add a user that has not yet responded to the invitation with accept or block
  Message is displayed:

  The user sunjay has not yet responded to a previous request to become an owner sent by owneruser. sunjay can respond to this invitation at the following URL:
  https://crates.io/awesome_invite_endpoint/sunjay/coolcrate/example_url

  To cancel this invitation, run the following command:
  cargo owner --remove sunjay

• Owner attempts to add a user that has blocked invitations from that crate using cargo owner --add sunjay on coolcrate
  Message is displayed:

  The user sunjay has blocked any further requests from this crate. If this was done by mistake, sunjay can visit the following URL to unblock requests from this crate:
  https://crates.io/awesome_invite_endpoint/sunjay/coolcrate/example_url

  Once sunjay unblocks invitations, they will need to be added again by an owner of coolcrate in order to become an owner.

The text itself is not final at all. In each of these scenarios, the messages are very clear about what just happened and what the user can do. The owner can communicate with the person added about what they need to do with no confusion at all. When we add emailing, sending an invitation will also send an email with similar instructions aimed at the person being invited. They'll know that if they block, they can't be added again until they are unblocked. After the initial invitation, they won't receive any further emails unless they unblock and are invited again.

The invited user will do everything through the online interface. No commands to be run or anything so we can fully spell out what the implications of accepting or blocking are. We'll make sure there's a way to get to the invites from crates.io so no one needs to send a URL to finish the flow.

Note that this flow is only for individual users. For adding/removing teams, everything is exactly the same as it is now. (no invites or anything)

Looking at this when it's spelled out this way, do you have any feedback?

[carols10cents]

@isislovecruft This state machine looks like what I'd expect!

[owner] (no new invites allowed after this?)

Yeah, I think to prevent abuse of running add/cancel repeatedly in a loop, we should probably disallow further invites after that and say to email us and we'll mess with the database if that was a mistake. Should be pretty rare.

¹ Might be a separate issue to the invite state machine? We should probably check this first, of course, i.e. "Is the user we're trying to invite blocking us? If so, bail."

Yep! We've got a separate issue for implementing blocking.

@sunjay

My question is about terminology. It sounds to me like the behavior of rejecting an invitation here is the same as the behavior I imagined that block would have. Maybe it would make sense to make the two options for handling an invitation: accept or block. Then having an unblock command would be natural. Unblock would do the same thing as "undo reject", just with more familiar terminology.

I was thinking that blocking should be blocking another user, not blocking a crate, while declining an invitation should only apply to the crate. If @isislovecruft invites me to a canoe crate (;)), I might want to decline that invitation but not decline all future invitations from @isislovecruft. However, if a person is harassing another person and the harasser keeps creating new crates and inviting to new crates, we shouldn't make the person being harassed block each crate invitation-- they should be able to block the person doing the harassing and not get any more invites from the person. So I think declining an invitation to a crate and blocking a person are different. Does that make sense @sunjay ?

The invited user will do everything through the online interface. No commands to be run or anything so we can fully spell out what the implications of accepting or blocking are.

I don't see a reason to rule in or out a command line interface for this at this point. As stated in the issue, we need to provide a web interface for people using older versions of cargo, but there's no reason we couldn't add a command line interface for this in the future. The messages you've proposed would all be fine on the command line as well as in a web UI.

[sunjay]

I agree that being able to block a user is important, but I wouldn't rule out the idea of blocking a crate. If someone is the victim of brigading (a coordinated effort by a group of people), then it is very easy to abuse them with only a single bad crate and a lot of users.

I think the functionality of declining a request that we're describing here is the same as blocking an entire group of people (the crate) and that's why I suggested that name.

The invitation page could have three options: accept, block coolcrate, block otheruser.

We could probably release the feature with one type of blocking and then quickly follow that release with the other. As far as our data model or implementing this goes, this doesn't actually change anything. I'm just trying to get the language we're using right because I don't think "decline" or "reject" make sense if the person can't add you again without undoing that. You wouldn't expect someone to be unable to ever add you again on Facebook after you declined their request. You have to block them to do that. Given that we're not allowing another request after the first one unless you take a special action, I think the word "block" makes sense here. I'm open to your feedback if you think there's another better word.

but there's no reason we couldn't add a command line interface for this in the future

Sorry for being unclear. I didn't mean to rule out the command line in the future. I was just focusing on what we're planning for now. I agree with you that all of this works for both and I had that in mind when I was writing it. :)

[carols10cents]

I agree that being able to block a user is important, but I wouldn't rule out the idea of blocking a crate. If someone is the victim of brigading (a coordinated effort by a group of people), then it is very easy to abuse them with only a single bad crate and a lot of users.

Since the unique index I proposed keeps track of only the user being invited and the crate, not who is sending the invite, using multiple users to invite the same person to the same crate should already not be possible. Does that make sense?

FWIW, Github gives you the option to accept a collaboration invitation, decline the invitation, or block the user who sent the invitation (and blocking has other implications, such as that user can't comment on your stuff). Natalie and I just tested this out, if she declines an invitation of mine (but doesn't block me), I can invite her again. Which sounds like it's similar to what facebook does as well.

Perhaps we should just implement that behavior to be consistent with other services? That is, have the choices be accept invitation/decline invitation/block user, and just delete declined invitations so that they can be sent again? This would mean we'd have to make blocking users part of this feature.

[sunjay]

Perhaps we should just implement that behavior to be consistent with other services? That is, have the choices be accept invitation/decline invitation/block user, and just delete declined invitations so that they can be sent again? This would mean we'd have to make blocking users part of this feature.

I must not have been clear before, but this is what I was suggesting when I said we should allow people to invite again after an invitation has been declined. I think the consensus before was that this would leave too much room for abuse and so we should make it "accept or block" only.

If you think this direction is also fine, should we start to discuss that too? Am I misunderstanding anything?

[carols10cents]

I must not have been clear before, but this is what I was suggesting when I said we should allow people to invite again after an invitation has been declined. I think the consensus before was that this would leave too much room for abuse and so we should make it "accept or block" only.

If you think this direction is also fine, should we start to discuss that too? Am I misunderstanding anything?

I think my mind is changing slightly. I think we should do what github does-- accept invitation, decline invitation, or block user.

[sunjay]

If that's the case, we should rework the plan then in favor of that.

[carols10cents]

If that's the case, we should rework the plan then in favor of that.

Updated! #924

[carols10cents]

Hey @sunjay, thank you for digging into this and helping to flesh out the design a bit more! I want to move a bit faster on this though, so that we can get invitations up before there are copycat canoes :( So I've made #1040 that replaces this so that @natboehm can get started on the frontend part and the next pieces this week. Thank you!!

[sunjay]

@carols10cents No problem I totally understand. That canoe thing put a lot of spotlight on this issue. I'll help out with the next parts. :)