Closed (fixed)
Project:
Drupal core
Version:
x.y.z
Component:
profile.module
Priority:
Normal
Category:
Bug report
Assigned:
Reporter:
Created:
10 Oct 2006 at 09:31 UTC
Updated:
26 Nov 2006 at 19:46 UTC
Jump to comment: Most recent file
Create a new profile field (textfield).
visibility: Hidden profile field, only accessible by administrators, modules and themes.
Put in a value in this field for one user. (say at user/1/edit/yourfield )
You can later edit this value, change it, etc... but you cannot blank it out and remove it (i.e make the value in this profile field NULL or '').
| Comment | File | Size | Author |
|---|---|---|---|
| #13 | profile_31.patch | 4.05 KB | chx |
| #9 | profile_30.patch | 4.11 KB | RobRoy |
| #4 | profile_cleanup.patch | 3.98 KB | chx |
Comments
Comment #1
pwolanin commentedDoes this relate to the posts on the devel list about this part of the profile being saved in a serialized data field (if I remember correctly)?
Comment #2
beginner commentedIt's not directly related, but yes, I found this bug while trying to understand how the profile.module behaves.
Comment #3
chx commentedLet's follow the code flow. When saving a user, hook_user is fired with insert or update and then profile_save_profile is called. That function grabs some fields from profile_fields then iterates over them and inserts (in a bit clumsy way) the necessary values into profile_values. However, the fields will never include PROFILE_HIDDEN fields. Huh?
Grepping for PROFILE_HIDDEN only reveals similar visibility != %d lines , so I fail to see where these values are saved. Profile hidden was introduced very early in the 4.7 cycle by http://drupal.org/node/12737 . Is it possible that this was totally broken all the way?
Comment #4
chx commentedReading more: in profile_form_profile, if you have administer users perm then everything include profile_hidden is shown for editing. But it's never saved. (Chunks from http://drupal.org/node/21219 seems missing? But that issue never went in, or what?)
You saw them saved because they land in the big bad $user-.data .
I took a stab at code reusal while at it because it sounds quite logical that you are editing, validating and saving the same fields so the same DB query shall retrieve them. My preferences do not lie in writing four if-elseif. Instead, I have written two independent ones and stitch together the results at the end. If the powers that be say the if-elseif is better than the mini query builder I have written, just tell me.
Comment #5
chx commentedCursory look at 4.7 profile tells me that it's an Issue there , too.
Comment #6
flk commentedi cant seem to reproduce this on on the latest head.
i did the following
turned on profile module then went about creating a new textfield:..
category: test
title: test field
form name: profile_test
visibility: 'Hidden profile field, only accessible by administrators, modules and themes.'
then save field... inputted something into the created field, submitted then went back and deleted it leaving the field empty (submitted it again)..and i was able to save it as empty.
i cleared cache even an checked again...twas still empty empty.
Comment #7
chx commentedCheck the database. Is it saved in profile_values or user.data?
Comment #8
RobRoy commentedI created a profile_test field.
Before applying the patch:
- After creation of the field: user->data and profile_values are both empty obviously
- After saving a value 'test' for this field: user->data now has s:12:"profile_test";s:4:"test"; and profile_values is empty
- After erasing the value: user->data has s:12:"profile_test";s:0:""; and profile_values is still empty
After apply patch in #4:
- After creation of the field: user->data and profile_values are both empty obviously
- After saving a value 'test' for this field: user->data has no profile info and profile_values has the correct row/value (1, 1, 'test')
- After erasing the value: user->data has no profile info and profile_values has the row (1, 1, ''). Is the row supposed to be removed if there is an empty value?
So this gets the data out of user->data successfully. The only issue is if the row should be completely removed, but if that is the desired effect this patch is good.
+1 for getting the data out of user->data and into profile_values
Comment #9
RobRoy commentedRe-roll for HEAD (no changes, just didn't apply cleanly before).
Comment #10
chx commentedWhether storing (1, 1, '') is needed or not, that's another issue. What we wanted here is the effect RobRoy describes. Thanks for the through and detailed review.
Comment #11
dries commentedIt would be nice if we could properly unset the values so that the empty row gets removed. Not sure if that takes a lot of engineering.
I'd group the initialization of $args and $filter. Now there is a $sql in between.
This patch slightly changes the queries so it is tricky to see if it works properly. Need a fair amount of testing. Otherwise, it is a nice code clean-up.
Comment #12
drummI'd like to see arg() calls stay as close to menus as possible. I think adding arguments for the various conditions (such as $registration = FALSE) could make the code simpler.
If I understand Dries's comment, we are storing blanks and not empty rows. I'm okay with storing blanks; there is a difference between "never seen the field" and "saw the field and left it blank."
Comment #13
chx commentedbalnks are OK for the reason Drumm mentioned -- you are entering emptiness, we are storing emptiness.
Moving arg() closer to menu is something I would love but... changing hook_user (for this is fired by hook_user) is not really an option at this point.
So after permforming a simple line switch Dries asked, I am tucking this back to the green queue.
Comment #14
dries commentedCommitted to CVS.
Comment #15
(not verified) commented