Skip to content

2.x posts api#2316

Merged
gchtr merged 111 commits into2.x-factoriesfrom
2.x-posts-api
Oct 7, 2020
Merged

2.x posts api#2316
gchtr merged 111 commits into2.x-factoriesfrom
2.x-posts-api

Conversation

@acobster
Copy link
Copy Markdown
Collaborator

@acobster acobster commented Aug 26, 2020 •
edited
Loading

Work in progress final port of the Posts API to use Factories under the hood.

  • Switch ::get_post[s]() API to use Factories
  • Replace a bunch of direct instantiations in the tests
  • Fix Term::posts()
  • Post::build() method
  • Attachment/Image class map callback
  • Update Twig functions (@gchtr doing on 2.x Update Timber Object Getter function names in Twig #2178)
  • Simplify PostQuery::__construct() to only accepting a WP_Query instance (with no wrapping it in an array)
  • Fix 2.x Update Timber Object Getter function names in Twig #2178 and merge in to this feature branch
  • Make Post::__construct() protected, annotate WP_Post arg
  • Timber::get_attachment_by()
  • Close the loop on whether we need to pass an array as a second arg to wp_check_filetype()
  • Attachment::__construct() has an option to pass an array like ['ID' => 123] ...are we good to drop support for this? Otherwise we would need to handle this special case in ::get_post(). we need to support this for ACF integration, so add a special case for this in ::get_post() (or PostFactory?)
  • @expectedIncorrectUsage is failing randomly on those two tests...y tho.
  • deprecate Attachment::get_pathinfo() and Image::get_pathinfo() and use Attachment::pathinfo() only to be in line with other post method names.
  • Support the merge_default option in ::get_post[s]()

gchtr and others added 30 commits January 18, 2020 15:52
@gchtr
Update Timber Object Getter function names in Twig
122ceac
@gchtr
Add Twig functions for getting comments
5f9189b
@gchtr
Update Upgrade Guide
d28fd53
@gchtr
Merge branch '2.x-factories' into 2.x-twig-get-functions-2
e99e1d2
@gchtr
Use get_image() instead of Image() in Twig
7d02bb4
@gchtr
Update missing function names
a96abb0
@gchtr
Enable functions for users and comments
48dee96
@gchtr
Merge branch '2.x-factories' into 2.x-twig-get-functions-2
8c70397 
# Conflicts:
#	docs/guides/posts.md
#	docs/guides/terms.md
#	lib/Twig.php
#	tests/test-timber-twig-objects.php
@gchtr
Delete unused process_term_args() function
37144bd
@gchtr
Update documentation with updates from master branch
fc99ace
@gchtr
Update function signature for Term::posts()
f5976a1
@gchtr
Remove unused option parameter
032edba
@gchtr
Fix coding standards
5000d0c
@IJMacD
Better control over Pagination stops
b8dbbdb 
I had the case where I needed to hide the last pages of pagination (the very oldest posts).

I found the `end_step` (not documented anywhere) controlled how many pages are shown at either end of the pagination range.
Additionally I was not able to set this to zero to hide the pages completely.

My proposal is to allow zero pages to be shown at the ends of the list. Also to enable different values for the start and end. The start value defaults to the same as the end value for backwards compatibility.
@IJMacD
Add tests for Pagination End Limits
99f0e98
@gchtr
Update name to learn-timber-theme
3a675a9
@jarednova
Merge pull request #2306 from timber/2.x-docs-learn-timber-theme
d64985a 
2.x – Rename "Getting Started Theme" to "Learn Timber Theme"
@jarednova
Merge pull request #2302 from IJMacD/patch-1
3dc9440 
Better control over Pagination stops
@oxyc
fix #2216: delete sideloaded image tmp files
e60c627
@jarednova
Update changelog and version numbers for 1.18.0
91675ac
#2093 transition away from array arg to PostQuery::__construct()
af82456
#2093 refactor or skip a bunch of transitioning tests
2b7850a
@nlemoine
#2301 Keep Twig native filter parity
e1a5037
@nlemoine
Fix variable name
59310de
@jarednova
Merge pull request #2314 from nlemoine/fix-array-filter
2efa330 
#2301 Keep Twig native `filter` parity
#2093 migrate some PostGetter tests
e963817
#2093 merge upstream deprecation of PostQuery::get_posts
47dac9f
#2093 deprecate ::query_post[s]()
e3d4ff3
#2093 migrate more PostGetter tests and skip temporarily failing Timb…
3fa4618 
…er tests
update a few comments
aed937a
@acobster
Copy link
Copy Markdown
Collaborator Author

acobster commented Sep 26, 2020

I’d be glad if you could finish this.

OK, done and merged in as part of this PR! They are inter-dependent, so one of them has to be done first. I figured I'd make that the smaller one.

@acobster
Copy link
Copy Markdown
Collaborator Author

acobster commented Sep 26, 2020

@gchtr @jarednova I'm calling this ready for review!

@acobster acobster mentioned this pull request Sep 27, 2020
Closed
3 tasks
@jarednova
Copy link
Copy Markdown
Member

jarednova commented Sep 30, 2020

Excellent, awesome to hear @acobster — I'll add to my queue for the week!

gchtr
gchtr reviewed Oct 5, 2020
View reviewed changes
@jarednova jarednova removed the wip Work in Progress label Oct 6, 2020
jarednova
jarednova requested changes Oct 6, 2020
View reviewed changes
Copy link
Copy Markdown
Member

@jarednova jarednova left a comment •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@acobster we've already been through this, so I didn't see much else but a few docs things for you to check out.

One last 🔧 /question is noticing the number of Image($image_id) that are now supposed to be called with Timber::get_post($image_id). I'm wondering if we should have a Timber::get_image() (or Timber::get_attachment()) for these. Forgive me if this has already been discussed/resolved and my brain is just melting. (If we do this, let's mark as a @todo and tackle in a seperate PR so we can move forward w the merge here)

