Skip to content
New issue

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

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

Already on GitHub? Sign in to your account

[RTM] allow selection of which interest groups should be mandatory #19

Merged
merged 1 commit into from
Jun 18, 2018

Conversation

fritzmg
Copy link
Contributor

@fritzmg fritzmg commented May 29, 2018

  • mailchimpMandatoryInterests now has an options_callback that loads the interest groups from the MailChimp list.
  • In order to ensure that the interest groups are loaded, I added 'submitOnChange' => true and 'includeBlankOption' => true to eval of mailchimpList.

Careful: this changes the usage of the mailchimpMandatoryInterests field. While previously it was a simple boolean, now it contains a blob of serialized groups that should be mandatory. Technically this would need to be released as a major version - though may be you can release it as a feature version, since mailchimpMandatoryInterests was only recently introduced...

@fritzmg
Copy link
Contributor Author

fritzmg commented May 29, 2018

If you think this is too much out of scope of this extension, any user could alternatively achieve something like that via hook, if implemented.

public function modifyMailChimpForm(\Haste\Form\Form $objForm, \Oneup\Contao\MailChimpBundle\Module\ModuleSubscribe $objModule)
{
    $objForm->getWidget('SALUTATION')->mandatory = true;
}

@bytehead bytehead self-requested a review June 4, 2018 13:15
@bytehead bytehead self-assigned this Jun 4, 2018
@bytehead
Copy link
Member

bytehead commented Jun 4, 2018

I think we should go for a feature release.

@@ -66,9 +68,11 @@
$GLOBALS['TL_DCA']['tl_module']['fields']['mailchimpMandatoryInterests'] = [
'label' => &$GLOBALS['TL_LANG']['tl_module']['mailchimpMandatoryInterests'],
'inputType' => 'checkbox',
'options_callback' => ['Oneup\Contao\MailChimpBundle\EventListener\DcaListener', 'onLoadInterests'],
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is it ok to use Oneup\Contao\MailChimpBundle\EventListener\DcaListener::class?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, that's the way to go, didn't think of that.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ping @fritzmg who's gonna change? you or me?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Me :) sorry, haven't had time yet

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

done

@bytehead bytehead changed the title allow selection of which interest groups should be mandatory [WIP] allow selection of which interest groups should be mandatory Jun 4, 2018
@fritzmg fritzmg force-pushed the interest-mandatory-selection branch from b951914 to b67a959 Compare June 18, 2018 07:38
@bytehead bytehead merged commit 6c2887e into 1up-lab:master Jun 18, 2018
@bytehead bytehead changed the title [WIP] allow selection of which interest groups should be mandatory [RTM] allow selection of which interest groups should be mandatory Jun 18, 2018
@fritzmg fritzmg deleted the interest-mandatory-selection branch June 18, 2018 08:01
@bytehead
Copy link
Member

Thank you! 4.3.0 is out.

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

Successfully merging this pull request may close these issues.

2 participants