Date field on registration

[Mqlte]

Howdy,

I noticed a little bug: If a user doesn't fill in the date field on the registration page (e.g. birthday), it's automatically set to "1st January 1970" instead of hiding the field on the profile page.

Here, take a look at screenshot: ​http://img822.imageshack.us/img822/2889/bugreport.jpg

Regards

[Mqlte]

Bug

[DJPaul]

Confirmed, thanks.

[DJPaul]

Attaching a patch that fixes the issue. Would appreciate another dev to check it. It looks like we've got some duplicate xprofile logic. Wondering if best to put this patch into 1.5 branch, and refactor the logic between user registration and profile editing for 1.6?

[boonebgorges]

This looks fine to me, DJPaul. Let's go with this simple fix for 1.5.x. It's a good idea to abstract out some of the validation logic so we can be a bit more DRY between registration and xprofile edit, and if someone could find the time to do it for 1.6, it'd be great - but if it had to get pushed off to 1.7, your temp fix could go into 1.6 too.

[DJPaul]

Patch would need rebuilding for the 1.5 branch, as the diff doesn't apply. Any takers? :)

[Mamaduka]

1.5 does validation check for dates, only difference with this branch is, that it concatenate the values without white space between. ​http://buddypress.trac.wordpress.org/browser/branches/1.5/bp-members/bp-members-signup.php#L78

If needed, I'll provide diff with white space fix.

[djpaul]

(In [6083]) Stop date fields on the registration template defaulting to the unix epoch if field left empty. See #4187.

[DJPaul]

Decided to put in the 4187.patch into trunk for 1.6, and moving the ticket to future release so we can de-duplicate.

[lakrisgubben]

I've abstracted this check out into its own function, not at all sure though on the name of the function and where it should live. ;) Happy to hear thoughts on that!

I've also added a simple check to make sure that the POST data can be turned into a timestamp, so that any malformed data in the $_POST results in an empty date field being saved instead of defaulting to the unix epoch. I found 4 places in the code where this check was used and have changed them to use the new function, see attached patch.

[boonebgorges]

@lakrisgubben This is really great - thanks for the patch. I'm having a hard time applying it, though - I get a "patch unexpectedly ends in middle of line" error. Can you try regenerating it? (If it's very hard to do, I can try applying it manually.)

The whole idea of modifying $_POST values like this is a bit unsettling to me. I wonder if we might take this opportunity to put the values of $_POST into a separate variable before modifying. (This, in turn, would make it easier to write unit tests.)

[lakrisgubben]

Thanks for the reply @boonebgorges. Don't know what went wrong with the patch but I've made a new one that applies cleanly for me at least, hopefully it'll do that for you as well. :)

Regarding the modifying of $_POST I'll whole heartedly agree with you that it feels a bit icky, but not sure how to change that without also changing all of the logic that comes after the place in the code where the patch has switched to using bp_xprofile_maybe_format_datebox_post_data since all of that is tightly coupled to the $_POST data. Do you have any good ideas here, if so I can try to work that into the patch!

[boonebgorges]

In 11295:

XProfile: Abstract date-parsing logic during save routines into standalone function.

This ensures that validation works the same way in all contexts where
date fields can be saved.

Props lakrisgubben.
Fixes #4187.

[boonebgorges]

Thanks for the updated patch, @lakrisgubben!

Let's continue to think about how to clean up some of the awful mess in our xprofile save controllers. In the meantime, your patch is a huge improvement, so let's put it in.

[DJPaul]

Good job patching a scary part of the codebase!

[lakrisgubben]

thanks @DJPaul @boonebgorges :)