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 74 forks source link

Store origin data alongside localized data #114

Closed ryanmitchell closed 1 year ago

ryanmitchell commented 1 year ago

As discussed here, we should store origin entry data alongside localisation data so it can be queried at the database level.

Let me know if youre happy with this approach as I'll need to add some test coverage on this before it should be merged.

ryanmitchell commented 1 year ago

Hmm actually this will only work on 1st save, after that localizable_fields will contain everything, which isnt right. What's the best way to get the fields that are actually localised - through the blueprint?

FrittenKeeZ commented 1 year ago

@ryanmitchell there also seems to be an issue with date not being updated for localized entries when the origin is changed and the date field isn't localizable

what-the-diff[bot] commented 1 year ago
ryanmitchell commented 1 year ago

@FrittenKeeZ yep, I was battling the tests a bit so moved this to draft. Should be working now with the latest commit.

FrittenKeeZ commented 1 year ago

@ryanmitchell @jasonvarga we're facing rather big performance issues with blueprint being stored in json instead of its own field, so I just wanted to hear you out if you're okay with pulling blueprint out of json with an index so queries don't take 5+ seconds to complete?

FrittenKeeZ commented 1 year ago

@ryanmitchell do you have any sense of whether this is ready for use? The missing blueprint on translated entries are causing us big issues currently, so I'm torn between using this patch to get the missing data or creating a PR for storing blueprint in a separate field in the database for improving performance as well.

ryanmitchell commented 1 year ago

As far as I can see it is, but Jason may feel differently

FrittenKeeZ commented 1 year ago

We just talked it through with the team and I'll be making a PR to fix #126 soon, as it will fix three issues we have regarding performance, code complexity and data duplication for blueprint.

FrittenKeeZ commented 1 year ago

@ryanmitchell I can see I added some additional handling regarding parent, but I can't remember whether it was an issue or I just added it as a precaution. https://github.com/statamic/eloquent-driver/pull/144/files#diff-31fcf8a95fd0dbc8da9e8dc7ae856aa51bd921c50a8059e23df7d411737ab65fR71

ryanmitchell commented 1 year ago

Yeah we're not storing parent going forward - its redundant as we have origin as a column.

FrittenKeeZ commented 1 year ago

Yeah we're not storing parent going forward - its redundant as we have origin as a column.

It's not quite the same, especially when it comes to multisite. Origin is the fallback entry, but parent is the entry in the tree hierarchy which differs for each site.

ryanmitchell commented 1 year ago

Ah right, so german might be the parent of french, but english is the fallback? Ok, storing the ID is fine (as your PR did) - I think until this PR its storing the whole entry which is a lot.

FrittenKeeZ commented 1 year ago

Ah right, so german might be the parent of french, but english is the fallback? Ok, storing the ID is fine (as your PR did) - I think until this PR its storing the whole entry which is a lot.

Not quite... each site / language is its own regarding parents, like so:

                 Origin
Contact (EN)  - - - - - - - Kontakt (DE)
    |                         |
    | Parent                  | Parent
    |            Origin       |
About US (EN) - - - - - - - Über Uns (DE)
ryanmitchell commented 1 year ago

@FrittenKeeZ are you happy that this covers everything you found now?

ryanmitchell commented 1 year ago

Thanks @FrittenKeeZ