Skip to:

Opened 11 years ago

Closed 6 years ago

Last modified 6 years ago

#4187 closed defect (bug) (fixed)

Date field on registration

Reported by: mqlte's profile Mqlte Owned by: boonebgorges's profile boonebgorges
Milestone: 2.8 Priority: normal
Severity: minor Version: 1.5.5
Component: Members Keywords: has-patch dev-feedback



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:


Attachments (4)

bugreport.jpg (180.6 KB) - added by Mqlte 11 years ago.
4187.patch (2.1 KB) - added by DJPaul 11 years ago.
4187.diff (6.4 KB) - added by lakrisgubben 7 years ago.
4187.1.diff (6.4 KB) - added by lakrisgubben 6 years ago.

Download all attachments as: .zip

Change History (19)

11 years ago


#1 @DJPaul
11 years ago

  • Milestone changed from Awaiting Review to 1.6

Confirmed, thanks.

#2 @DJPaul
11 years ago

  • Keywords has-patch dev-feedback added; needs-patch removed

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?

11 years ago

#3 @boonebgorges
11 years ago

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.

#4 @DJPaul
11 years ago

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

#5 @Mamaduka
11 years ago

1.5 does validation check for dates, only difference with this branch is, that it concatenate the values without white space between.

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

#6 @djpaul
11 years ago

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

#7 @DJPaul
11 years ago

  • Keywords needs-patch added; has-patch dev-feedback removed
  • Milestone changed from 1.6 to Future Release

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

#8 @lakrisgubben
7 years ago

  • Keywords has-patch dev-feedback added; needs-patch removed

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.

7 years ago

#9 @boonebgorges
6 years ago

  • Keywords needs-refresh added; has-patch removed

@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.)

#10 @lakrisgubben
6 years ago

  • Keywords has-patch added; needs-refresh removed

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!

6 years ago

#11 @DJPaul
6 years ago

  • Milestone changed from Future Release to 2.8

#12 @boonebgorges
6 years ago

  • Owner set to boonebgorges
  • Resolution set to fixed
  • Status changed from new to closed

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.

#13 @boonebgorges
6 years ago

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.

#14 @DJPaul
6 years ago

Good job patching a scary part of the codebase!

#15 @lakrisgubben
6 years ago

thanks @DJPaul @boonebgorges :)

Note: See TracTickets for help on using tickets.