Multiple vocabularies assigned to abitrary profile categories

Hi!
Thanks for posting the code :)

I've just been made comaintainer of this module, so I'm giving it a tidy up and looking to make a 1.0 release soon.

I've briefly looked at allowing vocabularies to go into different categories as I may need it for a project myself, I ran into the same problem you clearly did:

In user_terms_save_profile, you're only given the form data for the current category. So the approach in the current module code of deleting everything in {user_terms} for user will kill their data for vocabs not in the current category.
So, suppose you have vocabs A and B, shown in profile categories A and B. I go to edit category A and save it -- this kills the data I had for vocab B!

Your code fixes this by storing the category along with the term-user relation in the {term_user}. I don't think this is the right way to fix this, as this will break if a site admin moves the vocab to be in a different category. It's mixing data up with presentation.

I think the right way to do this would be to look in $user->account, which has the existing data in it, then either pick out the terms from all the other categories and add them to $user_terms, or do as you do and delete selectively. Probably the former, as you can't delete and select in the same query (http://dev.mysql.com/doc/refman/5.0/en/delete.html).

Following on from my comment above, I reckon writing some tests for this would be a good idea.

I reckon we could use the technique I used on #728972: Terms are clobbered by user operations when terms are on account, and store in a hidden form value the vids of the vocabs currently being edited in the form.

#728972 is in and it puts a list of of vids into the form.

I've no idea when I will get round to this, as I'm very busy, so 1.0 will be released without it.

Here's a list of what needs doing in case anyone wants to tackle it first:

- create an admin UI for putting vocabs into categories. I think something like the table at admin/user/profile would be best. Easy to use and consistent with what users already use.
- show only those in the form
- in the save function, we need to (quite laboriously):
1. load the *existing* terms on the user AND their vids (so in other words, a JOIN on user_terms and term_data)
2. figure out which ones are NOT in vocabs currently being edited
3. add those to the array come in from the form
4. delete the current terms and save our new list, as in the existing code.

Quick mockup of what I have in mind.

#754642: Remove some vocabularies from the register page suggests we add a checkbox to set which vocabs are on registration.

On second thoughts, making it drag&drop may give users the wrong impression that they can order the individual vocabularies: they can't. Perhaps this can be covered with a node in the form? At any rate, I'm going to leave off drag&drop for now; it can be added later with Drupal core tabledrag if anyone feels like working on it :)

Here's a patch that makes a start. It makes the admin UI.

Still to do, feel free to jump in and help:

- some JS to disable the registration checkbox for disabled vocabs
- save the account category settings in a new variable
- update function to convert the old single category variable to the new one (ie, set all the vocabs to the old single value)
- only show the right vocabs in a particular category
- correctly save only the edited vocabularies (ie don't clobber the others)

I made real the following:

• some JS to disable the registration checkbox for disabled vocabs
• save the account category settings in a new variable
• update function to convert the old single category variable to the new one (ie, set all the vocabs to the old single value)
• only show the right vocabs in a particular category
• now 'default' category is available for selection in "Category" column new

But I didn't understand these:

• correctly save only the edited vocabularies (ie don't clobber the others)

Why is it necessary? We still save data in variables, so we need to change all variables, even if we change only 1 vocabulary.

Thanks for moving this forward! :D

+++ user_terms.js	2 Apr 2010 04:40:56 -0000
@@ -0,0 +1,14 @@
+    $('#edit-vocabularies-' + user_terms_vocabs[i] + '-enabled').click(onClick);

I wonder if the checkbox classes or distribution in the HTML could make them more jQ-friendly.
They are in the same table row; that might be something to use. If all the checkboxes had classes either 'enable' or 'register', then the jQuery traversing logic is:

- find 'enable' - bind click()
- go up to enclosing table row
- find register within this row
- affect it.
and we don't have to do the regexps.

+++ user_terms.module	2 Apr 2010 04:40:56 -0000
@@ -227,6 +218,17 @@ function user_terms_form_profile($edit, 
     $vids = variable_get('user_terms_vocabs', '');
+    if ($register)
+      $vids = variable_get('user_terms_vocabs_register', '');

Don't do an if without {}. Also, we can probably put the other assignment of $vids in an else clause for readability.

+++ user_terms.module	2 Apr 2010 04:40:56 -0000
@@ -227,6 +218,17 @@ function user_terms_form_profile($edit, 
+      $def_path = variable_get('user_terms_profile_path', '') == $category;

Shouldn't this variable have been deleted/replaced in the update?
Early morning review, so I may be wrong...
But isn't this the placement of all vocabs in a category -- hence we delete it and move the value to the new variable in the update.
I know I left it in my patch; it's meant to go :)

I'll have a more detailed look at this when more awake ;)

> * correctly save only the edited vocabularies (ie don't clobber the others)
> Why is it necessary? We still save data in variables, so we need to change all variables, even if we change only 1 vocabulary.

Because when you submit the form, you have to clear the existing data and save the new stuff.
Imagine with have profile cats A and B and vocabs A and B.
We save profile cat A. There is no way to clear only the terms from vocabs A! (Because we'd have to join to {term_data} to find out which vocabs a term is in and that's not possible in mySQL.)
So we have to delete ALL user terms for the current user. So now we've lost the terms for B, and we don't know what they are because they've not come in the form.
My plan for this was to store in the form which vocabs we're editing, then in the submit handler load up all terms to get a complete list of all the user's terms before we delete. Though it occurs to me we could more simply store ALL terms in the form, but put the ones that aren't being edited in as hidden values. Will ponder...

Powered by Dreditor.

1)Fixed. Great idea.
2)Fixed
3)No. Because there is special 'default' category. And it's stored in this variable.
4)Are you saying about "Edit profile" page?

> 3)No. Because there is special 'default' category. And it's stored in this variable.

