Problem/Motivation

Find/Replace functions have a test to verify that the data is a string before acting on it. However when you try to parse an ID inside a feed and match it to a taxonomy term, the integer data will return an error "Input should be a string".

Steps to reproduce

Try to add a plugin FindReplace to a JSON feed with integer ID to match an existing taxonomy term name.

"category": {
  "id": 123,
  "label": "Feed category name "
},

So here we would like to replace 123 by our existing Taxonomy term "Website category A" so the match can be done whenever the label in the field would change.

Proposed resolution

Make a broader test to include numerics for those tests :

    if (!is_string($data)) {
      throw new TamperException('Input should be a string.');
    }

Also should booleans be allowed too ?

Remaining tasks

Patch to review

User interface changes

None

API changes

None

Data model changes

None

Comments

tostinni created an issue. See original summary.

tostinni’s picture

Status: Active » Needs review
StatusFileSize
new1.92 KB

Here is a patch to allow numeric inside Find/Replace & sprintf plugins.

Status: Needs review » Needs work

The last submitted patch, 2: 3170553-2.patch, failed testing. View results

jamesdixon’s picture

Thanks for the patch.

Looks like we need to update the test to check for the exception if it's not a string or numeric and check that the exception string is "Input should be a string or numeric." instead of the old value.

tostinni’s picture

Status: Needs work » Needs review
StatusFileSize
new3.3 KB

Here is the new patch with the fixed test and also allowing numeric for StrPad plugin.
I don't think this need additional tests as the existing already cover the scope ?

jrochate’s picture

Thanks. It works great.

An alternative could be a tamper to convert number to string, so we could cover these and other cases where the input is numeric.

nwom’s picture

Status: Needs review » Reviewed & tested by the community

This worked perfectly. Thank you so much for the patch!

megachriz’s picture

StatusFileSize
new4.14 KB
new6.76 KB

Here is the same, but with tests. Tests only patch should fail.

The last submitted patch, 8: tamper-3170553-tests-only.patch, failed testing. View results

  • MegaChriz committed b7a818b on 8.x-1.x
    Issue #3170553 by tostinni, MegaChriz, jamesdixon, jrochate, NWOM: Allow...
megachriz’s picture

Status: Reviewed & tested by the community » Fixed

Committed #8!

Status: Fixed » Closed (fixed)

Automatically closed - issue fixed for 2 weeks with no activity.

jrochate’s picture

StatusFileSize
new554 bytes

Actually I had to add a check for boolean value in order to work.

Something like the patch below, applied after the above commit.

Tamper was sending an error message on line 100, FindReplace, because of a value of "true" that wasn't numeric nor string.