Sending messages without a subject

[damien]

Hey guys!

First off, thanks for mailboxer. Great piece of work!

That being said, is there any particular reason why a subject is required to send a message? I have a requirement to send messages without requiring a subject and ended up overriding the default send_message method like so:

# Override's Mailboxer's #send_message such that subjects are not required.
# @see https://github.com/ging/mailboxer/blob/56987403939d6077ddb72bc708df90ab6e672ab6/lib/mailboxer/models/messageable.rb#L48-57
# @param [Hash] opts An options hash used to configure and send a message
# @option [User, Array<User>] :to Users to send a message to
# @option [String] :subject An optional subject for this message (nil)
# @option [File] :attatchment An optional attatchment to include with this message (nil)
# @options [Boolean] :sanitize Weather or not to sanetize input (true)
def send_message(opts = {})
  recipients = opts.delete(:to)
  subject    = opts.delete(:subject) || ""
  body       = opts.delete(:body)
  attachment = opts.delete(:attatchment)
  sanitize   = opts.delete(:sanitize) || true

  convo = Conversation.new(subject: subject)

  message = messages.new(body: body, subject: subject, attachment: attachment)
  message.conversation = convo
  message.recipients = recipients.is_a?(Array) ? recipients : [recipients]
  message.recipients = message.recipients.uniq

  message.deliver(false, sanitize)
end

Here's what I'm wondering:

1. Will this cause any issues with Mailboxer's data model?
2. Are there any implications to making a subject optional that I should consider or be aware of?
3. If everything here checks out, would you guys be open to me submitting this as a patch to send_message?

[Roendal]

Hi @damien!

If mailboxer is somehow great is for people like you submitting new ideas ;)

There is no particular reason for subjet to be required, indeed it seems a great bug if you don't need to use them. We never faced this problem, so we never came up with a solution.

The answers for 1 and 2 are NO. There should be no problem with Mailboxer and, as far as I remember, no other implication is present.

I will happily accept this patch, but as long as this will require a new minor version because of the change, I think I should be a bit deeper.

The change from method(param A, param B, param C,..) to method(params={}) has been delayed because of my lack of time. There are many other methods in need of this change, ie: in Messageable reply, reply_to_sender, etc. or in Notification notify_all. Of course, the test related to them have to ve updated.

You did more than enough by opening this issue and letting us know the bug, but do you feel brave enough to help us by submitting this change?

Thanks, many many thanks!

[damien]

@Roendal I was considering doing just that! I just wanted to make sure it would be an acceptable patch before I started any work.

I'm tight on time now, but I'll most likely be working on this tomorrow evening and/or this weekend. I'll update this issue as I make progress.

[Roendal]

Take the time you need @damien!

Thanks again :)

[damien]

Just an update; I'm currently working on this and it's turning out to be a bit more involved than I had originally anticipated. I'm doing a lot of updating of the test suite to get it green after changing most of the function signatures across the app. I'll definitely need a solid code review before this patch gets merged.

Also worth noting: I've been updating a lot of the inline YARD documentation for my own benefit. Thus far most of the functions I've touched now have documentation on the input paramaters and return types.

[Roendal]

Thanks for the update and good luck with the challenge. It's indeed a really great one!

You are doing a superb work @damien! Take the time you need!

[Texicitys]

Hello !

I have the same problem. I want that the subject is optional, I tried this :
conversation.rb :

class Conversation < ActiveRecord::Base
  attr_accessible :subject if Mailboxer.protected_attributes?
  has_many :messages, :dependent => :destroy
  has_many :receipts, :through => :messages
  #TODO Virer l'obligation d un sujet
  #validates_presence_of :subject

messageable.rb

def send_message(recipients, msg_body, subject='', sanitize_text=true, attachment=nil, message_timestamp = Time.now)
        convo = Conversation.new({:subject => subject})
        convo.created_at = message_timestamp
        convo.updated_at = message_timestamp
        message = messages.new({:body => msg_body, :subject => subject, :attachment => attachment})

But when I try :
current_user.send_message(current_user, 'salut')

Nothing happend.. What did I miss? Can I accept a mail without subject ?

Thank you for your help !

[w3irdrobot]

The Mailboxer::Notification class also requires a subject. That is probably why it didn't work for you.

[Joe312341]

Was this change ever submitted (ability to reply without a subject) ?

[stevelacey]

Anyone reading – I opt for setting the subject to - and then never showing it anywhere.

¯_(ツ)_/¯