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

Split configs for asset containers, collection trees and navigation trees #133

Closed piljac1 closed 1 year ago

piljac1 commented 1 year ago

Fixes https://github.com/statamic/eloquent-driver/issues/147

This small pull request enables the possibility to define a different driver for collections and collection trees.

This can be useful if you want your collection definitions to be stored as flat files (so they're available and synced throughout all environments), but have eloquent entries which has a direct impact on the tree structure (which would not be the same in all environments).

All tests are passing, but I saw no config-based tests I could alter or enrich. However, it was tested in all 5 possible configurations on a local site:

Let me know if you need me to do some changes !

P.S. This is a non breaking change, so people that don't have the new key in their published config file won't be affected.

Second P.S. My intent is to do the same kind of changes where a split would be possible for other "models" (i.e. navigations) without introducing breaking changes.

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

PR Summary

piljac1 commented 1 year ago

@ryanmitchell Thanks for the feedback! I just applied the changes. Sorry for the long wait, I have tons of work right now haha.

ryanmitchell commented 1 year ago

@piljac1 sorry for the delay in looking at this.

I've pushed a change in approach. It checks that the tree_model config exists, which if it does shows that we're using the legacy keys, and so should defer any config calls to those. With the previous approach it was deferring to the collection config which when using the new config may not be what you want.

I've also gone ahead and added support for splitting navs and trees, as well as asset containers and assets which makes sense as part of the same change.

Let me know what you think.

piljac1 commented 1 year ago

@ryanmitchell Sounds good! I also aimed at splitting navs and other things, but I attempted to split navs briefly last week (applied the same logic that was done with collections) and it didn't seem to work properly (exceptions were encountered). Have you tested the nav changes you've implemented in a Statamic project (note that I tested my fix attempt on a Statamic v3.3 project and not a v4+, so it might work properly on v4+)?

It worked fine when both the nav and nav trees were set to eloquent, but if I set nav to file and nav trees to eloquent, it failed.

ryanmitchell commented 1 year ago

I've pushed some fixes for the nav situation

ryanmitchell commented 1 year ago

Just leaving a note here so its not missed, before this merges we will need to update the imports for assets, collections and navs to allow users to decide what they want to import.