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

feat: add allow_newsletter field to account settings #57

Conversation

BryanttV
Copy link
Contributor

@BryanttV BryanttV commented Dec 12, 2024

Description

This PR adds the allow_newsletter field (Checkbox) in the Account Settings. This allows each user to update the value of the field.

Related Issues

Testing Instructions

  1. Create a Tutor environment in the Redwood version.
  2. Install this plugin with the changes in this PR.
  3. Create a mount of edx-platform with the changes in this PR.
  4. Update the account.redirect_to_microfrontend Waffle Flag. Set Everyone to No. This allows access to the legacy interface.
  5. Go to {lms_domain}/account/settings
  6. You should see the new checkbox field in the Additional Information section.
  7. Try to update the field and verify that the value was updated in the model.

Screenshots

image

@BryanttV BryanttV force-pushed the bav/reactive-schedules-nudge-emails branch from 805dcbb to d10bb12 Compare December 12, 2024 17:52
@BryanttV BryanttV force-pushed the bav/add-allow-newsletter-field-to-account-settings branch from 44e1f00 to 5770b9e Compare December 12, 2024 19:03
@BryanttV BryanttV marked this pull request as ready for review December 13, 2024 21:31
@BryanttV BryanttV requested a review from MaferMazu December 13, 2024 21:37
@BryanttV BryanttV force-pushed the bav/add-allow-newsletter-field-to-account-settings branch from a9a3d06 to 81d79a7 Compare December 16, 2024 21:08
@BryanttV BryanttV requested a review from igobranco December 17, 2024 13:09
@BryanttV BryanttV force-pushed the bav/add-allow-newsletter-field-to-account-settings branch from 81d79a7 to 546ebce Compare December 17, 2024 13:19
Copy link
Contributor

@MaferMazu MaferMazu left a comment

Choose a reason for hiding this comment

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

Thanks for this, @BryanttV, this worked as expected ✨

My only suggestion, which is not a blocker, is that although this feature is related to the scheduled nudged emails, it does not have a direct dependency. Therefore, to ensure clarity and that the PRs do not block each other, I recommend that the PR go to redwood.master instead of the nudged branch.

Copy link
Member

@igobranco igobranco left a comment

Choose a reason for hiding this comment

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

LGTM ✔️

@igobranco igobranco merged commit 569cd4b into bav/reactive-schedules-nudge-emails Jan 2, 2025
5 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants