Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Record rendered too early when adding validation #367

Closed
Tao-Galasse opened this issue Jan 29, 2020 · 6 comments
Closed

Record rendered too early when adding validation #367

Tao-Galasse opened this issue Jan 29, 2020 · 6 comments

Comments

@Tao-Galasse
Copy link

Tao-Galasse commented Jan 29, 2020

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
Copy link
Owner

brendon commented Jan 29, 2020

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
Copy link
Author

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
Copy link
Owner

brendon commented Jan 30, 2020

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
Copy link
Author

Tao-Galasse commented Jan 30, 2020

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
Copy link
Owner

brendon commented Jan 30, 2020

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
Copy link
Owner

brendon commented Jun 4, 2024

Closing due to inactivity.

@brendon brendon closed this as completed Jun 4, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants