Allow reply-to individual instead of topic/forum (mailing list feature)


(Steve Combs) #14

@DeanMarkTaylor since the reply would only go to one user, not sure we need to sort out the multiple users in email response issue. It would be the same approach though–recipient would be identified with a reply key for the PM.

I do think showing the parties to the message in the email is important (critical for multiple party PMs) and trying to match the online experience for PMs would be good at least for consistency purposes:

Not required for this feature since there would only be two users, but would love to fix the “show multiple parties to PM” issues.


(Steve Combs) #15

I assumed exposing emails and the “reply” function of email clients would be the two biggest issues.

Letting any user discover the email of any other user on the system would be a big deal for a lot of communities. I understand listservs operate that way. Could be abused in lots of different ways and require additional security/privacy features. Would have to be opt-in (user account setting; defaulted to off) for existing communities and disclosed fully for new communities. If the private messaging features work via email, not sure why a user MUST HAVE the email address. It would eliminate protections like blocking users and add potential liability to community operators (email address can expose a lot of info about people). Users can choose to put that info in their profile or post.

I agree the reply-to issue is important for users. The reply-all feature of most email clients is not the default and sort of hidden many times. So we are making community users do more work to participate in the community or, even worse, users may think they are replying to the community (current Discourse default) and it just goes to the one user.

Instead of

Consider the following:

If the user hits “Reply”, then it goes to the user who sent the message [username] [replies+fasdfsdfa@example.com] AND the community [sitename] [replies+asdfasasdfa@example.com] (with the replies keys for each listed separately in the To: field. The user could delete the sitename address out of the To: field if they choose. (Would need to not send duplicate emails to the recipient if both emails remain). Alternatively, just add a setting so users can choose the reply functionality (like in your screenshot above). This would also fix the confusion about “reply to this email to respond.” I’d support that approach.


(Tarek Loubani) #16

Agreed that exposing people’s email addresses is in no way a must have. I suggest it only if it makes things simpler.

For example, considering that I personally lack the programming experience, I imagine that my hack to do this will consist of introducing the “reply to” as the sending user’s email address and the “CC” as the replies+ address, such that the expected behaviour people know (reply = just the user; reply all = the whole community) would work. Yes, some issues are introduced (namely, duplicate reception of emails), but it would generally solve the need.

In that sense, I would suggest that “reply” from an email client (e.g., reply-to header) should be the SENDER and “reply all” should be the topic.

tarek : )


(Erlend Sogge Heggen) #17

Is that really how “normal mailing lists” work? Google Groups, easily the most popular mailing list platform in existence, does not appear to work this way. By default, my reply will be to the mailing list address and noone else.

I like @scombs’ proposal to add a special PM button and have that open a new email dialogue. It’s not prone to user error; there are no surprises here as there could be if we’re getting involved with Reply/Reply-to-all conventions.


(Tarek Loubani) #18

I just took a look at a few lists I’m on. Of about ~ 15 google groups, all of them have reply-to-list as their default (not sure that reply-to-sender is an option). Of about ~ 15 GNU/Mailman lists, all of them had reply-to-individual as default. I don’t have enough yahoo group subscriptions to make any statements about them.

So perhaps it’s a philosophical choice that the different list providers make on behalf of list admins?


(Tarek Loubani) #19

I’ve been playing around with the code, and arrived at this:

In line 148 of lib/email/message_builder.rb, this would result in what I would like to do:


      if allow_reply_by_email?
        result['X-Discourse-Reply-Key'] = reply_key
        result['Reply-To'] = "test1@test.com"
        result['CC'] = reply_by_email_address
      else
        result['Reply-To'] = from_value
      end

However, now I’m trying to figure out what the ‘sender email address’ variable would be in ruby world…


A list of server-side functions for weekend/novice Discourse developers?
(Tarek Loubani) #20

After lots of support and help from others, I have finally sorted out the rawest form of how to accomplish this. It is presented below as a patch:

diff --git a/lib/email/message_builder.rb b/lib/email/message_builder.rb
index f099b11..2a69249 100644
--- a/lib/email/message_builder.rb
+++ b/lib/email/message_builder.rb
@@ -147,7 +147,15 @@ module Email
 
       if allow_reply_by_email?
         result['X-Discourse-Reply-Key'] = reply_key
-        result['Reply-To'] = reply_by_email_address
+        if @opts[:private_reply] == true
+          result['Reply-To'] = reply_by_email_address
+        else
+          p = Post.find_by_id @opts[:post_id]
+          result['Reply-To'] = "#{p.user.name} <#{p.user.email}>"
+          result['CC'] = reply_by_email_address
+        end
       else
         result['Reply-To'] = from_value
       end

This patch does the following:

  1. By default, users who hit ‘reply to’ will only reply to the sending user.
  2. For a user to post a public message to the mailing list / topic, they would have to hit ‘reply all’.
  3. It does not act on private messages.

Note: This exposes the sender’s email address to all recipients.

Unfortunately, I will implement this patch for our use-case effective immediately.

