Skip to content
Permalink

Comparing changes

Choose two branches to see what’s changed or to start a new pull request. If you need to, you can also or learn more about diff comparisons.

Open a pull request

Create a new pull request by comparing changes across two branches. If you need to, you can also . Learn more about diff comparisons here.
base repository: mailboxer/mailboxer
Failed to load repositories. Confirm that selected base ref is valid, then try again.
Loading
base: master
Choose a base ref
...
head repository: subvertical/mailboxer
Failed to load repositories. Confirm that selected head ref is valid, then try again.
Loading
compare: master
Choose a head ref
Can’t automatically merge. Don’t worry, you can still create the pull request.

Commits on Jun 11, 2016

  1. Copy the full SHA
    7350b8c View commit details
  2. Copy the full SHA
    39fc803 View commit details
  3. Copy the full SHA
    e6b7ee4 View commit details

Commits on Dec 19, 2016

  1. Remove enforcement of subject presence for message

    David Rodríguez authored and tapn2it committed Dec 19, 2016
    Copy the full SHA
    6aee285 View commit details
  2. Call save before delivering email

    David Rodríguez authored and tapn2it committed Dec 19, 2016
    Copy the full SHA
    67e09d0 View commit details
  3. Remove Ruby 1.9 references in Travis

    David Rodríguez authored and tapn2it committed Dec 19, 2016
    Copy the full SHA
    e5cb868 View commit details
  4. Remove redundant configuration in travis

    David Rodríguez authored and tapn2it committed Dec 19, 2016
    Copy the full SHA
    6f2b09f View commit details
  5. Build against all modern rubies in travis

    David Rodríguez authored and tapn2it committed Dec 19, 2016
    Copy the full SHA
    2f43323 View commit details
  6. Copy the full SHA
    6a88aa3 View commit details
  7. Copy the full SHA
    72825a7 View commit details
  8. Replace depricated call to uniq

    t27duck authored and tapn2it committed Dec 19, 2016
    Copy the full SHA
    d6276ff View commit details
  9. Copy the full SHA
    9744109 View commit details
  10. Do not use backticks for table names in between scope

    This is a reimplementation of PR #406 by @jurgens that has been rebased
    with master and with a passing build.
    
    The use of backticks for table names is specific to the MySQL syntax.
    Other DBMSes like PostgreSQL use double quotes. However, all DBMSes
    should get along fine with only the table name in the query.
    t27duck authored and tapn2it committed Dec 19, 2016
    Copy the full SHA
    337abf8 View commit details
  11. Remove defunct Rails version checks

    Since the gemspec has Rails 4.2 and later as a dependency, the old < 4
    version checks for Rails should never be executed. This removes the now
    dead code.
    t27duck authored and tapn2it committed Dec 19, 2016
    Copy the full SHA
    3c9b936 View commit details
  12. Fix Rails 5 deprecation

    David Rodríguez authored and tapn2it committed Dec 19, 2016
    Copy the full SHA
    5ce88f3 View commit details
  13. Don't hardcode model / table name unless necessary

    Added a regression test because I was not sure removing the class name
    from the `belongs_to` would work. It does.
    David Rodríguez authored and tapn2it committed Dec 19, 2016
    Copy the full SHA
    3f263a2 View commit details
  14. Fix another Rails 5 deprecation

    David Rodríguez authored and tapn2it committed Dec 19, 2016
    Copy the full SHA
    ebee6a6 View commit details
  15. Add a regression test

    The `autosave` flag in the notifications's `belongs_to` is currently
    necessary but removing it woudn't make the tests fail. This test catches
    it.
    David Rodríguez authored and tapn2it committed Dec 19, 2016
    Copy the full SHA
    6975cfd View commit details
  16. Follow up b6ffa85

    Since that commit, notifications no longer require a subject, so we'll
    never get duplicate validation errors and we don't need to clean those
    up.
    David Rodríguez authored and tapn2it committed Dec 19, 2016
    Copy the full SHA
    7d397d3 View commit details
  17. Save objects in a more straight way

    It's simpler to just save the parent and let the children be autosaved
    (and autovalidated) since this is how Rails works out of the box. We
    just need to make sure the receipts are properly tied to the parent
    notification. We get that, again, by using Rails standard constructs
    like `association#build` instead of the ad-hoc `build_receipt` method.
    
    The previous behavior caused issues like getting duplicated errors in
    the notification object, because of the fact that everytime a receipt
    was saved, the parent notification would be saved too and validations
    would be (re)run.
    David Rodríguez authored and tapn2it committed Dec 19, 2016
    Copy the full SHA
    4418735 View commit details
  18. Bump version to 0.14.0

    jcoyne authored and tapn2it committed Dec 19, 2016
    Copy the full SHA
    60bc52e View commit details
  19. Add a change log

    David Rodríguez authored and tapn2it committed Dec 19, 2016
    Copy the full SHA
    c526e92 View commit details
  20. Copy the full SHA
    556ad5f View commit details
  21. Use Rails conventions for ActiveRecord wheres and orders

    When using `where` with a `joins`, you can reference the the join
    table as a key in a hash in `where`.
    
    Updated `Mailboxer::Receipt`, `Mailboxer::Notification`, and
    `Mailboxer::Conversation` to use this convention where possible.
    
    Also use a symbol and/or a hash for `order` where applicable.
    t27duck authored and tapn2it committed Dec 19, 2016
    Copy the full SHA
    9ea3d34 View commit details
  22. Copy the full SHA
    074c088 View commit details
