search

backdrop-contrib / feeds_tamper

Feeds Tamper provides a small plugin architecture for Feeds to modify data before it gets saved.
GNU General Public License v2.0
0 stars 2 forks source link

Invalid character in Config object name #1

Closed jenlampton closed 4 years ago

jenlampton commented 4 years ago

Thanks so much for porting this module :) I use it heavily on almost all my sites with feeds :)

I'm setting it up for the first time on a backdrop site, trying to import videos from a YouTube channel RSS feed using the feeds_xpathparser module. I want to use Tamper to hard-code a field value (instead of pulling from source). I chose the "Plugin" Set value or default value and entered my preferred value. Immediately on clicking Add I get an error:

Invalid character in Config object name feeds_tamper.youtube_videos.xpathparser:6.

I also tried the Rewrite plugin, but had the same problem.

I expect the problem is with the colon added by the feeds_xpathparser module to the source name, which I think may be an invalid character in a machine name.

I have a PR that replaces the colon with a hyphen, but maybe we should use something like system_transliterate_machine_name() to make sure we catch all possible bad characters?

jenlampton commented 4 years ago

I tried it with system_transliterate_machine_name() and it got really messy. It requires a language code, meaning the tamper would have different versions if the site had different languages, and I don't think that's what we want. I think replacing the colon alone might be a better way to go.

herbdool commented 4 years ago

I've been thinking about @laryn's suggestion of saving the feeds_tamper config inside the feeds config objects instead of having them separate. This would hopefully solve this issue you've discovered plus also making the feeds clone action work how people expect it to work by also copying over the tamper instances.

jenlampton commented 4 years ago

Ooh, I like that idea!

herbdool commented 4 years ago

@jenlampton @BWPanda I've got a PR to test https://github.com/backdrop-contrib/feeds_tamper/pull/9. It changes the structure of the config to store them in the feeds_importer mappings. This might help with @BWPanda other issue as well, not sure.

jenlampton commented 4 years ago

I can test it later today, I have a new feeds project to work on :)

herbdool commented 4 years ago

@jenlampton have you had a chance to test the PR?

jenlampton commented 4 years ago

The fix for this was merged, should we close this issue?