r-a-y / bp-reply-by-email

Reply to BuddyPress items from the comfort of your email inbox. Currently in pre-release.
GNU General Public License v2.0
63 stars 9 forks source link

Attachment error emails missing topic name #111

Closed modelm closed 6 years ago

modelm commented 6 years ago

When posting a group topic reply by email with attachments that cause errors (too large), I found that by the time attachment_error_email() runs, the _bbp_topic_id meta for the reply being posted has not yet been set. I confirmed this by adding a bp_rbe_log() with the post meta in both attachment_error_email() & bbp_update_topic_id() - the latter runs last, which leaves the topic name an empty string in the error message.

If I add this line at the beginning of attachment_error_email(), the meta is set and the topic name is correct in the subsequently generated email message:

bbp_update_reply_topic_id( $post_id );

I first tried increasing the priority of add_action( 'bp_rbe_bbpress_after_new_post', array( $this, 'post_attachments' ), 10, 2 ); but it didn't help. There is probably a better long-term solution than my patch but this does fix the problem for now.

r-a-y commented 6 years ago

Can you try commit c37109d and report back?

modelm commented 6 years ago

That does work to get the topic metadata in time. However it also changed the email type from HTML to plaintext which was a surprise. This might require us to change a filter we have on wp_mail that tries to convert all plaintext emails to HTML, I'm not sure yet.

Actually this might be cause by something else, disregard for now.

modelm commented 6 years ago

Sorted out the HTML email problem, it was unrelated. Your change fixes the topic metadata problem. Thanks!

r-a-y commented 6 years ago

The fix I added unfortunately broke how attachment data is appended to the email generated by the BP Group Email Subscription plugin. This is due to moving the 'bp_rbe_bbpress_after_new_post' hook after the creation of the activity item, which GES relies on.

I'm going to revert commit 29b396c and add a better fix for this.