23 changes: 10 additions & 13 deletions .travis.yml
Original file line number Diff line number Diff line change
@@ -3,22 +3,19 @@ cache: bundler
sudo: false

rvm:
- 2

matrix:
include:
- rvm: 1.9.3
gemfile: gemfiles/rails4.1.gemfile
- rvm: 2.0
gemfile: gemfiles/rails4.1.gemfile
- rvm: jruby-19mode
gemfile: gemfiles/rails4.1.gemfile
- 2.1.10
- 2.2.5
- 2.3.1

gemfile:
- gemfiles/rails3.2.gemfile
- gemfiles/rails4.0.gemfile
- gemfiles/rails4.1.gemfile
- gemfiles/rails4.2.gemfile
- gemfiles/rails5.0.gemfile

matrix:
exclude:
- rvm: 2.1.10
gemfile: gemfiles/rails5.0.gemfile

env:
global:
- NOKOGIRI_USE_SYSTEM_LIBRARIES=true
12 changes: 2 additions & 10 deletions Appraisals
Original file line number Diff line number Diff line change
@@ -1,11 +1,3 @@
appraise "rails3.2" do
gem "rails", "~> 3.2.21"
end

appraise "rails4.0" do
gem "rails", "~> 4.0.13"
end

appraise "rails4.1" do
gem "rails", "~> 4.1.13"
appraise "rails4.2" do
gem "rails", "~> 4.2.6"
end
62 changes: 62 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,62 @@
# CHANGELOG

## Master (Unreleased)

### Changed

* When sorting entries by `created_at`, also sort by `id` descending. This
is to ensure proper sorting of items for databases that do not store
nanoseconds for timestamp columns.

### Fixed

* When trying to delete a Mailboxer object, a `NameError` may be thrown due
to a missing namespace

## 0.14.0 - 2016-07-29

### Added

* Rails 5 compatibility.

### Fixed

* `Mailboxer::Message` object no longer requires to have a subject.
* Objects are now saved before mails are sent, you you can use them in the
mailer templates (to build URLs, for example).

### Changed

