Skip to content

Added tests + Small refactor#22

Open
kevinmeijer97 wants to merge 8 commits intomasterfrom
feature/tests
Open

Added tests + Small refactor#22
kevinmeijer97 wants to merge 8 commits intomasterfrom
feature/tests

Conversation

@kevinmeijer97
Copy link
Member

@kevinmeijer97 kevinmeijer97 commented Mar 9, 2026

So added tests and fixed alot of phpstan + pint errors.
Going forward it should be pretty maintainable, should've done this sooner though!

@kevinmeijer97 kevinmeijer97 marked this pull request as ready for review March 9, 2026 14:33
@kevinmeijer97 kevinmeijer97 changed the title Refactored code base + added tests Added tests + Small refactor Mar 9, 2026
$taxonomyValue = $template->get('use_for_taxonomy');

if (! $taxonomyValue) {
$taxonomyValue = $template->use_for_taxonomy; /** @phpstan-ignore-line */
Copy link
Collaborator

Choose a reason for hiding this comment

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

isn't doing
$template->get('use_for_taxonomy'); on line 108 exactly the same as $template->use_for_taxonomy

Copy link
Member Author

Choose a reason for hiding this comment

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

Sadly no, $template->get('use_for_taxonomy'); gets the raw value which in this case is a string.
Calling it directly through $template->use_for_taxonomy gives us the actuall Collection object.
I couldn't get the tests & phpstan to work with only using $template->use_for_taxonomy so thats why i ended up with this.

if (! $collection instanceof Collection) {
return 0;
}
// @codeCoverageIgnoreEnd
Copy link
Collaborator

Choose a reason for hiding this comment

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

Why the ignore?

Copy link
Member Author

Choose a reason for hiding this comment

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

Couldn't really get this to work in my tests

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants