Skip to content

Conditional eager-loading of post meta values#2012

Closed
aj-adl wants to merge 3 commits intotimber:masterfrom
aj-adl:eager-loading-dev
Closed

Conditional eager-loading of post meta values#2012
aj-adl wants to merge 3 commits intotimber:masterfrom
aj-adl:eager-loading-dev

Conversation

@aj-adl
Copy link
Copy Markdown
Contributor

@aj-adl aj-adl commented May 24, 2019

Ticket: Fixes #2011

Issue

Allows the user to control what post meta values get eager-loaded into \Timber\Post->custom

Solution

Supports both disabling, and conditional loading

Impact

This filter currently cannot affect how the parent function of get_post_custom runs. There is a potential issue if people have been hooking into here to perform some action, and returned null / void etc as previously the return value had no effect. I doubt many people are getting that into the guts of Timber, but it is something to be considered

Usage Changes

Allows the user to return false or null to stop the eager-loading, or return an array of $key => $value which will then be processed and imported into the Timber\Post or derivative object.

Considerations

naming? not suer how this looks in 2.0, seems like some changes there

Testing

Unit tests included and passing in tests-timber-post.php

aj-adl added 2 commits May 24, 2019 11:44
- Allow the user to stop eager loading by returning `null` or `false` via timber_post_get_meta_pre filter.
- Allow the user to choose what to eager load by returning an array of k/v meta
- Test for 'short circuit' where no meta should be eager loaded
- Test for custom list of eager-loaded post meta
@aj-adl aj-adl requested review from jarednova and palmiak as code owners May 24, 2019 06:15
@codecov

This comment has been minimized.

@coveralls
Copy link
Copy Markdown

Coverage Status

Coverage increased (+0.002%) to 94.264% when pulling 0ef9f04 on aj-adl:eager-loading-dev into 1b4c1bb on timber:master.

@gchtr gchtr self-assigned this May 24, 2019
@jarednova
Copy link
Copy Markdown
Member

@gchtr has been working on re-writing meta value handling in 2.x, so I'm gonna let him take this from here on how to best pull this (or its ideas) in

@gchtr
Copy link
Copy Markdown
Member

gchtr commented May 30, 2019

Hey @aj-adl, I just opened #2014 to match the behavior that I added in 2.0. Turns out that you would approach this the same way as I would do it 👍.

I think we’ll leave the name as it is. You’ll have to update the filter to the new name timber/post/pre_get_meta_values as soon as 2.0 is released, though.

jarednova added a commit that referenced this pull request May 31, 2019
Ref #2012 – Update skipping get_post_meta() to match behavior in 2.0
@jarednova
Copy link
Copy Markdown
Member

@aj-adl thanks again for submitting. I see that @gchtr pulled the underlying work and tests over in #2014. Really appreciate you flagging the issue, submitting the PR AND supplying tests. Triple crown!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Allow timber_post_get_meta_pre filter to short circuit or alter Timber\Post::get_post_custom()

4 participants