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

Taxonomy filter stopped working for integer values #2446

Closed
brickcamp opened this issue Apr 13, 2019 · 12 comments
Closed

Taxonomy filter stopped working for integer values #2446

brickcamp opened this issue Apr 13, 2019 · 12 comments

Comments

@brickcamp
Copy link

brickcamp commented Apr 13, 2019

Hi there,

I just noticed that taxonomy terms formatted as integers stopped working with URL taxonomy filters. I reproduced this behaviour with Grav 1.6.3 and the vanilla develop branch.

Here's an example of what I mean: Consider a page with a YAML header containing a string and an integer term:

taxonomy:
  count: ["1", 2]

What happens is that a URL filter on the first works properly but a URL filter on the second fails to deliver a result. Before the latest release, both worked. (I linked my real-world example for reference).

Changing every integer term to string isn't an option for me as I have a huge amount of them. I already tried to strval every term automagically via plugin hook. It works locally, but not on production. I'll keep on tinkering around, but maybe there are more people out there having this issue.

Edit: Fixed typo

@rhukster
Copy link
Member

This is 'probably' related to the stricter YAML parsing in Symfony 4.2 (https://learn.getgrav.org/16/advanced/grav-development/grav-16-upgrade-guide#yaml-parsing)

I will investigate because it probably needs to have the data normalized to strings.

@rhukster rhukster added the bug label Apr 13, 2019
@brickcamp
Copy link
Author

I fixed it by doing this at "onPageProcessed" (here in context). Works for me now, but maybe has side effects for others who depend on integers elsewhere 🤔

        $page = $event['page'];
        $taxonomy = $page->taxonomy();
        foreach ($taxonomy as $tax => $terms) {
            foreach ($terms as $key => $term) {
                $taxonomy[$tax][$key] = strval($term);
            }
        }
        $page->taxonomy($taxonomy);

@rhukster
Copy link
Member

Please try the fix I just commited above. Thanks!

@rhukster rhukster added the fixed label Apr 13, 2019
@brickcamp
Copy link
Author

brickcamp commented Apr 13, 2019

This fix raises some issues with handling post taxonomies/terms as arrays - when there is a taxonomy with only one term. In my custom coding (which I can adapt - it's a mess honestly) as well as in the related-pages plugin, seen here:

Bildschirmfoto-20190413224555-1276x544
Zip of full error page

Maybe also elsewhere but this is what comes up on my page 🤔

@rhukster
Copy link
Member

I can't seem to replicate this error... I'm using the blog skeleton: https://getgrav.org/download/skeletons/blog-site/2.0.0

It has taxonomy plugin, as well as taxonomy with only one entry (such as category: blog)

taxonomy:
    category: blog
    tag: ['1', 2, journal, link]

@rhukster
Copy link
Member

I guess the problem might be is your taxonomy has arrays further in where it shouldn't have. Can you give me a sample of the pages taxonomy that are causing the problems???

@brickcamp
Copy link
Author

No further arrays that I'm aware of. To reproduce, pull vanilla Grav develop branch, replace the user folder by this user.zip and run bin/grav install for the dependencies. The error occurs on /tech/technic-brick-octagon. 🤔

I honestly don't know why there is no issue with the skeleton, I tried it myself but didn't dive in deeper to force it there.

@rhukster
Copy link
Member

rhukster commented Apr 14, 2019

it's probably that partcount not only being a single item, but not being an array:

taxonomy:
  part: ["3894", "32000", "2780"]
  partcount: 80

I will look into it today, even though this is not formatted correctly, it should still be backwards compatible with stuff that worked in 1.5.

@brickcamp
Copy link
Author

brickcamp commented Apr 14, 2019

Thanks a lot! 👍 And I'll look for ways to bulk update the formatting on my website.

@rhukster
Copy link
Member

OK this should fix it once and for all!

9a5fa7e

@brickcamp
Copy link
Author

Thank you so much! I'll test it in the evening after work 👍

@brickcamp
Copy link
Author

Tested it, works perfectly - thanks once again for your patience with this issue. I finally close it now.

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

No branches or pull requests

1 participant