Skip to content
This repository has been archived by the owner on Feb 8, 2018. It is now read-only.

Add support for queuing emails without a participant linked #4547

Closed
rohitpaulk opened this issue Jul 15, 2017 · 7 comments
Closed

Add support for queuing emails without a participant linked #4547

rohitpaulk opened this issue Jul 15, 2017 · 7 comments

Comments

@rohitpaulk
Copy link
Contributor

rohitpaulk commented Jul 15, 2017

This is a requirement for sign-up via email.

Our email_queue currently only supports queuing emails linked to a participant. Extend this to support queuing emails with only an email address.

If a linked participant is present, stick to throttling based on participant_id. If only email_address is provided, throttle based on email_address.

@rohitpaulk rohitpaulk changed the title Add support for queueing emails without a participant linked Add support for queuing emails without a participant linked Jul 15, 2017
@chadwhitacre
Copy link
Contributor

How about a stub participant like we do for accounts elsewhere?

@rohitpaulk
Copy link
Contributor Author

rohitpaulk commented Jul 18, 2017

Hmm, that could work. I'm on the edge here though, there are pros and cons to this approach.

Upside

Schema + code is simplified in some places - we don't end up implementing the stuff that is common, can reuse some features.

Downside

We'll need special handling in a lot of places to differentiate claimed participants from unclaimed participants. It seems easier for bugs to creep in if we're not explicitly stating that a participant record could be half-baked. For example, the changes we'll need to make if we go with this approach:

  • email_queue will now have the handle the case of an unclaimed participant differently. We should be careful to not link the random username anywhere
  • We'll need to create a random participant the first time, and then pick the same participant every time after so that throttling based on participant works as intended.

I see how this approach made sense under Gittip/Gratipay 1 - A lot of the core features related to users who hadn't signed up yet (pledging etc), and hence unclaimed participants were closer to the mainstream usage rather than being an exception. I'm not sure if we should continue with that pattern for auth, I do see benefits in being explicit about the inexistence of an actual user record to deal with.

This posts explains my thoughts better: http://rdingwall.com/2009/11/20/the-trouble-with-soft-delete/ ('soft create' is what we're dealing with)

@rohitpaulk
Copy link
Contributor Author

@whit537 (pinging so that this appears in your notifications) thoughts?

@chadwhitacre chadwhitacre mentioned this issue Sep 7, 2017
10 tasks
@chadwhitacre
Copy link
Contributor

From http://rdingwall.com/2009/11/20/the-trouble-with-soft-delete/:

Primary transactional tables should only contain data that is valid and active right now.

I think stub participants are valid and active. We are actively initiating package claims for them, no? And over on #4598 we want to start placing credit card holds for them as well.

@chadwhitacre
Copy link
Contributor

We want to send them email: #4548. That seems active.

@chadwhitacre
Copy link
Contributor

Honestly I'm still drawn to the idea of using elsewhere for email:

gratipay.com/sql/schema.sql

Lines 311 to 325 in 139dca6

CREATE TABLE elsewhere (
id integer NOT NULL,
platform text NOT NULL,
user_id text NOT NULL,
participant text NOT NULL,
user_name text,
display_name text,
email text,
avatar_url text,
is_team boolean DEFAULT false NOT NULL,
extra_info json,
token json,
connect_token text,
connect_expires timestamp with time zone
);

@chadwhitacre
Copy link
Contributor

Because then we get stub participants for free:

# Claiming
# ========
# An unclaimed Participant is a stub that's created when someone visits our
# page for an AccountElsewhere that's not been connected on Gratipay yet.
def resolve_unclaimed(self):
"""Given a username, return an URL path.
"""
rec = self.db.one("""
SELECT platform, user_name
FROM elsewhere
WHERE participant = %s
""", (self.username,))
return rec and '/on/%s/%s/' % (rec.platform, rec.user_name)

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants