brendon / acts_as_list

An ActiveRecord plugin for managing lists.
http://brendon.github.io/acts_as_list/
MIT License
2.04k stars 355 forks source link

What's the correct behavior for add_new_at: nil ? #332

Closed democlitos closed 3 months ago

democlitos commented 5 years ago

The description says that add_new_at: nil "will result in new items not being added to the list on create, i.e, position will be kept nil after create", but it not says that subsequent scope changes will not change it's position. What should be the correct behavior?

Some context: I have a scope acts_as_list scope: [:distribution_center, state: :preparation_queue], and I'm having trouble adding elements to this list. I noticed that when a record satisfy this scope but already have a position value lower then the bottom element it duplicates the correspondent value on the list before sending the element to bottom. So I just put all positions to null when it's not on the list, and the one case that is missing is when a new element is created.

brendon commented 5 years ago

Hi @democlitos, I don't quite follow the problem you're having. Could you post some code to look at? From the code you did post I noticed that you have a hash in your scope array. As far as I know, the values in the hash need to be fixed (they're not calculated at runtime). I could be wrong, but the example in the README has a fixed value.

democlitos commented 5 years ago

Hi @brendon. So If I use acts_as_list scope: [:distribution_center, state: "preparation_queue"] should be any different? I call acts_as_list scope on my order.rb Model that has this states:

aasm :whiny_transitions => false, :column => 'state' do
    state :pending, :initial => true
    state :preparation_queue,
      :preparation,
      :error,
      :packaging_queue,
      :packaging,
      :ready,
      :canceled

    event :add_to_preparation_queue do
      transitions from: [:pending, :error], to: :preparation_queue do
        guard do
          has_valid_distribution_center
        end
      end
    end

    event :go_to_error do
      before :unassign_employee
      transitions from: [:pending, :preparation, :preparation_queue, :packaging_queue, :packaging, :ready], to: :error
      after :dispatch_issues
    end

    event :start_preparation do
      transitions from: :preparation, to: :preparation
      transitions from: :preparation_queue, to: :preparation do
        guard { |id| assign_employee(id) }
      end
    end
    # ...

orders_controller:

def preparation
    @order = Order.preparation(current_employee.id, order_params[:distribution_center])
    authorize @order if @order.present?
    if @order && @order.start_preparation!(current_employee.id)
      render json: @order, include: ['*.*']
    else
      render status: 204
    end
  end

The problem occurs when I call preparation (from orders_controller), that should remove my order from the acts_as_list scope, as state would be equal to "preparation", and not "preparation_queue". Instead I have to call the next step, doing state = "packaging_queue" to update the list correctly. I tried to fix this by calling directly order.remove_from_list, but another issues appeared. What you think?

brendon commented 5 years ago

Hi @democlitos, I think I kind of get what you're talking about. One thing I thought you could try would be:

acts_as_list scope: [:distribution_center, :state]

Even if you don't care about ordering other states, this should allow you to have a unique list per distribution centre and state. Acts as list should handle the transition between those lists with its callbacks.

Give that a try and let me know how you get on.

democlitos commented 5 years ago

Didn't work. Same behavior. Do you think that is possible that the aasm transitions is blocking the acts_as_list callbacks?

brendon commented 5 years ago

That could be the case. From here I'd recommend looking at the SQL logs to try and figure out what exactly is happening. I think there's a tool you can use that links the SQL to the lines of code.

brendon commented 3 months ago

Closing due to inactivity.