Problem/Motivation

When there's only hidden subscriptions, and no "regular" ones, the registration form processing doesn't process the hidden subscriptions.

Steps to reproduce

- Add one or multiple newsletters
- Make sure there are no newsletters with the options "Default on" or "Default off"
- Make sure there is at least one newsletter with the option "Silent"
- Register a new user
- See there is no Subscriber entity created and the settings aren't followed

Proposed resolution

- Update simplenews_user_profile_form_submit to ensure subscriptions and hidden are both processed.

Issue fork simplenews-3502221

Command icon Show commands

Start within a Git clone of the project using the version control instructions.

Or, if you do not have SSH keys set up on git.drupalcode.org:

About issue forks

Comments

daften created an issue. See original summary.

daften changed the visibility of the branch 3502221-hidden-subscription-on-register to hidden.

daften opened merge request !81

daften’s picture

daften commented
Assigned: daften » Unassigned
Status: Active » Needs review

Created an MR

I think #1689930: Silent and default subscription shouldn't depend on the registration form might be a better long-term fix, at least the approach to not rely on registration seems more sound to me, but that's a completely different approach.

adamps’s picture

adamps commented
Status: Needs review » Needs work

Thanks - one comment please.

daften’s picture

daften commented
Status: Needs work » Needs review

I processed with what I think was meant, it wasn't 100% clear to me, but removing the $subscriptions check was a logical thing to do and a stupid oversight. Thanks!

erwangel’s picture

erwangel commented

Warning: Undefined variable $subscriptions in simplenews_user_profile_form_submit() lines 429 and 436 of simplenews.module. This is because the patch removes the $subscriptions initialization.

From the patch diff:

  // Process subscription check boxes.
  - $subscriptions = array_filter($form_state->getValue('subscriptions', []));
  - if ($subscriptions) {
  // Process subscription check boxes and hidden subscriptions.
 +$hidden = $form_state->getValue('simplenews_hidden');
 + if ($subscriptions || $hidden) {

So, I think you have to set back the $subscriptions definition line
$subscriptions = array_filter($form_state->getValue('subscriptions', []));

adamps’s picture

adamps commented
Status: Needs review » Needs work

#7 seems correct

loze made their first commit to this issue’s fork.

loze’s picture

loze commented
Version: 4.1.0 » 4.x-dev
Status: Needs work » Needs review

Rebased onto 4.x and restored the $subscriptions line per #7.

Added SimplenewsSilentRegistrationTest, covering the case that triggers this: every newsletter set to new_account: silent, so no subscription checkboxes render at all. SimplenewsAdministrationTest always has an "on" newsletter alongside the silent ones, so a ticked checkbox carries the hidden ones through and the bug never surfaces.

Verified failing before the fix and passing after.

  • adamps committed b88d0412 on 4.x authored by daften 
    fix: #3502221 When there's only silent/hidden newsletters, default...
adamps’s picture

adamps commented
Status: Needs review » Fixed

Thanks

Now that this issue is closed, review the contribution record.

As a contributor, attribute any organization that helped you, or if you volunteered your own time.

Maintainers, credit people who helped resolve this issue.

Status: Fixed » Closed (fixed)

Automatically closed - issue fixed for 2 weeks with no activity.