cyclestreets / cyclescape

Cyclescape - cycle campaign group toolkit
https://www.cyclescape.org/
MIT License
33 stars 15 forks source link

Invitations API, resend #950

Open davidearl opened 4 years ago

davidearl commented 4 years ago

Further to #683 , if someone doesn't respond to the original invitation email (whether sent out manually or via the API, though it's the API that I'm more interested in), a further attempt gets an error back from the API. The problem is that people constantly don't respond to invitations (despite asking for them) and/or lose the emails. We'd very much like to be able to send a further group invitation even if someone has already been sent one, and irrespective of whether they are already registered in Cyclescape (typically they won't have a password, though; but not necessarily - they may have signed up themselves and requested group membership within cyclescape, but failed to act on it previously).

nikolai-b commented 4 years ago

I don't want to mix adding a user as a group member and inviting them any more than we do already!

If we change the response from membership API to:

{
  "status": "Unprocessable Entity",
  "data": "membership already a exists"
}

if the membership exists then add a new resend invitation API where you could POST the email and if the user exists and you are a committee member of a group they were in then we'd resend an invitation email if (and only if) they did not have a password set. @mvl22 thoughts?

davidearl commented 4 years ago

I don’t mind particularly how it’s done, so long as people who don’t respond in time to the original email can be sent a retry.

But ideally I’d prefer not to have a different workflow in the two cases. How about separating out completely the three things that are going on here and I make three requests:

  1. Add new account (which might fail if they already have one) 2, add account (by email address or I’d returned from 1) to group (again might fail if already the case)
  2. Send invitation email (password set or reset, should only fail if account not identified correctly)

Then I can just make those three requests In order, without needing to know the current state, or having to worry whether they signed up manually previously but weren’t a group member, or whatever.

nikolai-b commented 3 years ago

So sending an invitation email when they have their password set feels overstepping committee responsibilities. Once the user has signed up then they can follow the "forgotten my password" flow if they need to reset, I don't think the committee needs to be involved in this. A re-invite makes sense if they have lost the original invitation email. I don't think you should be re-inviting people unless they are in a group where you are a committee member.

In your post above you suggested only failing if the email is not found but I would prefer it to fail if they have set their password or are not in a group in common. Is that acceptable? If not would you be able to explain more on why? Thanks

davidearl commented 3 years ago

Yes, if they've set their password already and have access to the group, then they are already in, and no more is needed. And, no I'm not trying to invite them to an unrelated group.

The problem at the moment is before people get in - they wouldn't even know where to go to get a password reset. The scenario is this:

  1. user joins group, from website form, and ticks box to say they want to join our "discussion forum" = cyclescape
  2. we process their application, and as part of that send them an invitation to Cyclescape. The CycleScape API allows us to automate this, and do everything in Cameo, the membership database (previously we had to log in to CS separately and re-enter details)
  3. they either don't receive the email, or lose it or overlook it. This happens amazingly often. People just don't attend to emails that need their attention.
  4. Months later they get a newsletter that has an item they are interested in that references a Cyclescape discussion, and they realise they never followed through, and ask us to be let in.
  5. What we'd really like to do at that point is just re-issue the invitation, but the CS API won't do this.

What I was then suggesting is splitting out the invitation sending API into three separate steps (API endpoint requests), each of which can fail independently of the others, so

I believe these are the steps the single API is currently doing (though I may be wrong). By separating them, at my end I can just do the three steps myself and ignore the failures from the first two (just the "already done" failures, obviously, not any other unrelated failures) when repeating it (as at step 5 above).

Equally, they may already have signed up for themselves but don't get to be in the groups, so again, I can automate this with the same operation, which fails on adding an account, but succeeds on the other two,

Obviously, we shouldn't be able to invite people to groups we're not related to. I would hope the API key restricts what I can do. Cameo is used by more than one group, so by adding their own CS credentials to Cameo, they are delegating their instance of Cameo (and only theirs) to act on their behalf in how the group in CS is managed (just in this trivial way of inviting new, but in principle could be other things too). If this isn't already the case, then there is already a problem, irrespective of repeat invitations, as I could invite and subscribe anyone to any group with the API, and that's clearly wrong, as you say.

sueedwards commented 3 years ago

hi, this is Sue Edwards, Camcycle membership secretary for the last few years. Thanks for looking at this. As David said, we quite often have Camcycle members who were sent an invitation to the Cyclescape camcycle group some time (possibly years) ago but didn't act on it at the time for one reason or another, and if they ask later to join the group that's a problem for us. I've found that sometimes removing the user from the cyclescape group, waiting a day or two and then adding them back in works, but not always, and it would be much easier if we could simply send the invitation email again. I can see quite a lot of camcycle group members with status invite pending in cyclescape.

We'd also like an API to remove someone from the cyclescape group so that we could do that automatically when their membership is marked as expired, should I create another issue for that?

mvl22 commented 2 years ago

I think the best overall solution is for the group to use the 'Potential membership list' functionality at:

/groups/2-camcycle/potential_members/new

There you submit a list of the current e-mail addresses of all members. We do not store the e-mails, only hashes. When a member a registers, that list is checked against. It also means auto-expiry happens for people who have left the organisation.

That should mean the only thing that needs to happen is to send out an invite to the site as a whole if they are not already a member of the site. When they register, the existence of a hash of the e-mail (that they just signed up with) on this list should kick in automatically.

Do you think that is a better architectural solution than lots of API calls and separate e-mails?

If we don't have an API to that page, we could add one.

PS The sign up process is radically changed in the new design - see the new wizard about half-way down. This should get rid of a lot of group addition usability failures. The HTMLification of the design work is due to complete any day now, and we'll then move on to the server-side implementation.

davidearl commented 2 years ago

The key thing is automation. There is no point in doing anything if we can't automate it.

But as it stands this doesn't really help much. Currently, we can do the necessary in one place, as a side-effect of enrolment, if they ticked the box on the membership form,

But to manage the list you are suggesting is nightmarish - we'd have to generate a full list of everyone eligible in every place where membership status can change and upload it. That's logisitically hard and open to error as we have to remember to do this when new code is added that might change status; and it is also slow - we effectively have to do a whole database dump and upload for a minor change that most often isn't relevant anyway, when previously the change was entirely localised, but I have no way of telling. I really don't want to put hooks for cyclescape scattered across the whole of Cameo.

If the API for this could APPEND to the list rather than uploading it all, that would be a step forward. If we don't remove addresses when people get deleted (which is probably not a big deal, given we don't remove people automatically from the group if they did sign up to cyclescape), then we could use it similarly to how we do it now.

I don't think it reduces complexity though. It's essentially the same from my POV, though it may resolve the particular problem. However, it does introduce a new problem, in that currently we maintain the link between email addresses, but associating it after they sign up would lead to people providing different email addresses (which people do remarkably often) in the two places, and the two not then matching.

So, no, I'm not convinced, sorry.