Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

2.x Cleanup todo in code #2757

Merged
merged 9 commits into from
Jul 21, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
1 change: 0 additions & 1 deletion src/Comment.php
Original file line number Diff line number Diff line change
Expand Up @@ -564,7 +564,6 @@ protected function avatar_host($email_hash)

/**
* @internal
* @todo what if it's relative?
Copy link
Member Author

Choose a reason for hiding this comment

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

If this is an issue in the future, it will pop up as an issue.

* @param string $default
* @param string $email
* @param string $size
Expand Down
2 changes: 0 additions & 2 deletions src/CoreEntity.php
Original file line number Diff line number Diff line change
Expand Up @@ -145,7 +145,6 @@ protected function fetch_meta($field_name = '', $args = [], $apply_filters = tru
$args
);

// @todo Remove when deprecated filters will be gone
Copy link
Member Author

Choose a reason for hiding this comment

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

We can remove all apply_filters_deprecated() calls with the next major release.

if ($object_type !== 'term') {
/**
* Filters the value for a post meta field before it is fetched from the database.
Expand Down Expand Up @@ -236,7 +235,6 @@ protected function fetch_meta($field_name = '', $args = [], $apply_filters = tru
$args
);

// @todo Remove when deprecated filters will be gone
if ($object_type === 'term') {
/**
* Filters the value for a term meta field.
Expand Down
1 change: 0 additions & 1 deletion src/Factory/UserFactory.php
Original file line number Diff line number Diff line change
Expand Up @@ -86,7 +86,6 @@ protected function from_user_object($obj): CoreInterface
));
}

// @todo return a UserCollection instance?
Copy link
Member Author

Choose a reason for hiding this comment

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

Not at the moment. We discussed somewhere that user collections or collections for other types of content are not needed at the moment (#1689). We can add it if we really need it.

protected function from_wp_user_query(WP_User_Query $query): iterable
{
return array_map([$this, 'build'], $query->get_results());
Expand Down
4 changes: 1 addition & 3 deletions src/Loader.php
Original file line number Diff line number Diff line change
Expand Up @@ -198,7 +198,7 @@ public function render($file, $data = null, $expires = false, $cache_mode = self
/**
* Filters …
*
* @todo Add summary
* @todo Add summary
*
* @deprecated 2.0.0, use `timber/output`
*/
Expand Down Expand Up @@ -587,7 +587,6 @@ public function clear_cache_twig()
$twig = $this->get_twig();

// Get the configured cache location.
// @todo What if this is a custom caching implementation?
Copy link
Member Author

Choose a reason for hiding this comment

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

This will pop up as an issue in case it becomes a problem.

$cache_location = $twig->getCache(true);

// Cache not activated.
Expand All @@ -596,7 +595,6 @@ public function clear_cache_twig()
}

