ranked-model icon indicating copy to clipboard operation
ranked-model copied to clipboard

Relative re-ordering?

Open alexhanh opened this issue 12 years ago • 12 comments

Does ranked-model support something like move_higher and move_lower like the acts_as_list gem does?

alexhanh avatar Dec 06 '12 15:12 alexhanh

@alexhanh no not explicitly. You should be able to foo.update_attribute :age_position, foo.age_position-1. But nothing built in.

mixonic avatar Dec 21 '12 19:12 mixonic

How should foo be loaded? I had issues trying to do this sort of thing, as in the following code.

For the below I have answers belonging to answer-sets, with answers ranked per answer-set & using 'sequence' (the rank name) as their default scope ordering.

class Answer < ActiveRecord::Base
  include RankedModel
  ranks :sequence, :with_same => :answer_set_id
  default_scope order('answer_set_id ASC, sequence ASC')
end

# in some spec w/ machinist 1: 
  @a, @b, @c, @d = (1..4).map{answer_set.answers.make}
  @d.update_attribute :sequence_position, @c.sequence_position
  answer_set.answers(true).map(&:id).should == [@a.id, @b.id, @d.id, @c.id]
#  =>       expected: [9361, 9362, 9364, 9363]
#            got: [9361, 9362, 9363, 9364] (using ==)

  puts @c.sequence_position.inspect
#  => :last

  puts @c.reload.sequence_position.inspect
#  => :last   

  puts Answer.find(@c).sequence_position.inspect
#  => nil

Actually all of the elements think they are :last, which I guess they were when they were created? They don't seem to update. Is the virtual-attribute not working here because I didn't load the model instances through rank? It seems like it should load/reload the position somehow, or raise an exception.

Also undefined method -' for :last:Symbol` is a catch with the suggested move-lower.

nruth avatar Feb 04 '13 11:02 nruth

This is a problem I've just come across when trying to use this gem. As far as I can tell, models loaded from the database have no idea what their own position within the list as the *_position accessor is nil until set by the code that writes the position to the DB.

I don't think there is a way to determine the item's current position without doing an SQL query. Which would be inefficient if you wanted positions from a collection of items.

Odaeus avatar Mar 14 '13 16:03 Odaeus

@nruth I've not used default_scope with ranked model, the behavior is unknown to me. I don't see any specs specifically testing setting the position of another ranked model, but there are complete specs for setting a position.

@Odaeus I think your issues is un-related? Individual ranked models do not know their position, that is correct. That they do not know their position is actually a feature, as that way ranked-model does not need to update lots of rows when a single item's position changes. See the "Internals" section in the Readme.

Please, keep this issue on topic: Adding move_higher and move_lower methods.

mixonic avatar Mar 15 '13 18:03 mixonic

I have a similar issue, but siblings_position is always nil for me:

undefined method-' for nil:NilClass`

I'm trying to set the position of a new item, relative to another item (both are siblings, because they share a parent_id)

item.siblings_position = other.siblings_position - 1

I don't have any default_scope and all siblings have a position set.

> Item.find(2).siblings_position
  Item Load (0.4ms)  SELECT "items".* FROM "items" WHERE "items"."id" = $1 LIMIT 1  [["id", 2]]
 => nil
> Item.find(2).position
  Item Load (0.4ms)  SELECT "items".* FROM "items" WHERE "items"."id" = $1 LIMIT 1  [["id", 2]]
 => 3932160

leonelgalan avatar Mar 18 '13 20:03 leonelgalan

@mixonic I brought it up because unless I'm really confused (not unheard of) it means your initial suggestion (foo.update_attribute :age_position, foo.age_position-1) will only work if the object has just been saved with a new position. As @leonelgalan seems to have confirmed... I think :)

Odaeus avatar Mar 18 '13 20:03 Odaeus

Based on the discussion in #10 I wrote this:

  def calculated_siblings_position
    self.self_and_siblings.where('position < ?', self.position).count
  end 

@mixonic make it clear on that issue that count might have trouble with big tables. I was wondering if we could use something like this, when *_position is nil, or if anyone has a "less expensive" way of retrieving the position.

leonelgalan avatar Mar 19 '13 13:03 leonelgalan

@Odaeus @leonelgalan it is absolutely correct that a record cannot determine its position without being saved. RM isn't very good at managing positions- only an order of items. If you need to know position and not just the order of fetched records (or a record at a position, which RM could do quickly with .offset), it might not be the best solution.

That said, it should be fairly trivial to implement .position_after and .position_before methods for models. I suppose this is already a feature request for .move_higher and .move_lower, and these would be similar. If you set the rank attribute of an instance to the rank of another model, it should already operate as .position_before:

me.queue_rank # => say, 50.
poor_dude.queue_rank # => say, 30.

me.queue_rank = poor_dude.queue_rank
me.save

me.reload.queue_rank # => 30
poor_dude.reload.queue_rank # => 31

This is not ideal, because it uses an update_all instead of just positioning me halfway between poor_dude and his lower neighbor, but it does work.

mixonic avatar Mar 19 '13 16:03 mixonic

Doesn't your example assume that I already know the relative positions of me and poor_dude?

Relative positioning seems like an obvious use case, esp. given the direct comparison to acts_as_list in the readme. I just assumed this was supported (which is not your fault, obviously).

I understand that leaving relative positioning out is consistent with treating speed as a top priority, so perhaps the solution is to make it clear in the readme that relative positions aren't supported?

sirvine avatar Jun 17 '13 01:06 sirvine

@sirvine To set me before poor_dude, poor_dude must indeed know its position. me does not.

I don't think RM shouldn't or can't support relative positions, I just haven't had the time to write the code :-p Hence this being marked as a new feature.

mixonic avatar Jun 17 '13 19:06 mixonic

Fair enough. I'd be glad to help, but I'm afraid my solutions so far aren't terribly useful at scale. Looking at the acts_as_list implementation, it's clear that it's both an update_all operation, and relies on the position values being sequential. As a result, I thought about converting the ranked collection into a hash with key of object id and value of position (or, to be more precise, the index of a map enumerator).

Of course, after mapping that hash, I'd have to use the id key to find the object so that I can pull the rank value of the upward or downward sibling. Under those circumstances, it's quite possible to avoid an update all, unless the next sibling in line were now out of sequence.

I suppose that's a very naive approach, but it was the best I was able to come up with so far.

sirvine avatar Jun 17 '13 19:06 sirvine

Have you found any good solution to sort above/below a given element? At least one that doesn't require an update_all or so. I mean, how can we do taking performance into account to add an element below/above another one?

arielscherman avatar Sep 15 '17 14:09 arielscherman

Closing due to inactivity. This is still one of the downsides of the gem. Happy to look at a fresh PR that addresses this.

brendon avatar Jun 04 '24 04:06 brendon