Project

General

Profile

Support #10245

Placeholders in action emails (activation, password reset) not being properly swapped

Added by Marilyn Weber about 1 year ago. Updated about 1 year ago.

Status:
Resolved
Priority name:
Urgent
Assignee:
Category name:
Email Notifications
Target version:
Start date:
2018-08-28
Due date:
% Done:

0%

Estimated time:

Description

User jenche reports:

Hi, I'm trying to change the email address currently associated with my CUNY Academic Commons account to my Grad Center email. The verification email says:

"go here to confirm the change."

But there's no link to click. Is there any way to confirm the change in email?

My old email (currently associated with the account):

New email:

Screenshot_2018-08-29_19-55-04.png (24.3 KB) Screenshot_2018-08-29_19-55-04.png Boone Gorges, 2018-08-29 03:55 PM

Related issues

Related to CUNY Academic Commons - Bug #10260: bad activation emailRejected2018-08-29

History

#1 Updated by Matt Gold about 1 year ago

  • Status changed from New to Assigned
  • Assignee set to Boone Gorges

okay -- I made the to her account for her. Leaving assigned to Boone to troubleshoot the missing link

#2 Updated by Boone Gorges about 1 year ago

I began debugging this and it turns out that an RBE 'bp_email_get_property' callback is doing some odd filtering. Specifically, the strpos logic here https://github.com/r-a-y/bp-reply-by-email/blob/b09c020d5a6e635d42c18eb035065d3f03c046b0/bp-rbe-core.php#L885 doesn't appear to be correct. I'm unsure whether this is specific to the password reset email.

Ray, could you please have a look?

#3 Updated by Raymond Hoh about 1 year ago

Eek, thanks for finding this bug.

I'm about to push a commit for this.

#4 Updated by Boone Gorges about 1 year ago

  • Subject changed from email change problem to Placeholders in action emails (activation, password reset) not being properly swapped
  • Priority name changed from High to Urgent

Ray, if you're looking into this, could you please update the ticket? I'm going to start working on it myself, but I don't want to duplicate effort.

#5 Updated by Boone Gorges about 1 year ago

  • Related to Bug #10260: bad activation email added

#6 Updated by Boone Gorges about 1 year ago

Oops, cross-post. Thanks Ray!

#7 Updated by Raymond Hoh about 1 year ago

I've added a fix to skip the non-RBE notice injection into registration with blog and email change emails - https://github.com/cuny-academic-commons/cac/commit/c98b1886a825db221e011729b913e2c8a37503d3

I've tested by changing my email address on the "Settings > General" page and the email content now successfully renders.

#10260 appears to be a different bug. I'm looking into this.

#8 Updated by Boone Gorges about 1 year ago

  • Status changed from Assigned to Staged for Production Release

Thanks - I'll mark this as Staged and test after release.

#9 Updated by Boone Gorges about 1 year ago

  • Status changed from Staged for Production Release to Resolved

Working now. Thanks for your quick attention to this.

#10 Updated by Raymond Hoh about 1 year ago

  • Category name set to Email Notifications

I wanted to spend a bit more time on this issue as the RBE workaround seemed to be unnecessary in my eyes.

I found the real cause of the issue. The problem is BuddyPress changed a bit of the email HTML template markup in BuddyPress 3.0, which broke how I was stripping the "Hi USER" salutation from HTML emails.

This caused invalid, broken HTML markup and was the actual reason for the truncated email content in the email address verification email.

I've fixed this here - https://github.com/cuny-academic-commons/cac/commit/5019aa2c74e2d715013ea87dbb7b927e9171f7aa

This also fixes how the user hyperlink was no longer showing up in group emails.

#11 Updated by Boone Gorges about 1 year ago

Thanks, Ray. This is the second or third time I've come across a similar issue in RBE since some changes in BP 3.0. Just spitballing, but do you think there's something that BP could do with its default templates to make this kind of strpos juggling unnecessary? Like what if the template included HTML comments that served as landmarks (sorta like WP actions) - something like <!-- bp.start_of_message --> - so that you could use this when moving your markers around?

#12 Updated by Raymond Hoh about 1 year ago

It's not a RBE problem. The stripping of the "Hi USER" salutation from BP HTML emails is a CAC customization.

One of my other thoughts was to override the BP email HTML template to remove the salutation. However, the salutation is used in GES digests and you can't use conditionals in the email template. Update - Actually, I think BP has an email template hierarchy, so this could be potentially done.

Although the usage of HTML comments to denote certain sections in the BP HTML email template sounds like a good idea in general.

#13 Updated by Boone Gorges about 1 year ago

It's not a RBE problem. The stripping of the "Hi USER" salutation from BP HTML emails is a CAC customization.

I'm referring to the fact that RBE has to use string manipulation to figure out where to put its marker. It's not so much that it's an RBE "problem", it's that RBE has come up with the only possible workaround to the fact that email templates are unstructured entities.

A custom email template in our theme seems like it may be a good idea.

#14 Updated by Raymond Hoh about 1 year ago

Yes, RBE has to rely on HTML source manipulation to re-position the marker, but that isn't the cause of this Redmine issue.

The cause of this issue is due to the changes to the BP HTML email template and the stripping of the "Hi USER" salutation, which also happens to use string manipulation. I was previously attempting to strip a trailing <br> after the <hr />, but that doesn't seem to be necessary any more in the latest BP HTML email template.

RBE doesn't strip the "Hi User" salutation, CAC does. I don't think it's necessary to override the BP HTML email template at this time as there shouldn't be any more problems going forward, unless there are more changes done to the salutation markup in the future.

#15 Updated by Boone Gorges about 1 year ago

I see. Sort of :)

I had one or two client sites experience some ugly issues after upgrading to BP 3.0 because of the RBE parsing, so I guess I was thinking that maybe there was the possibility of a longer-term fix. But I have put much less thought into this than you have, so I'm happy to go with whatever solution you think best!

#16 Updated by Raymond Hoh about 1 year ago

After looking at this a little closely, I do see now that RBE is aggressively removing the marker plus the trailing <br /> and whitespace after the salutation!

So RBE is somewhat at fault here. Although, this problem would only occur if your client sites have customized the main body of the BP HTML email template.

I'll create an issue over at the RBE repo to track this.

Also available in: Atom PDF