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

Double saving on registerAsLove{Reacter/Reactant} #154

Open
volkv opened this issue Feb 18, 2020 · 2 comments · May be fixed by #249
Open

Double saving on registerAsLove{Reacter/Reactant} #154

volkv opened this issue Feb 18, 2020 · 2 comments · May be fixed by #249
Assignees
Milestone

Comments

@volkv
Copy link

volkv commented Feb 18, 2020

Hi @antonkomarev ! just a question. why would we need to registerAsLove{Reacter/Reactant} on Created event observer? It causes double saving event firing and DB operation.
Can we hook on Creating event and just pass reacter/reactant_id before model is created?
It seems to be working for me. Just wondering is there can be any issues with such approach?

@antonkomarev
Copy link
Member

antonkomarev commented Feb 18, 2020

Good call! I don't remember right now why I made it on created event, but there definitely were a reasons. Will return back to this question in a free time.

If it will be done on creating event - we will be able to use non-nullable fields which you have asked for:

@antonkomarev antonkomarev added this to the v9.0.0 milestone Feb 18, 2020
@antonkomarev antonkomarev modified the milestones: v9.0, v10.0 Apr 9, 2023
@antonkomarev
Copy link
Member

Sorry for the long delay @volkv.

I created a MR which implements what you've said before (and it works):

I found time to dive deeper to this question and tried to remember the reason why this this decision was made.

My motivation was simple: if creation of the Reactable/Reacterable model will fail, we will have empty Reactant/Reacter model not attached to anything.

Scenario:

  1. Reacterable/Reactable model save called (creating observer method called)
  2. Reacter/Reactant model created and its id set (but not persisted yet) to the Reacterable/Reactable model
  3. Something bad happens (database connection issue, the electricity was cut off, etc)

But I think it may be solved by adding a console command, which will find all orphan Reactant/Reacter models and delete them.

What do you think about it?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
2 participants