Posted by mongolito404 on August 29, 2011 at 2:41pm
43 followers
| Project: | Address Field |
| Version: | 7.x-1.x-dev |
| Component: | User interface |
| Category: | bug report |
| Priority: | normal |
| Assigned: | Unassigned |
| Status: | needs review |
Issue Summary
currently, the only condition for an addressfield value to be non-empty is to have a country value. In many cases, a site may want to require other part of an address to consider it no-empty.
Address field should provide an option (at the field level, not the widget) to select which columns are required for a field value to be considered as non-empty.
Comments
#1
Here is a patch to implement this.
#2
#3
sThe previous patch has a typo, use this one.
#4
That looks pretty good. Should the validate function be changed as well to set a better error message?
#5
Subscribe. Patch applied cleanly and functioned asexpected.
#6
Thanks for the patch. I think it's a nice addition to the module.
The patch in #3 seems to be missing a default value for the setting in hook_field_info().
#7
After looking at this again i dont think this patch was functioning properly because both arrays (item and field[settings][non_empty] were fully populated with 0 values.
I just added an array_filter() on the $non_empty_fields setting and it works like it should because that removes the unused array items.
#8
Default settings arent necessary iirc. _field_info_collate_fields() takes care of setting them as empty for fields, widgets and formatters i think.
http://api.drupal.org/api/drupal/modules--field--field.api.php/function/...
If you would like to add them do you just want to use address_field_default_values()?
#9
Just confirming that #7 applies cleanly and appears to work. Not commenting on #8.
#10
#7 works great. it fixed #984788: Add "Remove" button to clear out values in a multi-value address field widget & #1358084: Multi-value field with default value keep adding new addresses for me! thanks
#11
Good work. Please commit!
#12
I can confirm that the patch in #7 works perfect for us. Please commit.
#13
Patch #7 applies, and I see the new interface - but it doesn't seem to do anything for me.
If I have only the default country set for a node, and in order for it to be non-empty I have only the Street Address ticked, I would expect the address to register as empty, and therefore not be displayed when the node is viewed. But I still have "Afghanistan" showing. Am I doing something wrong?
#14
@13: I think it works upon saving a node, not simply on displaying it.
#15
Marking #1436222: Editing profile adds new address entry with default values as a duplicate of this issue.
#16
As an even simpler fix, you can test for the default conditions. For instance:
/**
* Implements hook_field_is_empty().
*/
function addressfield_field_is_empty($item, $field) {
$defaults = &drupal_static(__FUNCTION__, FALSE);
// Every address field must have at least a country value or it is considered
// empty, even if it has name information.
if (empty($item['country'])) {
return TRUE;
}
// If the address consists entirely of default values, it is empty.
if (!$defaults) {
$defaults = addressfield_default_values();
}
foreach ($defaults as $key => $val) {
if (isset($item[$key]) && $val != $item[$key]) {
return FALSE;
}
}
return TRUE;
}
#17
I tested the modifications from amarcus #16 on my installation. It works. Empty fields are now interpreted correctly and drupal is not craeating new fields after pressing save.I created a patchfile.
When creating a new node, it always creates two fields.
#18
Hi All,
#7 patch by michaelfavia on -dev (2011-Dec-13) worked for me. One thing I noticed is I can no longer set a default State though. In field settings if I set a default State then hit save, then go back to edit my field settings the default state is still set to -- (nothing).
However, if I type in a city and choose a default state they both are saved.
Screenshot of new settings from the patch are attached as well as "what_I_want.png" and "what_happens_when_I_save.png"
Thanks for reading,
-Tim
#19
After patching the module, I don't have the option to specify the country when creating a node. Am I missing something?
#20
Tried patch at #17 and it works great with the country field, but other fields with default values doesn't seems to be considered default value and are still printed out. So a field with default country Canada and all empty fields will not be displayed but a field with default country Canada and default state Quebec will be rendered
It seems that addressfield_default_values() only retrieves the default country. Or maybe there's something I don't get about this function.
Am I the only one experiencing this behavior?
#21
#17 cannot work, as the default value is stored in $instance, but $instance is unfortunately not available in hook_field_is_empty(). Also see #1489484: Add $instance parameter to hook_field_is_empty().
Thus, I think the original issue feature-request is the best way to solve this issue now in addressfield. Thus, I took the patch from #7 and added the default for the setting hook_field_info() + tried to improve the labels and descriptions a bit, so it's clear how that works.
Patch attached.
#22
ops, wrong default value. fixed that.
#23
#22 worked beautifully for me! Had "Unlimited" values and "Hide the country when only one is available" chosen so multiple addresses were being created with only the country set on each node save. Switched to require administrative area and locality instead and the country-only ones have been stripped out. Thanks @fago and @michaelfavia!
#24
*replied to wrong issue*
#25
#26
#16 worked for us after a cache-clear-all. Patch in #22 did not work for us.
#27
>Patch in #22 did not work for us.
How that? Did you configure suiting required fields for being non-empty?
#28
I'm confused by this issue. It seems to overlap http://drupal.org/node/968112, which asks for the ability to have no country. It seems to me that what we need is a true "no address" condition that has no country.
#29
Apologies: the patch from #22 is against the latest dev (2011-Dec-11) not the latest release (2012-May-29) correct?
In any event, I've tried applying the patch to either version and get three fails:
# patch -p0 < address_configurable_empty_fields.patchpatching file b/addressfield.module
Hunk #1 FAILED at 300.
Hunk #2 FAILED at 312.
Hunk #3 FAILED at 348.
3 out of 3 hunks FAILED -- saving rejects to file b/addressfield.module.rej
The patch file itself is loaded at the module's root (ie, right next to addressfield.module)...clues happily (and humbly) received!
#30
boabjohn: you'll need to use "patch -p1" for patch files with git prefixes (which are the "a/" and "b/" bits before the file names).
#31
Hey Les Lim, thank you so much! I recall this instruction now...had blanked on it earlier.
One more tho: usual practice is to patch against dev...but in this case we've got the official release 5 months ahead of dev...so that's a bit weird?
Can you (or others) just clarify that we're actually still patching the Dec 2011 dev?
Thanks heaps,
JB
#32
Except for some meta-information from packaging, both should be the same. Dev almost always reflects the last commit on the branch. The release doesn't contain any changes since the last dev commit 5 months prior.
#33
#22 applies well on last dev version but seems not working.
I configured my field to be non-empty only if country, zip code and city are set but when I leave my form untouched, an entry is created in the database and my field is shown in front office.
I tried with an existing node and with a new one without difference. I also tried with and without #1316788: Do not display country in address block if only one country option is available to avoid potential side effects.
Any idea ?
#34
After digging a bit into the code I realized I had misread the field description.
Using #22 the address is non-empty if ANY of the checked fields have a value.
What I need is that ALL of these fields have a value for the address to be non-empty.
Here is a patch which add to #22 an operator field to choose which of the two process you want.
#35
>Using #22 the address is non-empty if ANY of the checked fields have a value.
That's usually covered by validation then, not? But the question is when validation should apply, with #22 this is as soon as one of the values has been entered.
#36
I just propose an improvement of #22, not a fix.
In my case I need my users to be forced to fill country, zipcode and city if they choose to input an address, but they can also leave it empty if there is no address needed on the node.
#37
What I need myself too and how I understood the case summary is a way to set up e.g. Firstname and Lastname field are required, but all others not so that every individual field can be selected as required. I have the need to use addressfield to be flexible for future, but for now for privacy reasons I need to be able to make all optional except the Firstname and Lastname.
Is this something that this patch addresses, too?
#38
This issue doesn't address making fields required vs. non-required. This issue solves the question, "which fields have to be filled out in order for this address to be considered non-empty?" Since the empty criteria is currently that the country field is empty, and there is no way to set an empty value on the country field, the problem is that there is currently no way of saving an entity without an address row in the database.
Requiring certain fields to be provided on an address seems like something that can be accomplished with a custom validator via hook_form_alter. The problem in this issue is not as easily solved because the hook that determines if a field is empty is only called on the module that provides the field.
We've been running #22 without problem for the last couple days. It solves our problem better than #968112: Add a special -none- country entry in the "Available countries" list in order to make the address field really optional., because it allows us to continue to use the "Hide country if only one is available" functionality.
#39
Isn't this not the same on the end of the day? If I enable street, house number, city and country and the few others country specific can be skipped/left empty I have the same; if I require Firstname/Lastname only. Than the form validation wines only on empty Firstname/Lastname, isn't it?
#40
Patch from http://drupal.org/node/1263316#comment-6448340 was reviewed and tested by me and team I'm part of now. It basically does it's job, we applied it and are going to use it as a standard unless anything better will be shown up here. Untill then I strongly insist to apply this patch - maybe it's not perfect, but it's way better than the current state of "any single field will do".
#41
IMHO, this still needs work from a UX perspective:
1) If the address field is not required, I fill out some fields, but my address does not meet the requirement for 'not empty', field is saved as empty without any warning / validation. Expected behavior would be if any fields are filled out (i.e. different from default) and the address does not meet 'not empty' requirements, to fail validation even if the field is not required, as user clearly was attempting to input an address.
2) If country field is hidden and forced to default, a field not meeting the criteria for 'not empty' is still populated with the country.
I'd be interested in sponsoring work to fix these issues.
#42
I think it should be possible to fix these behaviors.
Feel free to contact me to discuss about the details of this sponsoring.
PS : I am not an official maintainer but I can provde well formed patches.
#43
Here is a patch to answer #41 cases and to generally improve this module UX.
As I am not english native speaker, the descriptions and error messages that I choosed to provide some help to the users may not be perfect. You will find a list at the end of this post, feel free to propose something better and maybe more consistent with other messages in Drupal in general.
Test contextes
Common to all contextes
Label : AddressCardinality : 2
Non-empty parts : zipcode, city
Context 1
Required : YesNon-empty parts operator : All
Screenshot : context1.png
Context 2
Required : YesNon-empty parts operator : Any
Screenshot : context2.png
Context 3
Required : NoNon-empty parts operator : All
Screenshot : context3.png
Context 4
Required : NoNon-empty parts operator : Any
Screenshot : context4.png
Test cases
Common to all cases
We assume that "Delta 0" is the first delta and "Delta 1" the second. Reorder the two fields in drag and drop cannot change the described behavior.
Case 1
Delta 0 : Leave empty
Delta 1 : Leave empty
Case 2
Delta 0 : Fill the zipcode field only
Delta 1 : Fill the zipcode field only
Case 3
Delta 0 : Fill the zipcode and city fields only
Delta 1 : Fill the zipcode and city fields only
Case 4
Delta 0 : Fill the address1 field only
Delta 1 : Fill the address1 field only
Case 5
Delta 0 : Fill the zipcode and address1 fields only
Delta 1 : Fill the zipcode and address1 fields only
Results
Error messages
Results table
Contextes
Delta 1 : No error
Delta 1 : Error#Custom3
Delta 1 : No error
Delta 1 : Error#Custom3
Delta 1 : Error#Custom3
Delta 1 : No error
Delta 1 : No error
Delta 1 : No error
Delta 1 : Error#Custom4
Delta 1 : No error
Delta 1 : No error
Delta 1 : Error#Custom3
Delta 1 : No error
Delta 1 : Error#Custom3
Delta 1 : Error#Custom3
Delta 1 : No error
Delta 1 : No error
Delta 1 : No error
Delta 1 : Error#Custom4
Delta 1 : No error
Improvable strings
As said in the introductions, here are all the strings I added to acheive a better UX. Feel free to propose better ones.
Descriptions
Errors
Sponsor
This patch has been sponsored by obrienmd. Thank him !
#44
Wow DuaelFr! I hope to get this reviewed today, but this looks super thorough :)
#45
Looks like exactly what I need - thank you very much :)
#46
There are some strings with missing t() lik All, Any. Additional % placeholder is already checkplained automatically. Double check_plain need to be removed.
#47
Thanks hass for your advices.
#48
I strongly suggest o change the All and Any string to a longer string that is selfspeaking. Otherwise translators may not able to translate correctly.
#49
I am not a native english speaker so could you please suggest better sentences for these items ?
#50
So I updated the patch making the following changes:
- Moved the field settings into the instance
- Implemented an addressfield element for validation purposes
- Got rid of double asterisks and used built-in required properties
- Cleaned up messages
Here is how the settings work:
- if field is required
- if field op is or
- one or more non-empty fields are required
- else op is and
- all non-empty fields are required
else field is not required
- if field op is or
- one or more non-empty fields are required for field to be considered non-empty
- else op is and
- all non-empty fields are required for field to be considered non-empty
#51
So, the original patch as sponsored in #43 solved a UX issue our users were having, in that it provided validation for a non-required field missing required elements, rather than zeroing it out.
If a user accidentally forgets to add postal code to a non-required address, but otherwise completes the address, they clearly meant to add that address - that should fail validation with a message that postal code is required. Using patch in #50, it just saves that field instance as empty. This confuses users, who don't know why the data they just entered is now missing.
There's a bit of a wrinkle in that county can be hidden if only one is available, so the logic should be as follows:
If any visible field is filled in an address, then should fail validation if non-empty requirement logic is not satisfied.
Else, save address as null/zero/whatever (no data, not even hidden country).
#52
Thanks for the additional information @obrienmd. We should be able to detect if the user was attempting to populate the address if individual field values are not equal to their respective default values. I will roll a new patch in the next couple days.
#53
Cool! I hacked something together, but it's not really patch-worthy :)
#54
I have updated the patch from #50 to set a form error if any address element field has a value other than the default value, checking if the user attempted to populate the address field.
Here is how the logic works:
There are two problems I have come across, but I think these should be handled in separate issues:
#55
It seems to me the patch in #54 has some issues.
When you have a field within the address field which is not required and does not have it's default state it will throw the form error because the valid state is set to FALSE by default.
Let's go trough the code step by step.
1. On line 452 the default value for $valid is set to FALSE
2. In the foreach loop on line 461 the condition for
if($field[#required])returns false so $valid will never be set to TRUE.3. If the last child field in the address field does not have it's default value the $default variable will be set to FALSE.
4. On line 482 both $default and $valid are set to false resulting in a form error to be set.
The entire $valid and $default check needs work. However when you set the default value for $valid to TRUE when
$element['#non_empty_fields_op'] == 'or'results in TRUE at least you can save your node.So in other words, changing
$valid = $element['#non_empty_fields_op'] == 'or' ? FALSE : TRUE;to
$valid = $element['#non_empty_fields_op'] == 'or' ? TRUE : FALSE;will at least enable you to save the node.
#56
Attached is an updated patch that fixes a bug where checking if the address was empty was returning TRUE in cases where it was in fact not empty.
@ericmulder1980
Per your comment at #55:
1. The valid state is only set to FALSE by default when the operator is set to OR
2. We do not need to set $valid to TRUE. As long as $default is TRUE, no errors will be thrown
3. If a single field is not in a default state, default should be set to FALSE. Position is not important
4. This should trigger an error
If you are populating a field non-required field and are not populating at least one required field with the operator set to OR, this will trigger an error because you are not meeting the minimum requirements you set.
#57
Applying that patch, I have the following errors:
Notice : Undefined index: non_empty_fields dans addressfield_generate() (ligne 132 dans sites/all/modules/addressfield/addressfield.module).
Notice : Undefined index: non_empty_fields_op dans addressfield_generate() (ligne 133 dans sites/all/modules/addressfield/addressfield.module).
Notice : Undefined index: non_empty_fields dans addressfield_field_widget_form() (ligne 694 dans sites/all/modules/addressfield/addressfield.module).
Notice : Undefined index: non_empty_fields_op dans addressfield_field_widget_form() (ligne 695 dans sites/all/modules/addressfield/addressfield.module).
Notice : Undefined index: non_empty_fields dans addressfield_generate() (ligne 132 dans sites/all/modules/addressfield/addressfield.module).
Notice : Undefined index: non_empty_fields_op dans addressfield_generate() (ligne 133 dans sites/all/modules/addressfield/addressfield.module).
Notice : Undefined index: non_empty_fields dans addressfield_field_widget_form() (ligne 694 dans sites/all/modules/addressfield/addressfield.module).
Notice : Undefined index: non_empty_fields_op dans addressfield_field_widget_form() (ligne 695 dans sites/all/modules/addressfield/addressfield.module).
Can you help me ?