I think that it is a good idea to put the fields of the Addresses module in a fieldset. This way, it looks more in trend in comparison to the other entries.

I'm proposing a patch to that effect. This properly shows up in the Login area and the user Edit window.

The only thing would be to add a user setting to let the user decide whether all of these fields should be visible or hidden by default. (see comment in patch)

Thank you.
Alexis Wilke

CommentFileSizeAuthor
#6 addresses-6.x-1.x-edit-fieldset.patch2.74 KBAlexisWilke
#2 addresses_user-fieldset-6.4-1.x.patch1.45 KBAlexisWilke
addresses-fieldset-6.4-1.x.patch787 bytesAlexisWilke

Comments

brmassa’s picture

brmassa commented

Alexis,

the code didnt work here. I tried it on user addresses using the "only one address" settings. Did you use the "address book" feature?

regards,

massa

AlexisWilke’s picture

AlexisWilke commented
StatusFileSize
new addresses_user-fieldset-6.4-1.x.patch1.45 KB

Massa,

Ah! No, I checked only with the user, not the address book.

I noticed today that my Save feature was dead. So I looked into it and to make the whole thing work, we need to correspondingly change the save function in the addresses_user sub-module. But that would not fix the Address Book... yet, the fix to the sub-module may tell you how to fix the Address Book. Let me know, otherwise, I will look into it.

I'm attaching another patch (you need to apply both patches to get it to work for users.)

Thank you.
Alexis

brmassa’s picture

brmassa commented

Alexis,

nice. I will commit the patches if you confirm it works (both inserting / updating) CCK nodes and address book too.

Also, i think it would be simplier if the second patch would be only:

  *   depending on the $type variable
  */
 function _addresses_user_address_save($address, $id) {
   if (!empty($address)) {
+  if (isset($address['address_information'])) {
+    $address = $address['address_information'];
+  }

Thanks a lot for all this work on Addresses module. great help!

regards,

massa

AlexisWilke’s picture

AlexisWilke commented

Massa,

Watch out for that line:

  $addr['aid'] = $address['aid'];

Of course, we should probably fix the code that sets up the aid in the first place rather doing some kind of a fix here! And that could very well suffice to make the whole thing work in all cases.

Now, in regard to security, we should verify that the aid & eid do correspond (i.e. make sure that someone cannot change someone else address just by changing the aid in his form!)

I do not have CCK and I have never tested CCK... so I cannot really test that right now. I did not test the Address Book either... maybe I'll find some time for that tomorrow. I was working on a sub-module to get a list of all the users (if they are marked as members, but that's a long story!) I'm thinking I could make that module available at some point. It could be useful to other people, I'm sure.

Thank you.
Alexis

--- UPDATE ---

Well! I actually tried to create new users with the field set and it fails. So I have to look closer into that. I could be that the fields are defined at the wrong level in the array when you go to the register area. More later...

AlexisWilke’s picture

AlexisWilke commented

Massa,

Another update, I tried to fix the Create User and so far, not so good. The hook_submit() is not getting called anymore!

Now, reading the hook_submit(), it should be used to tweak the data if necessary (which is kind of what we're trying to do that, I think) and the hook_insert() and hook_update() are supposed to be used to save the data. That means th insert or update calls may not actually happen!

That being said, I did not get the insert nor the update called from anywhere anyway!

Now, the call to all the save hooks is wrong if we are supposed to be able to edit the $addr variable. I will post a patch and some info about that. There are a few other mistakes too, btw. The submit receives a reference to the form data, not a copy.

Okay, it's late now! 8-)

Thank you.
Alexis Wilke

AlexisWilke’s picture

AlexisWilke commented
StatusFileSize
new addresses-6.x-1.x-edit-fieldset.patch2.74 KB

Massa,

Alright... I worked some more on this one and it had nothing to do with the two other problems I now fixed (the address info from the registration that would not take, and the edit/update of the Address Book entries.)

Yet, I simplified my change and also I added a flag so on the registration page we can make the fieldset collapsed by default. (extra admin flag.)

I hope you like it because I think it looks a lot better with the fieldset.

Thank you.
Alexis Wilke

brmassa’s picture

brmassa commented
Status: Needs review » Fixed

Alexis,

commited, with some small modifications

regards,

massa

flebas’s picture

flebas commented

Massa,

The fieldset modification does not work with CCK.
I suggest to enable fieldset only for user address. To display CCK address in a fiedset, it is allways possible to use a CCK group.
I have send some code to Alexis to implement this.

In addresses_elements_process (addresses.module) : 

if ($form['form_id']['#value'] == 'user_profile_form') {
  //$use_fieldset = variable_get('addresses_user_fieldset', TRUE); // set in addresses_admin
  $use_fieldset = TRUE;

}
else {
  $use_fieldset = FALSE;
}

Regards,

Frederic

flebas’s picture

flebas commented
Status: Fixed » Needs work

Setting status to code need works since fielset change is not compatible with CCK (see my previous comment).
The fieldset #description item value should be surrounded with t().

Frederic

AlexisWilke’s picture

AlexisWilke commented

Massa,

Yeah... one of us will have to add some code to support CCK properly with the fieldset (i.e. not put the fieldset if the CCK module is turned on).

I did not test the CCK with the addresses module!

Thank you Frederic for posting the info here too.

Thank you.
Alexis

brmassa’s picture

brmassa commented

Frederic,

fixed. Its now only added on user forms.

regards,

massa

brmassa’s picture

brmassa commented
Status: Needs work » Fixed

Frederic,

fixed. Its now only added on user forms.

regards,

massa

Anonymous’s picture

Anonymous (not verified) commented
Status: Fixed » Closed (fixed)

Automatically closed -- issue fixed for two weeks with no activity.