Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Feature: Double opt-in #40

Open
wants to merge 46 commits into
base: master
Choose a base branch
from
Open

Conversation

unicode-it
Copy link

!! Warning !! By enabling this feature without garantueeing that field is_confirmed is True of the old contacts, the ones who are older than a week will be deleted when campaign saving is triggered!

This PR extends Birdsong by a double opt-in feature necessary in the EU according to it's GPDR.
The idea is:

  • user signs up for newsletter
  • user receives a confirmation e-mail with a link
  • user clicks on the link and is confirmed
  • if not confirmed after a week, the contact is deleted
  • campaigns are only sent to confirmed contacts

Therefore the model was extended by user creation date, confirmation date and confirmation. The deletion of unconfirmed contacts older than a week is triggered by a signal, whenever the save method of campaign is called.
Additionally, Birdsong settings were added to the admin menu settings. There the double opt-in feature can be enabled and the content of the confirmation e-mail can be set.

Furthermore, redirect pages can be specified in those settings. Redirect after

  • #
  • confirmation
  • unsubscription
    so the wagtail admin can create the pages according to their own wishes. If they are not set, default templates are used.

@unicode-it unicode-it changed the title Pr branch Feature: Double opt-in Mar 28, 2023
@david-smejkal
Copy link
Contributor

Just a heads up. Some of the code in this feature clashes with functionality proposed in #36

  • New fields (birdsong_contact.is_confirmed and birdsong_contact.created_at) added in this feature are duplicating two new fields (birdsong_contact.is_active and birdsong_contact.created_at) added in Fixes #35: Implemented Frontend Subscribe Form #36
    • Making this feature work with is_active field could also "solve" the warning since is_active is grandfathered in for all existing contacts. It depends on how you look at it but I think it's safer to avoid deleting existing contacts.
  • #View() is a duplication of subscribe()
    • there are many different bits and pieces that make up this larger (sign-up/subscribe) block of functionality that we can perhaps discuss in more detail if you want
    • for example: send_confirmation() could be handled by send_mail() which is more generic
  • I like the concept of having some admin driven BirdsongSettings. Albeit, it would have been more convenient if that came in as a separate new feature proposal.

Let's work together and make these two features compatible to make reviewing easier for @seb-b

@unicode-it
Copy link
Author

@david-smejkal Thank you for your suggestions!

@seb-b Before we go any further it would be useful for us to know if you are interested in the feature? Please let us know.

@seb-b
Copy link
Member

seb-b commented Jan 7, 2024

Apologies for the delay.

The idea of double opt-in makes sense but this does suffer from a bit of the same problems as #36. I think we could cut a lot of the features in this PR and it'd be mergeable.

As a general rule of thumb this project is intended for dealing with campaigns and contacts only. We're not really interested in adding features that would be added to a website (e.g. subscribe forms/views/blocks), that should be entirely left up to the implementer since the contact model can be customised and we don't want to make any assumptions about how the site has been developed (e.g. which template engine/styling used/is it headless?). There are bits of this PR that do make sense though like a confirmation url since that will be embedded in the emails, the birdsong settings section and the default contact model changes.

# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants