search

statamic / eloquent-driver

Provides support for storing your Statamic data in a database, rather than flat files.
https://statamic.dev/tips/storing-content-in-a-database
MIT License
104 stars 75 forks source link

Don't double-save descendants #193

Closed jasonvarga closed 1 year ago

jasonvarga commented 1 year ago

In statamic/cms#8505 descendants will be saved. There is no longer a need to do it from the Eloquent repository side. It would only cause them to be saved twice.

I'm bumping the requirement of cms to the version where that PR would be available.

This is not currently an issue, but will be once that PR is released. It's only minor - the entries would be double saved, and EntrySaved events dispatched multiple times. Database update queries wouldn't even run since the model would have no changes the second time.

jasonvarga commented 1 year ago

The composer constraint is going to be wrong. It'll need to be updated before this gets merged.

jasonvarga commented 1 year ago

This PR will fail until #178 is completed.

ryanmitchell commented 1 year ago

Seems like a couple of the entry tests are failing now as the data isn't propagating to the database: it_propagates_origin_date_to_descendent_models and it_propagates_updating_origin_data_to_descendent_models

Whats strange is that if I add the same line of code back, it works, and if I add $entry->directDescendants()->each->save(); to line 203 of EntryTest it works. I cant see anything in the save() methods that would be causing it to need a double save.

Do you have any thoughts?

jasonvarga commented 1 year ago

Looks like it was because the test was saving through the repository rather than the entry. The descendant-saving logic is happening on the entry now. It seems like the the reason the repository was maybe just a copy paste error from somewhere else, because the entry is saved normally a few lines earlier.