Add a shortcut to get the unread conversations number

[Roendal]

This code

mailbox.inbox(:unread => true).count(:id, :distinct => true)

should have a method in mailbox or messageable in order to make it easier.

Similar methods should be taken into account.

[fabianoalmeida]

@Roendal Does this solve it?

mailbox.inbox.unread

[Roendal]

Hi @fabianoalmeida!

The code you suggests ...

mailbox.inbox.unread

... returns the unread messages. I mean, the messages themself.

What I was looking for is the number of them. If there are 3 new messages, I want a "3", not the 3 messages. This is achieved with:

mailbox.inbox(:unread => true).count(:id, :distinct => true)

But thi code is awfully large and not developer friendly. With this issue I meant there is a need for an alias to this code. Something like

mailboxer.unread_count
#or
mailboxer.inbox_unread_count

Which internally executed the awful and unfriendly code.

Does this help? :)

Thanks for the interest!

[fabianoalmeida]

@Roendal Whether I understood what you said... I wrote a test to demonstrate my idea (this test is green 😜):

first  = FactoryGirl.create(:user)
second = FactoryGirl.create(:user)

first_conversation  = first.send_message(second, "Body", "Subject").conversation
second_conversation = second.send_message(first, "Body", "Subject").conversation
third_conversation  = first.send_message(second, "Body", "Subject").conversation

first.mailbox.inbox(:unread => true).to_a.should have(1).items
first.mailbox.inbox.unread(first).to_a.should have(1).items
first.mailbox.inbox(:unread => true).first.should eql(first.mailbox.inbox.unread(first).first)
first.mailbox.inbox(:unread => true).count.should eql(1)
first.mailbox.inbox.unread(first).count.should eql(1)

second.mailbox.inbox(:unread => true).to_a.should have(2).items
second.mailbox.inbox.unread(second).to_a.should have(2).items
second.mailbox.inbox(:unread => true).first.should eql(second.mailbox.inbox.unread(second).first)
second.mailbox.inbox(:unread => true).count.should eql(2)
second.mailbox.inbox.unread(second).count.should eql(2)

In the other words, we could use

mailbox.inbox(unread: true).count

Or

mailbox.inbox.unread(user_instance).count

Am I correct? This example helps?

[Roendal]

Hi @fabianoalmeida! Now I understand what you mean ;)

We used this code

mailbox.inbox(:unread => true).count(:id, :distinct => true)

because some time messages got counted twice (or even more).

I don't remember the exact way to replicate it, and maybe, as It has almost a year, this issue is not legit anymore.

Feel free to do some testing with more populated conversations (4 o 5 users and messages) trying to see anything strange in the counting. If you are confident it is ok now, I leave you the pleasure of closing the issue ;)

Thanks again @fabianoalmeida and sorry for my misunderstanding!

[fabianoalmeida]

Hmmm... I understood now. Ok @Roendal, I'll try simulate this issue and whether I get reproduce it I'll send a pull request.

Don't worry about the misunderstanding... When we're writing It's normal to write things dubious. 😉

BTW... Can I try to refactor this gem? Try to create a better DSL, you know?

[Roendal]

Of course @fabianoalmeida, every help is welcome.

I moved to other project not related with rails almost a year ago and can't do any major work on Mailboxer. Its really good to see people trying to improve it ;)

[fabianoalmeida]

Nice!

@Roendal, I have a question... In my test above,

first_conversation  = first.send_message(second, "Body", "Subject").conversation
second_conversation = second.send_message(first, "Body", "Subject").conversation

first_conversation should be equal to second_conversation?
Cause your answer is false, what's the idea about a conversation model?

[Roendal]

Each time yo doy a messageable.send_message you create a new conversation.

In order to send other messages to a particular conversation you must use
http://rubydoc.info/gems/mailboxer/0.9.0/Mailboxer/Models/Messageable#reply_to_conversation-instance_method
http://rubydoc.info/gems/mailboxer/0.9.0/Mailboxer/Models/Messageable#reply_to_all-instance_method
http://rubydoc.info/gems/mailboxer/0.9.0/Mailboxer/Models/Messageable#reply_to_sender-instance_method

The idea of Conversation is pretty similar to the way Gmail handles the email conversations.

[atd]

Hi @fabianoalmeida, as @Roendal says every help making Mailboxer better is awesome.

I will only request that, if a better DSL is designed, the old one is keeped for a while along with deprecation warnings, so projects that are already using Mailboxer are given the chance to upgrade in a smooth way

[fabianoalmeida]

@atd Sure! I don't know how can do that, but whether I do, I'll submit a PR and we can discuss about the better way to make it.

@Roendal What is the idea about ​​Notification? And why Message is a Notification?

