Terms are clobbered by user operations when terms are on account

Please could you try this with the latest dev code?

I have just tried this on my test site running the latest code and it all works fine. The new rows appear in {user_terms} as soon as registration is complete, and when I log in as my new user I see the terms on my account page and in the edit form.

BTW, I really recommend the use of several browsers for this sort of thing, eg Firefox for admin and Chrome for peons! :)

The problem is the combination of admin approval and having the terms on the account page.

This works:
- admin approval
- no email confirmation
- terms on profile category

This works too:
- admin approval
- email confirmation
- terms on profile category

This does not work:
- admin approval
- no email confirmation
- terms on account

The simple way to reproduce this:

1. Find a user with user terms
2. Go to admin/user/user
3. Either block or unblock them.

When doing the (un)blocking operation, here's what happens in hook_user:

- about 6 loads (this HAS to be a bug in core)
- update (which still has the $user->user_terms
- a load, which NO LONGER HAS THEM.

The problem seems to be that user_terms_save_profile() doesn't work properly when invoked programatically, ie with data that has come straight from the object we loaded rather than a formAPI array. It deletes all the rows, but appears to have nothing to work on to reinsert them.

Shouldn't be too hard to debug and fix, but I am very busy for the rest of this week, so feel free to beat me to it :)

BTW, thanks for reporting this and doing the debugging -- I'm very glad this has been caught before a 1.0 release.

While this is not exactly a duplicate of #730346: user_terms_save_profile may delete user term, it is possible that the solution would take care of this as well.

At the end of user_terms_save_profile, there is $edit['user_terms'] = NULL;. This deletes the update from $form_state['values'] (or its equivalent) so that if this function is re-entered, the user term for the user is re-deleted, but not re-inserted. For an example of code that can trigger this, see the comment on http://api.drupal.org/api/function/user_authenticate/6. I have commented out that line and see no ill effects.

I was just looking at the -dev code. Something that always sends shivers down my spine is unsetting things - especially in objects. It is often overlooked, but if you do something like

  $object1->fielda = 1;
  $object2 = $object1;
  $object2->fielda = 2;

Guess what the value of $object1->fielda is? Not 1, but 2. Unlike arrays, an object assignment merely copies a pointer, it does not instantiate a new object. If you want a copy of an object to modify at will, you need to use drupal_clone.

The reason for this is that the user operations call

user_save($account, array('status' => 1));

Which means that our own user saving function user_terms_save_profile() gets called with an $edit array with only the status key. There is no way of knowing that the user operations is where we've been called from, so it's as if a user was being saved manually with no terms selected at all.

(Aside: I don't see the point of this functionality of user_save(), as you could just as easily alter the $account object first which you have to pass in anyway. Note to self: fix this in core.)

One way to fix this would be to disallow user terms on account pages. This is how core Profile module gets around the problem!

However, here is a patch that fixes the problem. It's a bit of a hack, in that we check the $edit array for signs that we have something -- anything -- to save regarding terms. If the $edit array has a key of the form user_terms_VID for any of the enabled vocabs (we just take the first one, it doesn't matter which) then we're fine. (Even if all terms are unselected, there will still be that array there, it'll just be an empty array).

Please could you test this?

It will be sometime next week before I can test because we are launching Tuesday. I got around the problem by removing the DELETE query for now.

> I got around the problem by removing the DELETE query for now.

That means you can never remove a term from a user!

It occurs to me that a slightly more graceful method might be to put in a FormAPI value into the form when we edit it, and use that to know that we have terms to save.

Here's a patch based on that approach. Rather better.

I might do more work on this in light of #656428: Multiple vocabularies assigned to abitrary profile categories, and rather than just store TRUE, store the vids being edited: two birds, one stone :D

New patch.

Instead of just a TRUE, it stores the array of $vids. This opens the door for some fancy footwork to figure out which vocabs the user is trying to edit (and therefore only delete terms from those vocabs) over at #656428: Multiple vocabularies assigned to abitrary profile categories.

No reviews yet and it's been a while; committing and rolling a new beta.

#728972 by joachim: Fixed data deletion by user operations when terms are on the account form.

I have the patches for #10 and #14 and seem to be working fine. Now I will have to change my code.

Nancy, this is now in the latest beta :)

The manager is on jury duty; when he returns, I will ask him if he wants to install a beta. I will still have to change my code because I used $updates['user_terms_edited'] = TRUE;. This is in mission-critical code and the issues we had getting it to work make everyone nervous about making changes.