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
Add setting in user profile page to Opt out of or Opt in to receiving individual emails #7586
Conversation
Test the previous changes of this PR with WordPress Playground. |
Test the previous changes of this PR with WordPress Playground. |
Test the previous changes of this PR with WordPress Playground. |
Test the previous changes of this PR with WordPress Playground. |
I don't think we need the subject, and it's difficult to distinguish between student and teacher emails currently. Could we do something like this instead? |
816cb19
to
100c2c8
Compare
Test the previous changes of this PR with WordPress Playground. |
Test the previous changes of this PR with WordPress Playground. |
Test the previous changes of this PR with WordPress Playground. |
Test the previous changes of this PR with WordPress Playground. |
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## trunk #7586 +/- ##
============================================
+ Coverage 51.77% 51.84% +0.07%
- Complexity 11401 11436 +35
============================================
Files 650 652 +2
Lines 48549 48656 +107
Branches 470 470
============================================
+ Hits 25135 25228 +93
- Misses 23033 23047 +14
Partials 381 381
... and 32 files with indirect coverage changes Continue to review full report in Codecov by Sentry.
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Works well 👍
Left two suggestions. Optional, but...
Really enjoyed reading test in tests/unit-tests/internal/emails/test-class-email-subscription.php.
<h3><?php esc_html_e( 'Sensei Email', 'sensei-lms' ); ?></h3> | ||
|
||
<table class="form-table"> | ||
<?php | ||
foreach ( $user_emails as $type => $emails ) { | ||
?> | ||
<tr> | ||
<th scope="row"> | ||
<?php | ||
switch ( $type ) { | ||
case 'student': | ||
esc_html_e( 'Student Emails', 'sensei-lms' ); | ||
break; | ||
case 'teacher': | ||
esc_html_e( 'Teacher Emails', 'sensei-lms' ); | ||
break; | ||
default: | ||
esc_html_e( 'Other Emails', 'sensei-lms' ); | ||
break; | ||
} | ||
?> | ||
</th> | ||
<td> | ||
<?php | ||
foreach ( $emails as $identifier => $email ) { | ||
$this->render_email_subscription_setting( $user->ID, $identifier, $email ); | ||
} | ||
?> | ||
</td> | ||
</tr> | ||
<?php | ||
} | ||
?> | ||
</table> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Could be extracted into a view file.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Extracted here: d4bbe81
<fieldset> | ||
<label for="<?php echo esc_attr( $this->get_field_id( $identifier ) ); ?>"> | ||
<input name="sensei-email-subscriptions[]" type="checkbox" value="<?php echo esc_attr( $identifier ); ?>" <?php checked( false, $is_unsubscribed ); ?> id="<?php echo esc_attr( $this->get_field_id( $identifier ) ); ?>"> | ||
<?php echo esc_html( $description ); ?> | ||
</label> | ||
</fieldset> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Same here: could be extracted into a view file.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Extracted here: d4bbe81
Test the previous changes of this PR with WordPress Playground. |
Test the previous changes of this PR with WordPress Playground. |
2f13ae5
to
3c1eb87
Compare
Test the previous changes of this PR with WordPress Playground. |
Test the previous changes of this PR with WordPress Playground. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Restore my approval.
Resolves #7497
Proposed Changes
Added a setting section in the User profile page in WpAdmin where the user can Opt out of (Or later Opt in to) individual emails they receive.
These were the considerations -
Note: We initially planned to add a link in the bottom of the emails with an URL to this page so that the users will know where to go to for unsubscribing. But @donnapep and I discussed later and decided not to do it here. Because in some sites, users are not given access to WP-Admin, putting this URL in the emails of those sites will not make sense.
Testing Instructions
New/Updated Hooks
sensei_send_emails
- Updated hook by adding a new param to identify the email being sent.Pre-Merge Checklist