@gchtr
Copy link
Copy Markdown
Member

gchtr commented Oct 6, 2020

One last 🔧 /question is noticing the number of Image($image_id) that are now supposed to be called with Timber::get_post($image_id). I'm wondering if we should have a Timber::get_image() (or Timber::get_attachment()) for these. Forgive me if this has already been discussed/resolved and my brain is just melting. (If we do this, let's mark as a @todo and tackle in a seperate PR so we can move forward w the merge here)

@jarednova Yes, we discussed this in #2316 (comment) and previous as well as in #2178 (comment).

My take is: Using {{ get_attachment() }} and {{ get_image() }} as well as Timber::get_attachment() and Timber::get_image() in the code would help in understanding what you’re working with. We now have Timber::get_attachment_by() to handle the special cases, so I guess Timber::get_attachment() and Timber::get_image() would just be aliases for Timber::get_post()? What about Timber::get_attachments() and Timber::get_images()?

acobster and others added 2 commits October 6, 2020 16:13
@acobster @jarednova
example for more specific get_attachment() twig fn
c66e53a 
Co-authored-by: Jared Novack <jarednova@upstatement.com>
@acobster @jarednova
update docs to reflect more specific get_image() twig fn
3dedab1 
Co-authored-by: Jared Novack <jarednova@upstatement.com>
@acobster
Copy link
Copy Markdown
Collaborator Author

acobster commented Oct 6, 2020

I think as long as we say something like:

Alias for Timber::get_post(). That means it's possible to get something other than an Attachment (such a plain Timber\Post object) back if you pass something other than an attachment ID. Intended to clarify your code when you already know you have an attachment ID.

...in the docs for ::get_(image|attachment), then @gchtr's proposal sounds good to me.

@gchtr
Copy link
Copy Markdown
Member

gchtr commented Oct 7, 2020

I think as long as we say something like:

Alias for Timber::get_post(). That means it's possible to get something other than an Attachment (such a plain Timber\Post object) back if you pass something other than an attachment ID. Intended to clarify your code when you already know you have an attachment ID.

...in the docs for ::get_(image|attachment), then @gchtr's proposal sounds good to me.

Yes, sure!

Coby Tamayo and others added 3 commits October 7, 2020 10:38
@acobster acobster mentioned this pull request Oct 7, 2020
Closed
@acobster
Copy link
Copy Markdown
Collaborator Author

acobster commented Oct 7, 2020 •
edited
Loading

@gchtr @jarednova OK, I believe that's all the bookkeeping stuff for this PR. Are we good to go? 😬 😅

THE FINAL COUNTDOWN

@jarednova
Copy link
Copy Markdown
Member

jarednova commented Oct 7, 2020

Nothing but green checkmarks over here!

@gchtr gchtr merged commit 1f6f3d7 into 2.x-factories Oct 7, 2020
@gchtr gchtr deleted the 2.x-posts-api branch October 7, 2020 18:36
@gchtr gchtr mentioned this pull request Oct 7, 2020
Closed
@acobster acobster mentioned this pull request Oct 15, 2020
Merged
acobster pushed a commit that referenced this pull request Oct 15, 2020
#2333 implement top-level image methods
8635301 
These are just aliases of ::get_post(). See also discussion in #2316, specifically:

#2316 (review)
@acobster acobster mentioned this pull request Oct 15, 2020
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@gchtr gchtr gchtr left review comments

@jarednova jarednova jarednova requested changes

@pascalknecht pascalknecht Awaiting requested review from pascalknecht

Assignees

@acobster acobster

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

6 participants

@acobster @gchtr @jarednova @IJMacD @oxyc @nlemoine