feat: make marketing email opt-in checkbox selectively ignorable by iloveagent57 · Pull Request #38042 · openedx/openedx-platform

iloveagent57 · 2026-02-23T16:12:23Z

ported from #38022

We want to support a flow for SSO-enabled Enterprise customers who have agreed off-platform that none of their learners will opt-in to marketing emails or sharing research data. This change proposes to do so by adding an optional field that, when enabled, disables the presence of the two checkboxes on this registration form and sets their values to false.

Local Testing

See the added doc on how to test locally. Do one pass with the new model option disabled and private.py settings that have

ENABLE_THIRD_PARTY_AUTH = True
MARKETING_EMAILS_OPT_IN = True

Visit http://localhost:18000/auth/idp_redirect/saml-default, be redirected to mocksaml.com, then click "Sign In".

Now change your provider config to set "Skip registration optional checkboxes" to true.
Visit http://localhost:18000/auth/idp_redirect/saml-default again, be redirected to mocksaml.com, then click "Sign In".

openedx-webhooks · 2026-02-23T16:12:29Z

Thanks for the pull request, @iloveagent57!

This repository is currently maintained by @openedx/wg-maintenance-openedx-platform.

Once you've gone through the following steps feel free to tag them in a comment and let them know that your changes are ready for engineering review.

🔘 Get product approval

If you haven't already, check this list to see if your contribution needs to go through the product review process.

If it does, you'll need to submit a product proposal for your contribution, and have it reviewed by the Product Working Group.
- This process (including the steps you'll need to take) is documented here.
If it doesn't, simply proceed with the next step.

🔘 Provide context

To help your reviewers and other members of the community understand the purpose and larger context of your changes, feel free to add as much of the following information to the PR description as you can:

Dependencies

This PR must be merged before / after / at the same time as ...
Blockers

This PR is waiting for OEP-1234 to be accepted.
Timeline information

This PR must be merged by XX date because ...
Partner information

This is for a course on edx.org.
Supporting documentation
Relevant Open edX discussion forum threads

🔘 Get a green build

If one or more checks are failing, continue working on your changes until this is no longer the case and your build turns green.

Details

Where can I find more information?

If you'd like to get more details on all aspects of the review process for open source pull requests (OSPRs), check out the following resources:

When can I expect my changes to be merged?

Our goal is to get community contributions seen and reviewed as efficiently as possible.

However, the amount of time that it takes to review and merge a PR can vary significantly based on factors such as:

The size and impact of the changes that it introduces
The need for product review
Maintenance status of the parent repository

💡 As a result it may take up to several weeks or months to complete a review and merge your PR.

kdmccormick

Is there any supporting information you can link to?

kdmccormick · 2026-02-23T20:53:04Z

openedx/core/djangoapps/user_authn/views/registration_form.py

+                                "SAML provider config not found for idp_name: %s",
+                                saml_provider_name
+                            )
+                except (ImportError, Exception) as e:  # pylint: disable=broad-except


Does this except need to be so broad? As it's written it'll swallow even things like a misspelled variable name (NameError).

At the very least please use log.exception so that the stack trace is shown in the logs.

kdmccormick · 2026-02-23T20:53:17Z

openedx/core/djangoapps/user_authn/views/registration_form.py

+                                            log.debug(
+                                                "SAML provider config not found for idp_name: %s",
+                                                saml_provider_name
+                                            )


Same comment

kdmccormick · 2026-02-23T20:53:32Z

openedx/core/djangoapps/user_authn/views/registration_form.py

+                                            try:
+                                                saml_config = SAMLProviderConfig.objects.current_set().get(
+                                                    slug=saml_provider_name
+                                                )


Same comment

kdmccormick · 2026-02-23T20:54:52Z

common/djangoapps/third_party_auth/models.py

+            "If enabled, the marketing emails opt-in checkbox will be optional for users "
+            "registering via this provider instead of required. When disabled, marketing email opt-in "
+            "is determined by the global MARKETING_EMAILS_OPT_IN setting."


I'm having trouble understanding this. What does it mean for an opt-in to be optional?

I'd prefer a field name more like "marketing_emails_opt_in_disabled". To say the "opt-in" is "optional" is illogical because an opt-in is already optional by definition.

This help text can be waay more clear about that:

Suggested change

"If enabled, the marketing emails opt-in checkbox will be optional for users "

"registering via this provider instead of required. When disabled, marketing email opt-in "

"is determined by the global MARKETING_EMAILS_OPT_IN setting."

"If True, the marketing emails opt-in checkbox will be hidden for users "

"registering via this provider, and marketing emails are forced OFF. "

"If False, the checkbox remains visible and the checked state defaults "

"to the global MARKETING_EMAILS_OPT_IN setting."

iloveagent57 · 2026-02-23T21:26:10Z

@kdmccormick Sorry for posting with empty description, I just updated it with what this code intends to support. good call on those exception handlers, we can clean that up.

pwnage101 · 2026-02-25T19:35:38Z

openedx/core/djangoapps/user_authn/views/registration_form.py

+                                    field_default = field_overrides[field_name]
+                                    if field_name == 'marketing_emails_opt_in':
+                                        field_default = False
+                                        log.info(
+                                            "Hiding marketing_emails_opt_in field and setting default to False"
+                                        )


Was this just a bug before any of these changes? This logic is insensitive to the value of skip_registration_optional_checkboxes. Seems unrelated to this PR and curious if it is an unintentional AI bugfix which we might consider for a separate PR.

pwnage101 · 2026-02-25T19:37:37Z

