corcel / acf

Advanced Custom Fields (ACF) plugin for Corcel
126 stars 98 forks source link

Support for option page, refactored & fixed unittests, fixed flexible content field #64

Closed tbruckmaier closed 5 years ago

tbruckmaier commented 6 years ago

Hey,

as mentioned in #48 this is the big PR to implement the acf option page. I basically moved a lot of code from the single Field classes to a new layer called "Repository". All the "old" code sits now in the PostRepository (which holds the $post instance). The single fields get the repository now in the constructor and all database access runs through that repository. For the option page, we have a second OptionPageRepository which returns the according data to the fields from wp_options. So we can have the same logic for fields from posts/pages and from a option page. Additionally, as FieldFactory::make() takes a repository now as parameter, users can write their own repositories and everything is a lot more decoupled and flexible.

The second big change covers the unit tests: i have seen that their was only a database.sql in the repo, and the unit tests run only on a real database. I have changed all unit tests to run with mocked models & factories as in the corcel package. Basically i have copied the values from the testing database into the code, so the tests did not need to change a lot. I have updated the underlying corcel version to 2.4 (for laravel 5.4 since 5.5 needs php>=7) and have added the orchestra test package as well.

Yeah, i hope you can take some time to review this, though i have made sure that everything is backwards-compatible

tbruckmaier commented 6 years ago

From my point of view, this is ready to be merged!

0xb4lint commented 6 years ago

Thank you, great job @tbruckmaier! What do you think @jgrossi?

tbruckmaier commented 6 years ago

The last changes contain fixes for the flexible layout field.

The old implementation basically pulled all variables from the database matching the flexible content's name (e.g. for a field like "modules" it would fetch all fields named "modules_%"). If a flexible content has been rearranged and saved in wordpress though, the obsolete variables would not be deleted from the database. Lets say, the first version had the first layout block as a type a with the two fields "test1" and "test2", so the postmeta contains "modules_0_test1" and "modules_0_test2", afterwards the user changed it and the first layout block would be a type b with just one field "test3" => the database still contains now "modules_0_test1", "modules_0_test2" and "modules_0_test3". The old implementation of the corcel acf flexible content would return all three values, and possibly screw up the field types. The changes from this PR take the top-down approach, e.g. it reads the subfields of the available layout blocks and the current order of layout blocks, and accesses the variables that way. So we do exactly end up with the correct values and the correct field types.

I also updated the corcel & php dependencies to 2.5 and 7.0 respectivly to make it work (laravel 5.4 & php 5 are not supported anymore anyway, so this should be fine)

seldimi commented 6 years ago

Any news on this ?

tbruckmaier commented 5 years ago

This pull request ended up as a complete rewrite, you can find the updated fork over here: https://github.com/tbruckmaier/corcel-acf