* Errors are now stored in the parent message/notification instead of being
stored in the sender receipt. That means you need handle mailboxer related
controller and views differently, and study the upgrade case by case (propably
by having a look at mailboxer's source code). As an example, if you were
previously doing something like this in your controller:

```
@receipt = @actor.send_message(@recipients, params[:body], params[:subject])
if (@receipt.errors.blank?)
@conversation = @receipt.conversation
redirect_to conversation_path(@conversation)
else
render :action => :new
end
```

you now need to do something like

```
@receipt = @actor.send_message(@recipients, params[:body], params[:subject])
@message = @receipt.message
if (@message.errors.blank?)
@conversation = @message.conversation
redirect_to conversation_path(@conversation)
else
render :action => :new
end
```

This might look more complicated at first but allows you to build more RESTful
resources since you can build forms on messages and/or conversations and
directly show errors on them. Less specially handling is now required to
propagate errors around models.
8 changes: 0 additions & 8 deletions app/builders/mailboxer/message_builder.rb
Original file line number Diff line number Diff line change
@@ -5,12 +5,4 @@ class Mailboxer::MessageBuilder < Mailboxer::BaseBuilder
def klass
Mailboxer::Message
end

def subject
params[:subject] || default_subject
end

def default_subject
"#{params[:conversation].subject}"
end
end
15 changes: 8 additions & 7 deletions app/models/mailboxer/conversation.rb
Original file line number Diff line number Diff line change
@@ -14,8 +14,8 @@ class Mailboxer::Conversation < ActiveRecord::Base

scope :participant, lambda {|participant|
where('mailboxer_notifications.type'=> Mailboxer::Message.name).
order("mailboxer_conversations.updated_at DESC").
joins(:receipts).merge(Mailboxer::Receipt.recipient(participant)).uniq
order(updated_at: :desc).
joins(:receipts).merge(Mailboxer::Receipt.recipient(participant)).distinct
}
scope :inbox, lambda {|participant|
participant(participant).merge(Mailboxer::Receipt.inbox.not_trash.not_deleted)
@@ -33,10 +33,11 @@ class Mailboxer::Conversation < ActiveRecord::Base
participant(participant).merge(Mailboxer::Receipt.not_trash)
}
scope :between, lambda {|participant_one, participant_two|
joins("INNER JOIN (#{Mailboxer::Notification.recipient(participant_two).to_sql}) `participant_two_notifications` ON `participant_two_notifications`.`conversation_id` = `mailboxer_conversations`.`id` AND `participant_two_notifications`.`type` IN ('Mailboxer::Message')").
joins("INNER JOIN `mailboxer_receipts` ON `mailboxer_receipts`.`notification_id` = `participant_two_notifications`.`id` ").
joins("INNER JOIN (#{Mailboxer::Notification.recipient(participant_two).to_sql}) participant_two_notifications " \
"ON participant_two_notifications.conversation_id = #{table_name}.id AND participant_two_notifications.type IN ('Mailboxer::Message')").
joins("INNER JOIN mailboxer_receipts ON mailboxer_receipts.notification_id = participant_two_notifications.id").
merge(Mailboxer::Receipt.recipient(participant_one)).
order("mailboxer_conversations.updated_at DESC").uniq
order(updated_at: :desc).distinct
}

#Mark the conversation as read for one of the participants
@@ -92,7 +93,7 @@ def originator

#First message of the conversation.
def original_message
@original_message ||= messages.order('created_at').first
@original_message ||= messages.order(:created_at).first
end

#Sender of the last message.
@@ -102,7 +103,7 @@ def last_sender

#Last message in the conversation.
def last_message
@last_message ||= messages.order('created_at DESC').first
@last_message ||= messages.order(:created_at => :desc, :id => :desc).first
end

#Returns the receipts of the conversation for one participants
2 changes: 1 addition & 1 deletion app/models/mailboxer/mailbox.rb
Original file line number Diff line number Diff line change
@@ -9,7 +9,7 @@ def initialize(messageable)
#Returns the notifications for the messageable
def notifications(options = {})
#:type => nil is a hack not to give Messages as Notifications
notifs = Mailboxer::Notification.recipient(messageable).where(:type => nil).order("mailboxer_notifications.created_at DESC")
notifs = Mailboxer::Notification.recipient(messageable).where(:type => nil).order(:created_at => :desc, :id => :desc)
if options[:read] == false || options[:unread]
notifs = notifs.unread
end
15 changes: 8 additions & 7 deletions app/models/mailboxer/message.rb
Original file line number Diff line number Diff line change
@@ -2,7 +2,7 @@ class Mailboxer::Message < Mailboxer::Notification
attr_accessible :attachment if Mailboxer.protected_attributes?
self.table_name = :mailboxer_notifications

belongs_to :conversation, :class_name => "Mailboxer::Conversation", :validate => true, :autosave => true
belongs_to :conversation, :validate => true, :autosave => true
validates_presence_of :sender

class_attribute :on_deliver_callback
@@ -26,16 +26,17 @@ def deliver(reply = false, should_clean = true)
self.clean if should_clean

#Receiver receipts
receiver_receipts = recipients.map { |r| build_receipt(r, 'inbox') }
receiver_receipts = recipients.map do |r|
receipts.build(receiver: r, mailbox_type: 'inbox', is_read: false)
end

#Sender receipt
sender_receipt = build_receipt(sender, 'sentbox', true)

temp_receipts = [sender_receipt] + receiver_receipts
sender_receipt =
receipts.build(receiver: sender, mailbox_type: 'sentbox', is_read: true)

if temp_receipts.all?(&:valid?)
if valid?
save!
Mailboxer::MailDispatcher.new(self, receiver_receipts).call
temp_receipts.each(&:save!)

conversation.touch if reply

40 changes: 15 additions & 25 deletions app/models/mailboxer/notification.rb
Original file line number Diff line number Diff line change
@@ -4,31 +4,30 @@ class Mailboxer::Notification < ActiveRecord::Base
attr_accessor :recipients
attr_accessible :body, :subject, :global, :expires if Mailboxer.protected_attributes?

belongs_to :sender, :polymorphic => :true
belongs_to :notified_object, :polymorphic => :true
belongs_to :sender, :polymorphic => :true, :required => false
belongs_to :notified_object, :polymorphic => :true, :required => false
has_many :receipts, :dependent => :destroy, :class_name => "Mailboxer::Receipt"

validates :subject, :presence => true,
:length => { :maximum => Mailboxer.subject_max_length }
validates :subject, :length => { :maximum => Mailboxer.subject_max_length }
validates :body, :presence => true,
:length => { :maximum => Mailboxer.body_max_length }

scope :recipient, lambda { |recipient|
joins(:receipts).where('mailboxer_receipts.receiver_id' => recipient.id,'mailboxer_receipts.receiver_type' => recipient.class.base_class.to_s)
joins(:receipts).where(:mailboxer_receipts => { :receiver_id => recipient.id, :receiver_type => recipient.class.base_class.to_s })
}
scope :with_object, lambda { |obj|
where('notified_object_id' => obj.id,'notified_object_type' => obj.class.to_s)
where(:notified_object_id => obj.id, :notified_object_type => obj.class.to_s)
}
scope :not_trashed, lambda {
joins(:receipts).where('mailboxer_receipts.trashed' => false)
joins(:receipts).where(:mailboxer_receipts => { :trashed => false })
}
scope :unread, lambda {
joins(:receipts).where('mailboxer_receipts.is_read' => false)
joins(:receipts).where(:mailboxer_receipts => { :is_read => false })
}
scope :global, lambda { where(:global => true) }
scope :expired, lambda { where("mailboxer_notifications.expires < ?", Time.now) }
scope :expired, lambda { where("#{Mailboxer::Notification.quoted_table_name}.expires < ?", Time.now) }
scope :unexpired, lambda {
where("mailboxer_notifications.expires is NULL OR mailboxer_notifications.expires > ?", Time.now)
where("#{Mailboxer::Notification.quoted_table_name}.expires is NULL OR #{Mailboxer::Notification.quoted_table_name}.expires > ?", Time.now)
}

class << self
@@ -81,11 +80,14 @@ def expire
#Use Mailboxer::Models::Message.notify and Notification.notify_all instead.
def deliver(should_clean = true, send_mail = true)
clean if should_clean
temp_receipts = recipients.map { |r| build_receipt(r, nil, false) }
temp_receipts = recipients.map do |r|
receipts.build(receiver: r, mailbox_type: nil, is_read: false)
end

if temp_receipts.all?(&:valid?)
if valid?
Mailboxer::MailDispatcher.new(self, temp_receipts).call if send_mail
temp_receipts.each(&:save!) #Save receipts
save!

self.recipients = nil
end

@@ -177,16 +179,4 @@ def object
def sanitize(text)
::Mailboxer::Cleaner.instance.sanitize(text)
end

private

def build_receipt(receiver, mailbox_type, is_read = false)
Mailboxer::ReceiptBuilder.new({
:notification => self,
:mailbox_type => mailbox_type,
:receiver => receiver,
:is_read => is_read
}).build
end

end
39 changes: 14 additions & 25 deletions app/models/mailboxer/receipt.rb
Original file line number Diff line number Diff line change
@@ -2,9 +2,9 @@ class Mailboxer::Receipt < ActiveRecord::Base
self.table_name = :mailboxer_receipts
attr_accessible :trashed, :is_read, :deleted if Mailboxer.protected_attributes?

belongs_to :notification, :class_name => "Mailboxer::Notification", :validate => true, :autosave => true
belongs_to :receiver, :polymorphic => :true
belongs_to :message, :class_name => "Mailboxer::Message", :foreign_key => "notification_id"
belongs_to :notification, :class_name => "Mailboxer::Notification"
belongs_to :receiver, :polymorphic => :true, :required => false
belongs_to :message, :class_name => "Mailboxer::Message", :foreign_key => "notification_id", :required => false

validates_presence_of :receiver

@@ -13,24 +13,27 @@ class Mailboxer::Receipt < ActiveRecord::Base
}
#Notifications Scope checks type to be nil, not Notification because of STI behaviour
#with the primary class (no type is saved)
scope :notifications_receipts, lambda { joins(:notification).where('mailboxer_notifications.type' => nil) }
scope :messages_receipts, lambda { joins(:notification).where('mailboxer_notifications.type' => Mailboxer::Message.to_s) }
scope :notifications_receipts, lambda { joins(:notification).where(:mailboxer_notifications => { :type => nil }) }
scope :messages_receipts, lambda { joins(:notification).where(:mailboxer_notifications => { :type => Mailboxer::Message.to_s }) }
scope :notification, lambda { |notification|
where(:notification_id => notification.id)
}
scope :conversation, lambda { |conversation|
joins(:message).where('mailboxer_notifications.conversation_id' => conversation.id)
joins(:message).where(:mailboxer_notifications => { :conversation_id => conversation.id })
}
scope :sentbox, lambda { where(:mailbox_type => "sentbox") }
scope :inbox, lambda { where(:mailbox_type => "inbox") }
scope :trash, lambda { where(:trashed => true, :deleted => false) }
scope :not_trash, lambda { where(:trashed => false) }
scope :deleted, lambda { where(:deleted => true) }
scope :not_deleted, lambda { where(:deleted => false) }

