Add tests for moving within scope and add method: move_within_scope

[philippfranke]

Addressed to #67 and #78

/cc @conzett

[swanandp]

Pulled, thanks!

[conzett]

@philippfranke Does this work with array scopes? The scope_name function introduced in #67 doesn't seem to take this into account, something I'm currently working through in #79

[philippfranke]

Unfortunately,I've just noticed it's not working properly with array scope. I missed that case. How about implementing: method scope_query instead of scope_name. In addition, we should change move_within_scope because of its attributes.

@conzett: Are you interested in implementing these changes?

[conzett]

@philippfranke I'm currently working towards something similar. We already have scope_condition available which you might be able to use for move_within_scope. I'm working on check_scope currently and I am finding myself in need of the raw scope object itself rather than a query string. If I get check_scope figured out move_within_scope should be fairly easy to update as well if scope_condition doesn't work for that.

[conzett]

So in short, yes I'll be attempting to implement them 😄

[conzett]

@philippfranke What was the use case for adding the move_within_scope method?

[philippfranke]

@conzett Like set_list_position; Just a much more comfortable way to update scope_id

[conzett]

I realize this is already merged but I'm not really sure how much that this method adds overall since it only works when you have a scope comprised of a single property. What should the behavior be when you have an array scope or a scope that relies on string interpolation? I think it might just be easier to use update_attributes on the model instance if a user wishes to change the scope. I would also suggest a name change perhaps as move_within_scope sounds like you're moving the object around in it's current scope rather than changing the scope itself. Let me know your thoughts.

[philippfranke]

I couldn't agree more, move_within_scope's name is confusing. In my opinion, it should behave and be implemented similar to scope_condition but for assignment.
What's your opinion?

[conzett]

@philippfranke I think to keep it consistent with the other methods already available we could call it move_to_list. You could pass either a single argument like your original implementation, or you could pass a hash for more complex scopes.

I also I saw in your original implementation you automatically moved it to the bottom of the list (similar to how creating a new item starts it out at the bottom of the list) Would you still like to see this behavior as well?

[philippfranke]

No, my original implementation moved it to the bottom or top of the list - depends on add_new_at. It makes sense to keep this behavior.

[conzett]

@philippfranke That's right! Sorry I forgot that was configurable, good point.