openedx/core/djangoapps/user_authn/views/registration_form.py

+        # Check if SAML provider wants to skip optional checkboxes
+        saml_config = self._get_saml_provider_config()
+        if saml_config and saml_config.skip_registration_optional_checkboxes:
+            log.info(
+                "SAML provider %s has skip_registration_optional_checkboxes=True, "
+                "setting default to False and required to False",
+                saml_config.slug
+            )
+            default_value = False  # When skipped, user opts out by default
+            field_required = False  # Make field optional
+
+            # Set field override to ensure our SAML-specific values are used
+            # This will override any values set by the TPA provider
+            log.info(
+                "Setting field override for marketing_emails_opt_in to "
+                "defaultValue=False, required=False"
+            )
+            # pylint: disable=protected-access
+            form_desc._field_overrides['marketing_emails_opt_in'] = {
+                'defaultValue': False,
+                'required': False
+            }
+


Have to ask, but is this entire section necessary? It seems like all the changes to _apply_third_party_auth_overrides() adequately override the default/required/hidden attributes of the fields we care about.

…ignorable We want to support a flow for SSO-enabled Enterprise customers who have agreed off-platform that none of their learners will opt-in to marketing emails or sharing research data. This change proposes to do so by adding an optional field that, when enabled, disables the presence of the two checkboxes on this registration form and sets their values to false. ENT-11401

iloveagent57 requested a review from a team as a code owner February 23, 2026 16:12

openedx-webhooks added open-source-contribution PR author is not from Axim or 2U core contributor PR author is a Core Contributor (who may or may not have write access to this repo). labels Feb 23, 2026

openedx-webhooks added this to Contributions Feb 23, 2026

github-project-automation bot moved this to Needs Triage in Contributions Feb 23, 2026

iloveagent57 force-pushed the aed/optional-marketing-checkbox branch 4 times, most recently from b36bac4 to d2bee6d Compare February 23, 2026 18:56

macdiesel approved these changes Feb 23, 2026

View reviewed changes

kdmccormick reviewed Feb 23, 2026

View reviewed changes

iloveagent57 force-pushed the aed/optional-marketing-checkbox branch 3 times, most recently from b719d8e to e2044a6 Compare February 24, 2026 13:45

mphilbrick211 moved this from Needs Triage to In Eng Review in Contributions Feb 24, 2026

iloveagent57 force-pushed the aed/optional-marketing-checkbox branch from e2044a6 to 316fcd5 Compare February 24, 2026 21:50

iloveagent57 changed the title ~~feat: make marketing_emails_opt_in optional~~ feat: make marketing email opt-in checkbox selectively ignorable Feb 24, 2026

iloveagent57 force-pushed the aed/optional-marketing-checkbox branch from 316fcd5 to db6b4f5 Compare February 25, 2026 19:04

pwnage101 reviewed Feb 25, 2026

View reviewed changes

iloveagent57 force-pushed the aed/optional-marketing-checkbox branch 4 times, most recently from 61eb27d to 0049ae5 Compare February 27, 2026 19:44

iloveagent57 force-pushed the aed/optional-marketing-checkbox branch from 0049ae5 to fdd2ea0 Compare February 27, 2026 19:58

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat: make marketing email opt-in checkbox selectively ignorable#38042

feat: make marketing email opt-in checkbox selectively ignorable#38042
iloveagent57 wants to merge 1 commit intomasterfrom
aed/optional-marketing-checkbox

iloveagent57 commented Feb 23, 2026 •

edited

Loading

Uh oh!

openedx-webhooks commented Feb 23, 2026

Uh oh!

kdmccormick left a comment

Uh oh!

kdmccormick Feb 23, 2026

Uh oh!

kdmccormick Feb 23, 2026

Uh oh!

kdmccormick Feb 23, 2026

Uh oh!

kdmccormick Feb 23, 2026

Uh oh!

pwnage101 Feb 24, 2026 •

edited

Loading

Uh oh!

iloveagent57 commented Feb 23, 2026

Uh oh!

pwnage101 Feb 25, 2026

Uh oh!

pwnage101 Feb 25, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

-            "If enabled, the marketing emails opt-in checkbox will be optional for users "
-            "registering via this provider instead of required. When disabled, marketing email opt-in "
-            "is determined by the global MARKETING_EMAILS_OPT_IN setting."
+            "If True, the marketing emails opt-in checkbox will be hidden for users "
+            "registering via this provider, and marketing emails are forced OFF. "
+            "If False, the checkbox remains visible and the checked state defaults "
+            "to the global MARKETING_EMAILS_OPT_IN setting."

Conversation

iloveagent57 commented Feb 23, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Local Testing

Uh oh!

openedx-webhooks commented Feb 23, 2026

Uh oh!

kdmccormick left a comment

Choose a reason for hiding this comment

Uh oh!

kdmccormick Feb 23, 2026

Choose a reason for hiding this comment

Uh oh!

kdmccormick Feb 23, 2026

Choose a reason for hiding this comment

Uh oh!

kdmccormick Feb 23, 2026

Choose a reason for hiding this comment

Uh oh!

kdmccormick Feb 23, 2026

Choose a reason for hiding this comment

Uh oh!

pwnage101 Feb 24, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

iloveagent57 commented Feb 23, 2026

Uh oh!

pwnage101 Feb 25, 2026

Choose a reason for hiding this comment

Uh oh!

pwnage101 Feb 25, 2026

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

iloveagent57 commented Feb 23, 2026 •

edited

Loading

pwnage101 Feb 24, 2026 •

edited

Loading