getgrav / grav-plugin-sitemap

Grav Sitemap Plugin
https://getgrav.org
MIT License
42 stars 42 forks source link

ignore paths not saved in admin config panel #26

Closed phivk closed 7 years ago

phivk commented 7 years ago

When I add new paths to the ignores on the admin config panel for the plugin and press 'save', the new path is not saved to the list. I do get a "Successfully saved" notification.

an example of a path I've tried to add: /chapters/3-70000-obeti

Adding the paths manually in user/config/plugins/sitemap/sitemap.yaml works.

I'm using OSX Chrome and running latest Grav with Sitemap v1.7.0

flaviocopes commented 7 years ago

Replicated. I went back to all Admin plugin changes and after a bit of bisecting I tracked the commit that introduces the problem, it's https://github.com/getgrav/grav-plugin-admin/commit/aba43374dea425d87aea357c750ff3bfdff8c12d

@w00fz do you have an idea why that might be happening? It's an array field:

    ignores:
        type: array
        label: Ignore
        help: "URLs to ignore"
        value_only: true
        placeholder_value: /ignore-this-route

cannot add new items after this commit. If not I'll try some debugging tomorrow.

w00fz commented 7 years ago

Not sure how would that affect the slashes, it's specific to square brackets...

flaviocopes commented 7 years ago

Not sure. I just tested before and after that commit, leaving everything else unchanged. Going to test again.

flaviocopes commented 7 years ago

I found the problem is in this line

escaped_name = escaped_name.replace(/\[/g, '%5B').replace(/]/g, '%5D');

as escaped_name in my case is an integer (the id of the current array row) and it does not have a replace() method, so there is a JS error in the console and processing is halted. I need to add toString() before, then it works:

escaped_name = escaped_name.toString().replace(/\[/g, '%5B').replace(/]/g, '%5D');

Going to commit this, as I tested and got no side effects, also because toString() is also working fine on strings, not just integers.

flaviocopes commented 7 years ago

Also, found the issue is only present when value_only: true.