#TODO Patched Victor Christensen at 2:40 PM on 6/11/16
# patched line below to deal with a conflict with permanent_records gem; method with same name
# scope :deleted, lambda { where(:deleted => true) }
# scope :not_deleted, lambda { where(:deleted => false) }

scope :is_read, lambda { where(:is_read => true) }
scope :is_unread, lambda { where(:is_read => false) }

after_validation :remove_duplicate_errors
class << self
#Marks all the receipts from the relation as read
def mark_as_read(options={})
@@ -73,12 +76,8 @@ def move_to_sentbox(options={})
end

def update_receipts(updates, options={})
ids = where(options).map { |rcp| rcp.id }
unless ids.empty?
sql = ids.map { "#{table_name}.id = ? " }.join(' OR ')
conditions = [sql].concat(ids)
Mailboxer::Receipt.where(conditions).update_all(updates)
end
ids = where(options).pluck(:id)
Mailboxer::Receipt.where(:id => ids).update_all(updates) unless ids.empty?
end
end

@@ -140,16 +139,6 @@ def is_trashed?

protected

#Removes the duplicate error about not present subject from Conversation if it has been already
#raised by Message
def remove_duplicate_errors
if errors["mailboxer_notification.conversation.subject"].present? and errors["mailboxer_notification.subject"].present?
errors["mailboxer_notification.conversation.subject"].each do |msg|
errors["mailboxer_notification.conversation.subject"].delete(msg)
end
end
end

if Mailboxer.search_enabled
searchable do
text :subject, :boost => 5 do
8 changes: 0 additions & 8 deletions gemfiles/rails3.2.gemfile

This file was deleted.

7 changes: 0 additions & 7 deletions gemfiles/rails4.0.gemfile

This file was deleted.

7 changes: 0 additions & 7 deletions gemfiles/rails4.1.gemfile

This file was deleted.

7 changes: 7 additions & 0 deletions gemfiles/rails5.0.gemfile
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
# This file was generated by Appraisal

source "https://rubygems.org"

gem "rails", "~> 5.0.0"

gemspec path: "../"
Loading