if (is_string($cache_location) && is_dir($cache_location)) {
// @todo What if not all files could be deleted?
Copy link
Member Author

Choose a reason for hiding this comment

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

Might be improved when we introduce better and streamlined error handling, see #2210.

self::rrmdir($cache_location);
return true;
}
Expand Down
3 changes: 1 addition & 2 deletions src/Post.php
Original file line number Diff line number Diff line change
Expand Up @@ -47,7 +47,6 @@
* </div>
* </article>
* ```
* @todo implement JsonSerializable?
Copy link
Member Author

Choose a reason for hiding this comment

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

Serialization can be added if needed. It’s documented in https://timber.github.io/docs/v2/guides/posts/#serialization.

*/
class Post extends CoreEntity implements DatedInterface, Setupable
{
Expand Down Expand Up @@ -165,7 +164,7 @@ class Post extends CoreEntity implements DatedInterface, Setupable
public $slug;

/**
* @var string Stores the PostType object for the Post
* @var string Stores the PostType object for the post.
*/
protected $__type;

Expand Down
1 change: 0 additions & 1 deletion src/PostsIterator.php
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,6 @@ public function current()
*/
$wp_post = parent::current();
// Lazily instantiate a Timber\Post instance exactly once.
// @todo maybe improve performance by caching the instantiated post.
Copy link
Member Author

Choose a reason for hiding this comment

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

Performance and caching is an area where we could improve in a lot of places. In case we want to improve post performance, we’ll find the right place to do it.

$post = $factory->from($wp_post);
$post->setup();

Expand Down
19 changes: 2 additions & 17 deletions src/Timber.php
Original file line number Diff line number Diff line change
Expand Up @@ -195,11 +195,6 @@ public static function handle_preview($data, $post)
$data['post_title'] = $preview->post_title;
$data['post_excerpt'] = $preview->post_excerpt;

// @todo I think we can safely delete this?
// It was included in the old PostCollection method but not defined anywhere,
// so I think it was always just falling into a magic __call() and doing nothing.
// $post->import_custom($preview_id);

add_filter('get_the_terms', '_wp_preview_terms_filter', 10, 3);
}

Expand Down Expand Up @@ -318,7 +313,6 @@ public static function get_attachment($query = false, $options = [])
{
$post = static::get_post($query, $options);

// @todo make this determination at the Factory level.
Copy link
Member Author

Choose a reason for hiding this comment

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

We might improve this in the future. For the moment, it seems to work.

// No need to instantiate a Post we're not going to use.
return ($post instanceof Attachment) ? $post : null;
}
Expand All @@ -344,7 +338,6 @@ public static function get_image($query = false, $options = [])
{
$post = static::get_post($query, $options);

// @todo make this determination at the Factory level.
// No need to instantiate a Post we're not going to use.
return ($post instanceof Image) ? $post : null;
}
Expand Down Expand Up @@ -457,9 +450,6 @@ public static function get_posts($query = false, $options = [])
);
}

/**
* @todo Are there any more default options to support?
Copy link
Member Author

Choose a reason for hiding this comment

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

We’ll figure them out if needed.

*/
$options = wp_parse_args($options, [
'merge_default' => false,
]);
Expand Down Expand Up @@ -863,7 +853,7 @@ public static function get_term_by(string $field, $value, string $taxonomy = '')
public static function get_users(array $query = [], array $options = []): iterable
{
$factory = new UserFactory();
// TODO return a Collection type?
Copy link
Member Author

Choose a reason for hiding this comment

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

See #1689.


return $factory->from($query);
}

Expand Down Expand Up @@ -903,11 +893,6 @@ public static function get_users(array $query = [], array $options = []): iterab
*/
public static function get_user($user = null)
{
/*
* TODO in the interest of time, I'm implementing this logic here. If there's
* a better place to do this or something that already implements this, let me know
* and I'll switch over to that.
*/
$user = $user ?: get_current_user_id();

$factory = new UserFactory();
Expand Down Expand Up @@ -1074,7 +1059,7 @@ public static function get_pages_menu(array $args = [])
public static function get_comments($query = [], array $options = []): iterable
{
$factory = new CommentFactory();
// TODO return a Collection type?

return $factory->from($query);
}

Expand Down
2 changes: 0 additions & 2 deletions src/Twig.php
Original file line number Diff line number Diff line change
Expand Up @@ -353,8 +353,6 @@ public function get_timber_filters()

/**
* Date and Time filters.
*
* @todo copy this formatting to other functions
*/
'date' => [
'callable' => [$this, 'twig_date_format_filter'],
Expand Down
19 changes: 0 additions & 19 deletions tests/test-timber-deprecated.php
Copy link
Member Author

Choose a reason for hiding this comment

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

This file didn’t really do anything.

This file was deleted.

12 changes: 0 additions & 12 deletions tests/test-timber-function-wrapper.php
Original file line number Diff line number Diff line change
Expand Up @@ -57,18 +57,6 @@ public function testFunctionInTemplate()
$this->assertEquals('bar!', trim($str));
}

public function testSoloFunctionUsingWrapper()
{
if (version_compare(Timber::$version, 2.0, '>=')) {
return $this->markTestSkipped(
'This functionality is disabled in Timber 2.0'
);
}
new Timber\FunctionWrapper('my_boo');
$str = Timber::compile_string("{{ my_boo() }}");
$this->assertEquals('bar!', trim($str));
}

public function testNakedSoloFunction()
{
add_filter('timber/twig', function ($twig) {
Expand Down
13 changes: 6 additions & 7 deletions tests/test-timber-post-array-object.php
Original file line number Diff line number Diff line change
Expand Up @@ -164,14 +164,13 @@ public function testArrayAccess()
'post_date' => '2020-01-03',
]);

// @todo once the Posts API uses Factories, simplify this to Timber::get_posts([...])
$wp_query = new WP_Query('post_type=post');

$collection = new PostArrayObject($wp_query->posts);
$posts = Timber::get_posts([
'post_type' => 'post',
]);

$this->assertEquals('Post 0', $collection[0]->title());
$this->assertEquals('Post 1', $collection[1]->title());
$this->assertEquals('Post 2', $collection[2]->title());
$this->assertEquals('Post 0', $posts[0]->title());
$this->assertEquals('Post 1', $posts[1]->title());
$this->assertEquals('Post 2', $posts[2]->title());
}

public function testIterationWithClassMaps()
Expand Down
17 changes: 10 additions & 7 deletions tests/test-timber-post-collection.php
Original file line number Diff line number Diff line change
Expand Up @@ -226,8 +226,10 @@ public function testFoundPostsWithPostsPerPage()
{
$this->factory->post->create_many(10);

// @todo once the Posts API uses Factories, simplify this to Timber::get_posts([...])
$query = new PostQuery(new WP_Query('post_type=post&posts_per_page=3'));
$query = Timber::get_posts([
'post_type' => 'post',
'posts_per_page' => 3,
]);

$this->assertCount(3, $query);
$this->assertSame(10, $query->found_posts);
Expand All @@ -249,12 +251,13 @@ public function testArrayAccess()
'post_date' => '2020-01-03',
]);

// @todo once the Posts API uses Factories, simplify this to Timber::get_posts([...])
$query = new PostQuery(new WP_Query('post_type=post'));
$posts = Timber::get_posts([
'post_type' => 'post',
]);

$this->assertEquals('Post 0', $query[0]->title());
$this->assertEquals('Post 1', $query[1]->title());
$this->assertEquals('Post 2', $query[2]->title());
$this->assertEquals('Post 0', $posts[0]->title());
$this->assertEquals('Post 1', $posts[1]->title());
$this->assertEquals('Post 2', $posts[2]->title());
}

public function testIterationWithClassMaps()
Expand Down
4 changes: 0 additions & 4 deletions tests/test-timber-post-content.php
Original file line number Diff line number Diff line change
Expand Up @@ -171,10 +171,6 @@ public function testPagedContentWithBlocksAndNextPageAtBeginning()
*/
public function testGutenbergExcerptOption()
{
global $wp_version;
if ($wp_version < 5.0) {
$this->markTestSkipped('Only applies to Block editor which is avaialble in WP 5.x');
}
$content_1 = '<!-- wp:paragraph -->
<p>Heres the start to a thing</p>
<!-- /wp:paragraph -->
Expand Down
62 changes: 26 additions & 36 deletions tests/test-timber-post.php
Original file line number Diff line number Diff line change
Expand Up @@ -178,25 +178,20 @@ public function testNextCategory()

public function testNextCustomTax()
{
$v = get_bloginfo('version');
if (version_compare($v, '3.8', '<')) {
Copy link
Member Author

Choose a reason for hiding this comment

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

We don’t need to support WordPress < 3.8.

$this->markTestSkipped('Custom taxonomy prev/next not supported until 3.8');
} else {
register_taxonomy('pizza', 'post');
$posts = [];
for ($i = 0; $i < 4; $i++) {
$j = $i + 1;
$posts[] = $this->factory->post->create([
'post_date' => '2014-02-0' . $j . ' 12:00:00',
]);
}
wp_set_object_terms($posts[0], 'Cheese', 'pizza', false);
wp_set_object_terms($posts[2], 'Cheese', 'pizza', false);
wp_set_object_terms($posts[3], 'Mushroom', 'pizza', false);
$firstPost = Timber::get_post($posts[0]);
$nextPost = Timber::get_post($posts[2]);
$this->assertEquals($firstPost->next('pizza')->ID, $nextPost->ID);
register_taxonomy('pizza', 'post');
$posts = [];
for ($i = 0; $i < 4; $i++) {
$j = $i + 1;
$posts[] = $this->factory->post->create([
'post_date' => '2014-02-0' . $j . ' 12:00:00',
]);
}
wp_set_object_terms($posts[0], 'Cheese', 'pizza', false);
wp_set_object_terms($posts[2], 'Cheese', 'pizza', false);
wp_set_object_terms($posts[3], 'Mushroom', 'pizza', false);
$firstPost = Timber::get_post($posts[0]);
$nextPost = Timber::get_post($posts[2]);
$this->assertEquals($firstPost->next('pizza')->ID, $nextPost->ID);
}

public function testPrev()
Expand All @@ -215,25 +210,20 @@ public function testPrev()

public function testPrevCustomTax()
{
$v = get_bloginfo('version');
if (version_compare($v, '3.8', '<')) {
$this->markTestSkipped('Custom taxonomy prev/next not supported until 3.8');
} else {
register_taxonomy('pizza', 'post');
$posts = [];
for ($i = 0; $i < 3; $i++) {
$j = $i + 1;
$posts[] = $this->factory->post->create([
'post_date' => '2014-02-0' . $j . ' 12:00:00',
'post_title' => "Pizza $j is so good!",
]);
}
$cat = wp_insert_term('Cheese', 'pizza');
self::set_object_terms($posts[0], $cat, 'pizza', false);
self::set_object_terms($posts[2], $cat, 'pizza', false);
$lastPost = Timber::get_post($posts[2]);
$this->assertEquals($posts[0], $lastPost->prev('pizza')->ID);
register_taxonomy('pizza', 'post');
$posts = [];
for ($i = 0; $i < 3; $i++) {
$j = $i + 1;
$posts[] = $this->factory->post->create([
'post_date' => '2014-02-0' . $j . ' 12:00:00',
'post_title' => "Pizza $j is so good!",
]);
}
$cat = wp_insert_term('Cheese', 'pizza');
self::set_object_terms($posts[0], $cat, 'pizza', false);
self::set_object_terms($posts[2], $cat, 'pizza', false);
$lastPost = Timber::get_post($posts[2]);
$this->assertEquals($posts[0], $lastPost->prev('pizza')->ID);
}

public function testPrevCategory()
Expand Down
2 changes: 0 additions & 2 deletions tests/test-timber-properties.php
Original file line number Diff line number Diff line change
Expand Up @@ -5,8 +5,6 @@
* @group comments-api
* @group called-post-constructor
* @group called-term-constructor
* @todo #2094 replace direct Timber\User instantiations
* @todo #2094 replace direct Timber\Comment instantiations
*/
class TestTimberProperty extends Timber_UnitTestCase
{
Expand Down
14 changes: 8 additions & 6 deletions tests/test-timber-static.php
Original file line number Diff line number Diff line change
Expand Up @@ -68,20 +68,22 @@ public function testFrontPageAsPage()

public function testStaticPostPage()
{
$this->markTestSkipped('@todo Undefined offset: 0 - do we need merge_default for this?');
$this->clearPosts();
$page_id = $this->factory->post->create([
'post_title' => 'Gobbles',
'post_type' => 'page',
]);
update_option('page_for_posts', $page_id);
$this->go_to(home_url('/?p=' . $page_id));
$children = $this->factory->post->create_many(10, [
$posts = $this->factory->post->create_many(10, [
'post_title' => 'Timmy',
]);

update_option('show_on_front', 'page');
Copy link
Member Author

Choose a reason for hiding this comment

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

This line was missing to get the test working.

update_option('page_for_posts', $page_id);
$this->go_to(get_permalink($page_id));

$posts = Timber::get_posts();
$first_post = $posts[0];
$this->assertEquals('Timmy', $first_post->title());

$this->assertEquals('Timmy', $posts[0]->title());
}

public function testOtherPostOnStaticPostPage()
Expand Down