[fabianoalmeida]

I was studying and rewriting some code... What do you think about it?

# other_users could be an unique User or a list of Users
User.message('Body', 'Subject').to(other_users).ship

I guess this option is more "readable", not?

[Roendal]

Yeah, It totally changes the way Mailboxer was implemented but its more readable. The problem I see is that the bigger the gap between codes is, the more difficult is to readapt your code. Please, make sure to add a good documentation for the sake of the users ;)

About Notification and Messages. Mailboxer supports the use of "system notifications" more or less similar to the ones of Facebook. A Notification is some sort of Message but with less attributes (no sender, for example, is "the system") and less functionality (Yo don't reply to a notification). But still they are pretty similar between them. Thats the idea of Message being a improved Notification sharing the same DB.

[fabianoalmeida]

@Roendal I can see your point, but what do you think about the @atd suggestion?

the old one is keeped for a while along with deprecation warnings

After I saw the code and read your explanation about Notification, I see it as an external model, i.e., something different of Message model. In my mind, Notification has a Message, you know?

I see a Notification, like on Facebook, as something optional, unplugged and asynchronous. This could be optional per User, where each User could choice send or receive automatically notifications. The idea about "unplugged" is to use as an asynchronous service: Sidekiq, Resque etc.

I'm launching a lot of new ideas that could be used to refactor it, because this is a great gem for me, however this could be better to work with. 😊

[Roendal]

I am really really happy you want to work with mailboxer :)

The main problem with your idea is what you understand by Message. Message isn't just the text itself, but the metainfo assciated with sender and recipients. So a message isn't just "Hello Mary, how are you?" a messa is "A told be <<Hello Mary, how are you?". If you think this way, a Notification has some text, but never a full Message.

I don't know if this is clear, but Notifications are NOT related with any Message. An example of Notifications, used in the related project ging/social_stream, is "A added you as a friend", "B commented on your photo", ... So they are used in a unplugged and asynchronous way.

Maybe they could be more unplugged, or more asynchronous, or even with a better structure, but we should try to avoid any bigger change to the DB. If you want to support the old way with deprecation warnings and make any update as smooth as possible, as @atd asked, maybe making a great change on Notification behaviour and/or data structure could be a bit disruptive.

Keep on with the great work and throw any singe idea you have to the arena ;)

[fabianoalmeida]

@Roendal sorry, but I didn't understand the idea about Message model. BTW, I'm also really really happy to help. Whether Message isn't only some text, as you said, where is generated this metainfo? 😕

And you also said:

Notifications are NOT related with any Message

But I can see this, ie Message is an inheritance of Notification, you know?

I understood the idea about Notification (and I love it), but this could be simpler. Take a look:

class Message < ActiveRecord::Base
  after_save :notify

  def notify
    # Depending the type of notification, of course... :added, :commented, :replied...
    Notification.send
  end
end

Using this way to send a new notification, we could send a notification from any place: Messageable, Conversation, Message...

What do you think?

[Roendal]

Hi @fabianoalmeida!

Message is an inheritance of Notification only due to their data model similarities. They share the same DB because almost al fields are the same. But there is no futher relation.

When you send a Message to someone, you create a Conversation, you create the first Message of this Conversation and a Receipt to relate each Messageable recipient with the Message.

When you notify something (and I mean, you tell the user something from your app like "User B likes your post", "Your credit is about to expire", etc. ) you create a Notification and a Receipt to relate each Messageable recipient with the Notification.

Message and Notification have the general information of the shared information as the subject or the body. Any metadata regarding the relation between Messageable and Message/ Notification (is read, is trashed, etc) is stored in Receipt.

If this doen't clarify a bit the matter, you should explain me with some use cases what you understand of Mailboxer models in order to see what I missed when designing it ;)

I dont get your code to "send a notification from any place". They are not intended to be used from within Mailboxer, but from the outside application. Maybe they should be packed In a "Notifier" gem, but we packed them together as a "Messaging and notifying system".

Take your time and make the reply as long as you need. If you prefer to discuss this by email, don't mind telling me ;)

[fabianoalmeida]

@Roendal I sent an email to you. It's better continuing this discussion by there. Take a look when you can. BTW, I'm loving discuss possibilities about refactoring this gem. ❤️ 😉

[Roendal]

Thanks for all your help and your willing to improve the gem 😃

I will close this issue and continue the discussion by email ;)

[fabianoalmeida]

@Roendal, sorry, I forgot to say... This issue still exists. I got it after simulating to send and responding a lot of messages. So, you can close this one, but we need solve that after. 😐

[Roendal]

My fault! I am a quick closer ;)

I will reopen it in order to be aware of it.