WordPress.org

Make WordPress Core

Opened 3 years ago

Last modified 17 months ago

#21977 new defect (bug)

Allow 0, NULL and empty string values for 'parent'.

Reported by: sam2kb Owned by:
Milestone: Future Release Priority: normal
Severity: normal Version:
Component: XML-RPC Keywords: has-patch dev-feedback 2nd-opinion needs-testing
Focuses: Cc:

Description

We must allow 0, NULL and empty string values for 'parent' in wp_newTerm and wp_editTerm

Attachments (4)

p (1.1 KB) - added by sam2kb 3 years ago.
21977.diff (926 bytes) - added by markoheijnen 3 years ago.
Patch from root instead of wp-includes folder
unittest.21977.diff (2.8 KB) - added by markoheijnen 3 years ago.
Changed unit tests for this patch
ticket.21977.2ndopnion.diff (1.3 KB) - added by hrishiv90 17 months ago.
Modified patch for the parent reset of term via xmlrpc

Download all attachments as: .zip

Change History (13)

@sam2kb3 years ago

  • Attachment p added

comment:1 @nacin3 years ago

  • Milestone changed from Awaiting Review to 3.5
  • Severity changed from major to normal

I don't think this is major — the workaround is to simply not pass 'parent'.

But yes, switching from isset() to ! empty() makes sense.

comment:2 @nacin3 years ago

Looks like test_parent_empty() will need to change, and probably be expanded a bit: http://unit-tests.trac.wordpress.org/browser/trunk/tests/xmlrpc/wp/editTerm.php. A similar test does not exist yet for wp/newTerm.

comment:3 @nacin3 years ago

  • Type changed from enhancement to defect (bug)

I think it is safe to call this a bug.

@markoheijnen3 years ago

Patch from root instead of wp-includes folder

@markoheijnen3 years ago

Changed unit tests for this patch

comment:4 @markoheijnen3 years ago

Changed the unit tests and added extra ones to wp.editTerm and wp.newTerm. Also updated the patch of Sam to have it from the root instead of the wp-includes folder.

comment:5 @nacin3 years ago

  • Keywords dev-feedback punt added

Does this patch allow for a term with a parent to be set to then have no parent?

comment:6 @westi3 years ago

I'm not sure about this.

By definition the datatype that should be passed here is a string containing an integer, this is the return value of get_term and the new/edit functions should expect the same.

So ideally the correct value for parent is probably and empty string or '0' and all the tests should be updated to send numbers as strings.

I'm not sure [UT1118] adds completely correct tests.

comment:7 @nacin3 years ago

  • Keywords punt removed
  • Milestone changed from 3.5 to Future Release

comment:8 @nacin3 years ago

  • Keywords 2nd-opinion added

@hrishiv9017 months ago

Modified patch for the parent reset of term via xmlrpc

comment:9 @hrishiv9017 months ago

  • Keywords needs-testing added

I have added modified patch solution which allows to reset the term parent to 0 (i.e no-parent), and also handles properly the null and empty values for parent.
https://core.trac.wordpress.org/attachment/ticket/21977/ticket.21977.2ndopnion.diff

Note: See TracTickets for help on using tickets.