XML-RPC returns invalid dates if the date is zero

[koke]

When a post has a 'pending' status, post_date_gmt is set to 0000-00-00 00:00:00 as a marker to update post_date when it's saved (see #5698).

mysql2date then proceeds to turn that date into a negative one, and IXR_Date destroys the thing a bit more, so 0000-00-00 00:00:00 turns into -0001113TT0::0::00, which is an invalid ISO8601 date

[koke]

Added a patch which sets the post_date_gmt value to the converted post_date for pending and auto-draft posts.

[DrewAPicture]

Related #16985 #18998

[nacin]

Looks good to me.

[koke]

Also, IXR_Date doesn't like "weird" dates.

• mysql2date returns -00011130T00:00:00. It looks weird but it makes sense: 0000-00-00 -> -1-12-00 -> -1-11-30
• IXR_Date works by taking substrings, and the leading '-' is what breaks everything.

Using a NULL for a 'zero' date might make sense, but I wonder how many things would break, probably including XML-RPC :(

[markoheijnen]

Added a new patch at #18429 that also fixes this issue. Do need to test it tomorrow if it works as it should.

[westi]

In [20153]:

XMLRPC: Intoduce a date generation helper method to improve the dates returned over XMLRPC when we have a 0 date stored for drafts.

This improves the ability of clients to work with the new wp Post APIs. See #18429 and #19733 props maxcutler.

[markoheijnen]

Added a patch that applies the patch of maxcutler to all the methods. I didn't applied the code of the first patch of this ticket. Is that still needed?

[westi]

I would really like to have the last patch in 3.4 but I also really don't want to commit it without us adding test cases because I just found a bunch of bugs in the featured image stuff we applied everywhere by writing the basic test cases for mw_(new|edit)_Post.

If someone wants to write up some tests to cover the date stuff that would be really helpful.

[markoheijnen]

I applied the patch of koke in a way that it only applies for incorrect dates. No matter what the post status is.
I also applied it to all the new XML-RPC methods so everything has the same date handling.

A benefit for this patch is that _convert_date and _convert_date_gmt only return an IXR_Date.

[westi]

I took a look at the last patch and all looks good apart from:

• _convert_mysql2date is a level of abstraction too far - lets just call mysql2date directly.
• $this-> _convert_date( $date_gmt ); - space in this.

@markoheijnen could you give the patch a refresh to address these?

[markoheijnen]

Fixed that in patch-core-19733-5.diff

[westi]

In [20353]:

XMLRPC: Make sure that we always return valid dates when no date is currently set - for example if the post is pending. Fixes #19733 props markoheijnen and koke.

[ericmann]

Unit tests for both _convert_date() and _convert_date_gmt()

[ericmann]

Same unit tests as proposed before, but using a subclass rather than reflection to access the protected methods.

[ericmann]

The above two patches add two different approaches to testing _convert_date() and _convert_date_gmt(). The first patch uses reflection to gain access to the protected methods in the XMLRPC server class. The second patch creates a subclass to expose them publicly instead. Both work (and pass) and I leave it to the committer to chose the best approach.

Note: While the Reflection approach is cleaner in code, the subclass approach is easier for new devs to understand. I would recommend committing 19733.tests-subclass.diff.

[markoheijnen]

I personally would go for 19733.tests.diff​. I think the reflection here is pretty easy to understand. It's a few lines of code and you did add a comment that you want to access a protected method.

I personally would split each test case by having a standard and a null test case.

[ericmann]

Replying to markoheijnen:

I personally would go for 19733.tests.diff​. I think the reflection here is pretty easy to understand. It's a few lines of code and you did add a comment that you want to access a protected method.

Easy to understand, but somewhat non-standard. Hence the second patch as an alternative.

I personally would split each test case by having a standard and a null test case.

While we could split them out, there's no real added benefit of doing that. Plus having the standard and null date in the same test method doesn't really have a drawback. That said, if anyone else disagrees it would be trivial to split them.

[markoheijnen]

#18681 was marked as a duplicate.