FriendsOfCake / search

CakePHP: Easy model searching
MIT License
170 stars 59 forks source link

Bake task for filter collections #300

Closed dereuromark closed 3 years ago

dereuromark commented 4 years ago

I whipped sth up Resolves https://github.com/FriendsOfCake/search/issues/296

I didn't check the visibility on the entity yet, if I should do that let me know But this already works quite well for default cases IMO.

We could think about further improvements as follow up:

codecov[bot] commented 4 years ago

Codecov Report

Merging #300 (3136c16) into master (8376995) will decrease coverage by 0.53%. The diff coverage is 90.24%.

Impacted file tree graph

@@             Coverage Diff              @@
##             master     #300      +/-   ##
============================================
- Coverage     93.92%   93.38%   -0.54%     
- Complexity      204      227      +23     
============================================
  Files            17       18       +1     
  Lines           510      590      +80     
============================================
+ Hits            479      551      +72     
- Misses           31       39       +8     
Impacted Files Coverage Δ Complexity Δ
src/Command/BakeFilterCollectionCommand.php 89.74% <89.74%> (ø) 23.00 <23.00> (?)
src/Model/Filter/Base.php 93.97% <100.00%> (+0.07%) 37.00 <0.00> (ø)
src/Processor.php 96.96% <100.00%> (+0.09%) 13.00 <0.00> (ø)

Continue to review full report at Codecov.

Legend - Click here to learn more Δ = absolute <relative> (impact), ø = not affected, ? = missing data Powered by Codecov. Last update 8376995...3136c16. Read the comment docs.

dereuromark commented 4 years ago

This whole file system generation and comparison thing is a bit strange For some reason it fails only for https://travis-ci.com/github/FriendsOfCake/search/jobs/428393497 still. This is prefer lowest, must be related to that.

ADmad commented 4 years ago

This is prefer lowest, must be related to that.

That's the problem with using prefer lowest, you don't get the bug fixes required :)

dereuromark commented 4 years ago

Any takes on what needs to be bumped? Or should we completely rewrite file based generation testing? Using memory files or sth?

ADmad commented 4 years ago

My guess would be it's the twig view plugin for which a particular big bugfix is required.

dereuromark commented 4 years ago

Otherwise all good? Or do you see any concerns so far?

dereuromark commented 4 years ago

Tests are now fixed :)

ADmad commented 4 years ago

CS job is still failing, wonder why stickler hasn't fixed it.

dereuromark commented 3 years ago

The CS fails seemed unrelated, but I fixed them now as well After rebase I expect this to be green now.

As for the $this chaining. I think this would need a helper method and some additional internal logic. If you don't mind I would like to hold off on that for now as a future follow up (maybe together with the improvements above).