brendon / acts_as_list

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

Record rendered too early when adding validation #367

Closed Tao-Galasse closed 5 months ago

Tao-Galasse commented 4 years ago

Hi !

I encountered a quite hard-to-explain bug with acts_as_list. I have a plant which has_many plant_stages, so PlantStage acts_as_list scope: :plant. When I update a plant to change position of a plant_stage in my controller, everything's fine : the plant and the plant_stages rendered by the api are correctly updated.

But, when I add a validation on my Plant model to ensure a plant has at least one plant_stage, something wrong happen : the plant and the plant_stages rendered by the api are not correct : the positions are not updated. BUT, in the database, everything's fine, so it seems like the plant in the controller is rendered BEFORE the plant_stages have been updated. (I hope I've been clear).

Here is my controller method:

  def update
    if @plant.update(update_params)
      render json: @plant.to_blueprint # this is for serializing the plant with its plant_stages
    else
      render_validation_error(@plant)
    end
  end

What I think is the positions of the plant_stages are updated after the update method on @plant. So @plant is rendered when it has been updated, but before the plant_stages positions have been updated, so the api is rendering wrong data even if it's correct in the database.

any help would be greatly appreciated 🙏 (if it wasn't clear / need more details, just tell me 😃)

brendon commented 4 years ago

Hi @Tao-Galasse, acts_as_list does all its work in an after_ callback so that would explain what you're seeing I guess. I can't think if anything else to help unfortunately. Let me know if you find out any more :)

Tao-Galasse commented 4 years ago

Thanks for the quick answer :) But I don't think this is the only explanation, because the issue doesn't occure when I don't have my validation 🤔 It was working perfectly, but when I added validates :plant_stages, length: { minimum: 1 } on my Plant model, the problem appeared. Isn't it something else which could explain this?

brendon commented 4 years ago

I honestly have no idea :D If it was me, I'd be using breakpointing (or just raise @plant.inspect to check the status of the various objects involved at various points in the code. Watch the logs too to see how the SQL is being executed. A validation failure shouldn't result in the record being committed to the database.

Are you using Rails 5? If not, they seem to have fixed some inaccuracies with regards to relation counting: https://github.com/rails/rails/blob/master/activerecord/lib/active_record/validations/length.rb#L3-L10

Tao-Galasse commented 4 years ago

The point is that the validation doesn't fail :/ This is exactly what seems odd to me. The validation ensures I always add at least one plant_stage, and it's true in this case ; I just tried to change the positions, not to delete them.

I checked my logs too and the SQL transactions are the same with or without the validation. The point here is in the controller: it seems like @plant.update(update_params) returns true before the plant_stages positions are persisted, but only if I add a validation. (I also tried to define a custom validation on the association and I have the same results).

I'm using Rails 6.

brendon commented 4 years ago

That's very strange. Definitely sounds like a bad interaction between rails and acts_as_list. Would you mind starting a PR with a failing test for this scenario so we can see how it plays out across rails versions? That might provide a clue.

brendon commented 5 months ago

Closing due to inactivity.