However, I recognize that it would be amazing if the following were true:

  1. A per-category or site-wide option to select this
  2. A per-category or site-wide option to expose email addresses.
  3. Integration into personal messages rather than via email directly.
  4. When the responding user hits ‘reply all’ and selects both the list and the sending user, the sending user should not receive an email notification from Discourse. Currently, two messages - one via email and one via Discourse - will arrive.

I hope the patch as it is helps somebody. If I could make it a per-category option, would you guys consider folding it into the master branch, @erlend_sh?


A list of server-side functions for weekend/novice Discourse developers?
A list of server-side functions for weekend/novice Discourse developers?
(Erlend Sogge Heggen) #21

Maybe @zogstrip can chime in here since he’s been our :e-mail: guy extraordinaire as of late.

If at all possible, I’d much rather have this tweak done in the form of a plugin. No other reply-by-email solution I’ve ever tried works this way. Your use case is valid, but incredibly rare.


(Tarek Loubani) #22

It’s less a question of desire than ability. I’ve searched for documentation about how to develop a plugin as a Rails novice, but had no luck. Obviously I would really prefer not to have to patch core every time…

Could you point me to a simple plugin I could manipulate / use as a base to do this?


(Jeff Atwood) #23

Discourse Needs help resources and documentation
(Tarek Loubani) #24

Hello all,

I am trying to understand how to properly make this plugin. I’m currently stuck in my plugin.rb, as it seems that I cannot properly replace the MessageBuilder class.

Here is the current plugin.rb (and the repo in general):

I have tried to understand how to do this by reading a few topics I could find in searches, but as of yet no luck:

When I run the plugin, I get the following, which I suspect reflects that the class is not properly replaced:

/vagrant/lib/email/message_builder.rb:20: warning: already initialized constant Email::MessageBuilder::REPLY_TO_AUTO_GENERATED_HEADER_KEY
/vagrant/lib/email/message_builder.rb:20: warning: previous definition of REPLY_TO_AUTO_GENERATED_HEADER_KEY was here
/vagrant/lib/email/message_builder.rb:21: warning: already initialized constant Email::MessageBuilder::REPLY_TO_AUTO_GENERATED_HEADER_VALUE
/vagrant/lib/email/message_builder.rb:21: warning: previous definition of REPLY_TO_AUTO_GENERATED_HEADER_VALUE was here

Any thoughts or advice?


(Mittineague) #25

I replaced a class method so it might need to be done a bit differently
(i.e. instance_eval instead of class_eval ?)
* and yes, the names are counter-intuitive


(Simon Cossar) #26

Take a look at this for how I have reopened the Email::Sender class:

https://rubymonk.com/learning/books/5-metaprogramming-ruby-ascent/chapters/24-eval/lessons/68-class-eval


(Tarek Loubani) #27

Simon, that seems to definitely change things, but now the class is totally unusable with the error:

Jobs::HandledExceptionWrapper: Wrapped NameError: uninitialized constant MessageBuilder

I’ve posted a Gist of the changes I made to mimic yours here:

Definitely this seems like a step forward, since at least I’m manipulating the class, but not sure what I’m doing wrong here. I will also try to understand @Mittineague 's solution and see if I can implement it.


Question over Private Messaging
(Tarek Loubani) #28

OK, I think I got it going by removing the after_initialize. Thank you!!!


(Tarek Loubani) #29

Hmm. Unfortunately still no dice.

I’ve added a byebug both after the class and after the method to see if they are being accessed. If added after the class, it is clear that the class is being tapped. However, after the method, it is not. Have I gotten the namespace wrong somehow?

Here’s is the most recent gist.

Incidentally, this error remains:

/vagrant/lib/email/message_builder.rb:20: warning: already initialized constant Email::MessageBuilder::REPLY_TO_AUTO_GENERATED_HEADER_KEY
/vagrant/lib/email/message_builder.rb:20: warning: previous definition of REPLY_TO_AUTO_GENERATED_HEADER_KEY was here
/vagrant/lib/email/message_builder.rb:21: warning: already initialized constant Email::MessageBuilder::REPLY_TO_AUTO_GENERATED_HEADER_VALUE
/vagrant/lib/email/message_builder.rb:21: warning: previous definition of REPLY_TO_AUTO_GENERATED_HEADER_VALUE was here

(Simon Cossar) #30

Maybe try something like this, just to see that everything is connected:

after_initialize do
  Email::MessageBuilder.class_eval do

    def body
      'hello world'
    end

  end
end

If you send yourself a password reset email, you should see ‘hello world’ as the text.


(Tarek Loubani) #31

OK, a few remarks:

  1. The require line is unnecessary and should be taken out.
  2. There appears to be a bug where the header_args cannot be inserted into a plugin as currently written. I will report this as a bug after a bit more testing and see what others suggest as a fix

(Tarek Loubani) #32

Finally, success!!!

I discovered the error logs (wow… how did this take so long!) They are at: http://127.0.0.1:4000/logs on a vagrant install.

I saw the following error:

Job exception: uninitialized constant MessageBuilder

Which led me to add Email::

Now, the plugin is working!!! Thank you @Simon_Cossar and @Mittineague!!! Now the plugin’s first working version is done. I will post more soon.


(Tarek Loubani) #33

Here is the plugin, which is working as best as I can tell:

Thank you again to all of those who have helped!