Skip to content

[ENHANCEMENT] ADD V3.4+ Email Template Approach#56

Draft
MaximilianoRicoTabo wants to merge 1 commit into
strangerstudios:devfrom
MaximilianoRicoTabo:enhancement/overhaul-email-templates-v3.4+
Draft

[ENHANCEMENT] ADD V3.4+ Email Template Approach#56
MaximilianoRicoTabo wants to merge 1 commit into
strangerstudios:devfrom
MaximilianoRicoTabo:enhancement/overhaul-email-templates-v3.4+

Conversation

@MaximilianoRicoTabo

@MaximilianoRicoTabo MaximilianoRicoTabo commented Mar 4, 2025

Copy link
Copy Markdown
  • Create a new class in the hierarchy of the PMPro_Email_Template class
  • Require the child class if Parent class exists. Add filter to old template otherwise
  • Call new class if we're in the class exists
image image image

All Submissions:

Changes proposed in this Pull Request:

  • Create a new class in the hierarchy of the PMPro_Email_Template class
  • Require the child class if Parent class exists. Add filter to old template otherwise
  • Call new class if we're in the class exists

How to test the changes in this Pull Request:

  1. Look for Users or Members table list view pages and click resend confirmation link.
  2. Test with 3.4+ and 3.4-

Other information:

  • Have you added an explanation of what your changes do and why you'd like us to include them?
  • Have you successfully run tests with your changes locally?

Changelog entry

Enter a summary of all changes on this Pull Request. This will appear in the changelog if accepted.

 * Create a new class in the hierarchy of the PMPro_Email_Template class
 * Require the child class if Parent class exists. Add filter to old template otherwise
 * Call new class if we're in the class exists
@MaximilianoRicoTabo

Copy link
Copy Markdown
Author

Made this one a draft @dparker1005 because of these doubts.

  1. The body seems to be dynamically build in the old approach based on the body string itself ? Kind of circular reference. Not sure how do we want to deal with this.
    https://github.com/strangerstudios/pmpro-email-confirmation/blob/dev/pmpro-email-confirmation.php#L431-L435
  2. Kept commented few variables on its description because I think they should work but not need to be listed but could be wrong, let me know your thoughts please and I amend. https://github.com/strangerstudios/pmpro-email-confirmation/pull/56/files#diff-47496af6c6f425f510897247d9007df45f7e608190d83df9748651a7633ae65dR97-R99

@andrewlimaza andrewlimaza requested a review from Copilot October 29, 2025 13:26

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Pull Request Overview

This PR refactors the email confirmation system to support the new PMPro Email Template framework while maintaining backward compatibility with the legacy email system. It conditionally loads the appropriate email handling based on whether the PMPro_Email_Template class exists.

Key changes:

  • Introduces a new PMPro_Email_Template_Resend_Confirmation class for modern email template handling
  • Modifies the plugin initialization to conditionally include the new email template class or register the legacy template filter
  • Updates pmproec_resend_confirmation_email() to handle both new and legacy email systems with proper success message handling

Reviewed Changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 5 comments.

File Description
pmpro-email-confirmation.php Adds conditional logic to support both new PMPro Email Template system and legacy email system, updates plugin initialization hook priority, and improves success message handling
classes/email-templates/class-pmpro-email-template-pmpro-email-confirmation-resend-confirmation.php New email template class extending PMPro_Email_Template for handling resend confirmation emails in the modern framework

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

class PMPro_Email_Template_Resend_Confirmation extends PMPro_Email_Template {

/**
* The parent user.

Copilot AI Oct 29, 2025

Copy link

Choose a reason for hiding this comment

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

The comment 'The parent user' is unclear. This should be 'The user receiving the confirmation email' or simply 'The user' to better describe the property's purpose.

Suggested change
* The parent user.
* The user receiving the confirmation email.

Copilot uses AI. Check for mistakes.
*
* @since TBD
*
* @param WP_User $user The user will receive the email.

Copilot AI Oct 29, 2025

Copy link

Choose a reason for hiding this comment

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

Corrected grammar from 'The user will receive' to 'The user who will receive' for better readability.

Suggested change
* @param WP_User $user The user will receive the email.
* @param WP_User $user The user who will receive the email.

Copilot uses AI. Check for mistakes.
Comment on lines +97 to +99
// '!!display_name!!' => esc_html__( 'The display name of the user need to confirm the email', 'pmpro-email-confirmation' ),
// '!!user_login!!' => esc_html__( 'The login name of the user need to confirm the email', 'pmpro-email-confirmation' ),
// '!!user_email!!' => esc_html__( 'The email address of the user need to confirm the email', 'pmpro-email-confirmation' ),

Copilot AI Oct 29, 2025

Copy link

Choose a reason for hiding this comment

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

These commented-out template variables are defined in get_email_template_variables() (lines 114-116) but not exposed in get_email_template_variables_with_description(). If these variables are not intended to be available for use in email templates, they should be removed from the method. If they should be available, uncomment and fix the grammar ('user need' should be 'user who needs').

Suggested change
// '!!display_name!!' => esc_html__( 'The display name of the user need to confirm the email', 'pmpro-email-confirmation' ),
// '!!user_login!!' => esc_html__( 'The login name of the user need to confirm the email', 'pmpro-email-confirmation' ),
// '!!user_email!!' => esc_html__( 'The email address of the user need to confirm the email', 'pmpro-email-confirmation' ),
'!!display_name!!' => esc_html__( 'The display name of the user who needs to confirm the email.', 'pmpro-email-confirmation' ),
'!!user_login!!' => esc_html__( 'The login name of the user who needs to confirm the email.', 'pmpro-email-confirmation' ),
'!!user_email!!' => esc_html__( 'The email address of the user who needs to confirm the email.', 'pmpro-email-confirmation' ),

Copilot uses AI. Check for mistakes.
* @return string The name of the email recipient.
*/
public function get_recipient_name() {

Copilot AI Oct 29, 2025

Copy link

Choose a reason for hiding this comment

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

Remove the empty line with trailing whitespace before the return statement.

Suggested change

Copilot uses AI. Check for mistakes.
$body = str_ireplace( "!!validation_link!!", $url, $body );
if ( class_exists( 'PMPro_Email_Template' ) ) {
$send_resend_confirmation_email = new PMPro_Email_Template_Resend_Confirmation( $user, $url );
$mail_sent_successfully = $send_resend_confirmation_email->send();

Copilot AI Oct 29, 2025

Copy link

Choose a reason for hiding this comment

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

When using the new PMPro_Email_Template system (lines 431-433), the success message is never set. The $pmproec_msg and $pmproec_msgt globals are only set within the else block (lines 465-468). This means users won't see a confirmation message when the email is sent using the new template system. Add the success message logic after line 433.

Suggested change
$mail_sent_successfully = $send_resend_confirmation_email->send();
$mail_sent_successfully = $send_resend_confirmation_email->send();
if ( $mail_sent_successfully ) {
$pmproec_msg = esc_html__( 'A confirmation email has been sent to', 'pmpro-email-confirmation' ) . ' ' . $user->user_email;
$pmproec_msgt = 'updated';
}

Copilot uses AI. Check for mistakes.
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.

2 participants