Extend tiebreaking to all association orderings

[goosys]

Previously, tiebreaking by primary key was introduced for direct column ordering.
However, association-based orderings (has_many, belongs_to, has_one) did not benefit from the same behavior.

This PR extends tiebreaking to all association orderings by extracting the logic into a shared with_tiebreak helper. Each association ordering method now passes its result through with_tiebreak, which appends an ORDER BY primary_key clause when applicable.

No tiebreak is added when the parent relation has no primary key.

[goosys]

@pablobm
Rebased

[pablobm]

Having to do tiebreak_key = relation.primary_key twice smells a bit wrong to me. I wonder if we can do this differently, perhaps making these functions a bit more generic and less tied to the concept of tiebreak. Perhaps even usable elsewhere in the class... For now, would this make sense?

Suggested change

	def with_tiebreak(relation, ordered_column: nil)
	tiebreak_key = relation.primary_key
	tiebreak_order = tiebreak_order_for(relation)
	if tiebreak_order && ordered_column.to_s != tiebreak_key.to_s
	relation.order(tiebreak_order)
	else
	relation
	end
	end
	def tiebreak_order_for(relation)
	tiebreak_key = relation.primary_key
	if tiebreak_key && column_exist?(relation, tiebreak_key)
	relation.arel_table[tiebreak_key].public_send(direction)
	end
	end
	def with_tiebreak(relation, ordered_column: nil)
	tiebreak_key = relation.primary_key
	if ordered_column.to_s != tiebreak_key.to_s
	sort_by(relation, tiebreak_key)
	else
	relation
	end
	end
	def sort_by(relation, column)
	if column && column_exist?(relation, column)
	order = relation.arel_table[column].public_send(direction)
	relation.order(order)
	else
	relation
	end
	end

This breaks some examples in spec/lib/administrate/order_spec.rb, but they are testing the private method which is also a bad smell anyway.

[goosys]

Thanks for the suggestion! One thing I like about keeping tiebreak_order_for is that it clearly separates the tiebreak-specific logic. I’m not sure introducing a generic sort_by helper is the right abstraction, since its current purpose is still specifically to add the tiebreak order.

I extracted tiebreak_order_for because developers may want to extend Administrate::Order and customize how ties are resolved in their own order classes. I agree that fetching the primary key in both with_tiebreak and tiebreak_order_for feels awkward, though—so I'd even consider going further and extracting tiebreak_key into its own method too, so it can be overridden independently. What do you think?