Contrib.social

Model for confirmations is flawed

Open on Drupal.org →
Created on 11 January 2023, over 1 year ago
Updated 18 July 2023, 12 months ago

Problem/Motivation

See 🐛 Anonymous user can alter fields of any subscriber Fixed and 🐛 Major confusion for subscriptions during user registration Fixed .

Both bugs come from a common problem: the need to store unconfirmed subscription information whilst waiting for the subscriber to confirm. The unconfirmed data must not overwrite the live data. The Subscriber entity already has two mechanisms that attempt to address this:

  • The changes field stores unconfirmed subscribe requests.
  • The subscriptions field includes a status with 3 states, one of which is SIMPLENEWS_SUBSCRIPTION_STATUS_UNCONFIRMED.

Both are flawed for the same reasons: they store only the subscriptions, and not the supplemental subscriber field data; they can only store a single pending change; there is no unique identifier to correlate data with the user event that triggered it.

Proposed resolution

Summary:

  1. Store each unconfirmed subscription in a separate subscriber object
  2. Add a new field to indicate the unconfirmed status and remove SIMPLENEWS_SUBSCRIPTION_STATUS_UNCONFIRMED
  3. Remove Subscriber::getChanges() and Subscriber::setChanges()
  4. When the subscription is confirmed, search for any existing subscription and merge them together
  5. These changes are not back-compatible so require a new 4.x branch.
  6. Also see all the "changes" sections below.

Details:

  1. In SubscriptionManager, remove sendConfirmations(), destruct(), addConfirmation(), requiresConfirmation($uid).
  2. In ConfirmationController::confirmCombined(), delete the block relating to no changes available. Remove the changes from the hash, and same in simplenews_tokens(). In the ‘else’ block around line 103, call $subscriber->setStatus(SubscriberInterface::ACTIVE) and same in ConfirmMultiForm::submitForm().
  3. In SubscriptionsBlockForm ::submitExtra(), if the subscriber isn’t confirmed then call sendConfirmation(). Remove the call to subscription manager, instead save directly to the subscriber.
  4. Modify the unique field constraint on mail to use a new custom constraint that requires a unique value only for confirmed subscribers. Add constraint for uid whilst we are changing things.

Remaining tasks

User interface changes

1) The “Subscribers” list page (/admin/people/simplenews) changes to match the new data model.

  • Rename "Active" column to "Status", with 3 values: ACTIVE, BLOCKED, UNCONFIRMED. Remove the “Unsubscribed” value from the “Subscriber Status” filter, and add to the "Status" filter.
  • Unconfirmed subscriptions are now listed in their own row (instead of being part of the row for the confirmed subscriber).

2) Remove combined_body_unchanged from simplenews.settings.

API changes

  • Add Subscriber::sendConfirmation().
  • Subscriber::setStatus() and Subscriber::getStatus() now use an integer with 3 possible values: ACTIVE, BLOCKED, UNCONFIRMED (instead of boolean). Add shortcut functions isActive() and isConfirmed().
  • Remove Subscriber::getChanges()/setChanges().
  • In Subscriber::subscribe(), deprecate the $status parameter (it must be set to NULL or SIMPLENEWS_SUBSCRIPTION_STATUS_SUBSCRIBED).
  • Add new parameter Subscriber::loadByMail($checkTrust = FALSE). If TRUE, then call skipConfirmation() and if FALSE force creating a new subscriber and set it to unconfirmed.
  • In Subscriber::loadByUid(), add a new parameter $confirmed = TRUE. if TRUE only return a confirmed subscriber.
  • In Subscriber::loadByMail(), add a new parameter $check_trust = FALSE. If TRUE and the current user is untrusted then force creating a new subscriber.
  • In SubscriptionManager::subscribe()/unsubscribe(), remove support for confirmation (the $confirm parameter must be set to FALSE). Instead the calling code needs to group changes together using a Subscriber object:
    • Subscriber::loadByUid()
    • Modify subscriptions
    • Subscriber::sendConfirmation()
  • In SubscriptionManager, remove sendConfirmations(), addConfirmation(), requiresConfirmation().

Data model changes

The Subscriber entity changes as follows:

  • Remove changes field.
  • Remove the status value SIMPLENEWS_SUBSCRIPTION_STATUS_UNCONFIRMED for the subscriptions field.
  • Change status field to a tiny int, with 3 values: ACTIVE, BLOCKED, UNCONFIRMED.
🐛 Bug report
Status

Fixed

Version

4.0

Component

Code

Created by

🇬🇧United Kingdom AdamPS

Live updates comments and jobs are added and updated live.
Sign in to follow issues

Comments & Activities

Not all content is available!

It's likely this issue predates Contrib.social: some issue and comment data are missing.

contrib.social Blog FAQ Discussions
Production build 0.69.0 2024