Why is this sometimes wrong?
Specifically the "last published on... by..." can be old or incorrect.
This is how it gets populated, for that view:
@latest_published_version = @hymnal_ad.published_versions.lastAnd let's look at a single published_versions.last....
> ad = ad
> ad.published_versions.last.id
=> 60571
Okay. Sure, why not?
Let's check them all:
> ad.published_versions.pluck(:id)
=> [60571, 60680, 60850, 63085]
Well, that seems a bit suspicious; I would have thought 63085 was the last one.
Hrm, let's try sorting explicitly...
> ad.published_versions.sort_by { |pv| pv.created_at }.last.id
=> 63085
Oh, well...that's the one we really want.
Well, fine, we can explicitly order in the relation, to make sure we're sorting by PublishedAd.created_at
has_many :published_versions,
-> { order(:created_at => :asc, :id => :asc) }
But... why is .last wrong? What else is going on?
> ad.published_versions.first.id
=> 60571
> ad.published_versions.last.id
=> 60571
Well, that's just weird.
What about this?
> ad.published_versions.last(1).map(&:id)
=> [63085]
Why is that different?! Why is .last different than .last(1)?
Let's go spelunking!
Let's start looking into .published_versions, and what happens with .last:
> ad.published_versions.class
=> PublishedAd::ActiveRecord_Associations_CollectionProxy
So we start at CollectionProxy
def last(*args)
@association.last(*args)
endThen we head into Association:
collection_association L128-130
def last(*args)
first_nth_or_last(:last, *args)
endcollection_association L632-640
# Fetches the first/last using SQL if possible, otherwise from the target array.
def first_nth_or_last(type, *args)
args.shift if args.first.is_a?(Hash) && args.first.empty?
collection = fetch_first_nth_or_last_using_find?(args) ? scope : load_target
collection.send(type, *args).tap do |record|
set_inverse_instance record if record.is_a? ActiveRecord::Base
end
endWe're calling this method, and type is :last; and args is either [], or [1]; depending on whether we're calling .last or .last(1)
Then we're calling send on whatever collection is; either scope or load_target. (I'm exhausted already.)
Which is it? What does that fetch_first_nth_or_last_using_find? return? Let's use some ruby magic to find out (if you think calling private methods using .send(:symbol) is hacking ruby, have fun with instance_variable_get).
> ad.published_versions.instance_variable_get("@association").send(:fetch_first_nth_or_last_using_find?, [:last])
=> true
Let's not worry about why; let's just keep digging. So it returns true, and collection = scope, and so we're (essentially) calling scope.last(*args)
and if we look at scope, it's:
> ad.published_versions.instance_variable_get("@association").send(:scope).class
=> PublishedAd::ActiveRecord_AssociationRelation
Confirmed, then: We're .sending [:last] (or [:last, 1], depending) to ActiveRecord_AssociationRelation.
And that looks like this:
def last(limit = nil)
if limit
if order_values.empty? && primary_key
order(arel_table[primary_key].desc).limit(limit).reverse
else
to_a.last(limit)
end
else
find_last
end
end(which we can find by doing grep -r 'def last(' inside the active_record gem.)
So, when we call .published_versions.last(1), we go into the first branch. Let's just have a quick check on order_values.empty? (which I assume has something to do with whether or not the relation is already ordered):
> ad.published_versions.send(:order_values).empty?
=> false
We end up with just: .published_versions.to_a.last(1), which is how we get the "right" answer:
> ad.published_versions.to_a.last(1).map(&:id)
=> [63085]
:thumbs-up:
But, when we call .published_versions.last, we're calling the finder_method above, with a nil param, so we take the second branch to find_last:
def find_last
if loaded?
@records.last
else
@last ||=
if limit_value
to_a.last
else
reverse_order.limit(1).to_a.first
end
end
endAnd here, we just end up in the last branch because of reasons:
> ad.published_versions.loaded?
=> false
> ad.published_versions.send(:limit_value)
=> nil
So .published_versions.last is the same as .published_versions.reverse_order.limit(1).to_a.first
Which makes sense abstractly: the "last" object is the same as the "first" object after you reverse them all.
...except it isn't; we saw that .first.id and .last.id were the same.
So...
> ad.published_versions.map(&:id)
=> [60571, 60680, 60850, 63085]
> ad.published_versions.reverse_order.map(&:id)
=> [60571, 60680, 60850, 63085]
The list is the same when it's reversed! Palindrome-tastic!
But, why?
> ad.published_versions.to_sql
=> "SELECT `published_ads`.* FROM `published_ads` INNER JOIN `operations` ON `published_ads`.`last_operation_id` = `operations`.`id` WHERE `operations`.`hymnal_ad_id` = 1337 ORDER BY `operations`.`created_at` ASC, `operations`.`id` ASC"
> ad.published_versions.reverse_order.to_sql
=> "SELECT `published_ads`.* FROM `published_ads` INNER JOIN `operations` ON `published_ads`.`last_operation_id` = `operations`.`id` WHERE `operations`.`hymnal_ad_id` = 1337 ORDER BY `operations`.`created_at` DESC, `operations`.`id` DESC"
That seems sensible, we're (by default) using the order specified on HymnalAd.operations.
> ad.published_versions.map(&:last_operation_id)
=> [26549, 26549, 26549, 26549]
Ah, all of the published versions are from the same version of the ad!
So... if we sort them using the default ordering (via the operations), then they're essentially not being ordered at all, because they all relate to the same Operation.
So... even if we "reverse the order", they stay the same.
So... when we call .last (which is the same as .reverse_order.first), it's as if we're calling .first
So... .first.id and .last.id are the same, and we get the value we don't expect for .last.
So... we needed to be explicit about the ordering anyway, but now we see why: because we need to be able to sensibly order published_versions that correspond to the same Operation.)