I'm not sure I see a case for having a 'default' setting. You usually have those if you have to set something up in different places and there are many of them. How many vocabs does the average site have? Mine are usually 5-6, maybe a dozen in extreme cases.
Needs pondering... :)

> 4)Are you saying about "Edit profile" page?

Any edit page. Profile category forms come in with the full user profile and try to save everything.

I tested the clobbering, and the patch does it as expected.

This code takes care of that:

# in the submit function before we insert the data
  // Add into the array of terms to save the terms already on the account from
  // the vocabularies we did NOT edit: otherwise they will be clobbered by the saving.
  $unedited_vids = array_diff($vids, $edit['user_terms_vids']);
  foreach ($account->user_terms as $tid => $term) {
    if (isset($unedited_vids[$term['vid']])) {
      $user_terms[] = $tid;
    }
  }

I'm away for a few days; I'll reroll this + some code tidying when I get back.

Here's an updated version of the patch.

I've tidied up the code for Drupal coding standards, streamlined the jQuery a bit, and added the above anticlobbering bit.
I've also remove the default category thing as I think it's overkill and clutters the UI.

I've tested jQuery and drupal coding standart with the Coder module and think that everything is OK (except field_term_user_tid.inc:67, but I don't know how we can do it more secure and beautiful)
Also I saw new update function. It looks and works good

I think it will be better if joachim will make a CVS branch for this post. IMHO it's wrong when patch's size is more than the source size. :)

You mean a 6.x-2.x branch? I don't think that's necessary.

I think not 6.x-2.x yet. I don't know CVS philosophy, but in SVN when you develop something big feature you can create a special branch for it. When feature will be completed this branch is merging with trunk.

Hehe... that's on SVN. Here we're on clunky old CVS. Branches are only for release branches, AFAIK. (It'll be cooler when we move to git!)

I ran this with a test upgrade, and it didn't work -- the same variable in update_6002, $vocabs_path, was being used for two things. Also, somehow the admin UI variables were wrong too :/

I'd appreciate this being given a thorough spin before I commit it :D

I found a bug when new vocabs are chosen without profile module present.

Please test this patch if you want this feature to go in the module.

Hmm, this *seems* to work fine. I must confess, I have not tested it extensively enough to mark R&TBC conclusively, but I cannot find any problems with the tests I have done. UI works great, things appear where they should, when they should - all good. If that's good enough for you, commit away! I've tested most things, the only thing I have *not* tested is multiple vocabularies (which I suppose is rather the point of the patch).

OK, I stopped being a lazy **** and added a couple of vocabs to test further. I'm happy this patch works. Get it in!

Brilliant! Thanks :D

Committed. I really don't have time right now to work on any other patches for this module, so I'm going to roll a release with just this change to get it out there.

#656428 by gnindl, joachim, yeputons: Added ability to assign vocabularies to different profile categories.

hi,

I'm not seeing this in the functionality in the 6.-1.3 version that's packaged with drupal commons. Am I missing something? Just wondering if it's worth my while to update to the -dev version. Thanks!
Matt

It's definitely in 1.3.

thanks joachim, I'll try to figure out what I'm missing.