Once one enables Forum module the "Parent" field become hidden in all terms form (forum and non-forum), I think this due to the following code in forum_form_alter function

if ($form_id == 'taxonomy_form_term') {
unset($form['advanced']['parent']);
}

Maybe the function should ask if (isset($form['vid']) && $form['vid']['#value'] == $vid) before disable the parent option.

Support from Acquia helps fund testing for Drupal Acquia logo

Comments

catch’s picture

Version: 6.x-dev » 6.0-rc2
Priority: Critical » Normal
FileSize
2.42 KB

This was my fault, introduced with the taxonomy forms cleanup when parents were switched on by default for all vocabularies. Shobaki's entirely correct. I also moved the variable_get to the top of the function which removes an if if and prevents a second one being introduced. Note this makes the patch look bigger than it actually is due to indentation.

catch’s picture

Version: 6.0-rc2 » 6.x-dev
Priority: Normal » Critical
Status: Active » Needs review

Also bumping to critical since this breaks functionality in unexpected ways. Please test!

Emad’s picture

Version: 6.0-rc2 » 6.x-dev
Priority: Normal » Critical

works perfect, now parent disabled only for forum terms,
thank you for the quick patch

cburschka’s picture

Followup problem: Unsetting the parent field, which after this patch will only happen in the forum vocabulary, may be responsible for a notice in taxonomy.admin.inc:taxonomy_form_term_submit().

If it is, perhaps this patch could try to fix its notice while it's at it - possibly by using something other than unset() - '#type' = 'value'?

cburschka’s picture

Applied patch, tested, turns out this does cause the notice.

I'm changing unset() to the following:

    $form['advanced']['parent'] = array(
      '#type' => 'value',
      '#value' => array(),
    );

This removes the notice without requiring a patch to taxonomy.module, and has the same effect.

Gábor Hojtsy’s picture

Status: Needs review » Needs work

What about wrapping it all in an if (isset($form['vid']) && $form['vid']['#value'] == $vid) and branch to the two form types from there? Would save us some repeating code, and would cleanup up how we focus our stuff on the proper taxonomy forms.

cburschka’s picture

Status: Needs work » Needs review
FileSize
1.76 KB

Done.

Incidentally, there is one setting in the "designated forum vocabulary" that is reminiscent of the role settings in the user editing form. Namely, the designated forum vocabulary must always be attached to the forum topic content type, so this checkbox should technically be disabled.

It's probably not worth repeating what was just committed to user.module, as that change has to work around form API quite inelegantly. But if Form API gains support for individually disabled checkboxes in the future, both of these forms could use it.

chx’s picture

Status: Needs review » Needs work

Hiding form elements are best done with slapping an #access FALSE on them.

cburschka’s picture

Status: Needs work » Needs review
FileSize
1.7 KB

For sure!

cburschka’s picture

I'll be gone for at least a week. Let this be the final version, or someone else will have to take care of it.

Gábor Hojtsy’s picture

Looks good except that we use much more "elseif" then "else if", so I'd use "elseif". This is a minor nitpick I can modify on commit. Otherwise with some more testing this looks good.

keith.smith’s picture

I rerolled this to use "elseif" instead of "else if", and to add a period at the end of one of the code comments (and since Arancaytar is afk). (Please no credit on commit for a two-character change.)

keith.smith’s picture

Status: Needs review » Reviewed & tested by the community

Sorry, I should have tested this as well, while I was here.

On an *unpatched* copy, I created a new vocabulary and gave it some terms. Noticed that I received a notice regarding "parent" when creating a term, and that the Parent selector did not appear in the Advanced options fieldset for my new, non-forum related vocabulary.

Patched my copy. Created more terms for the same vocabulary, and noted that I did not receive a php notice, and that the Parent select appeared properly. Assigned some terms as parents of other tags, and confirmed that the hierarchy was reproduced properly when listing terms.

Went to the forums vocabulary and confirmed that the Parent select was, indeed, still not present.

So, as far as I can tell based on that, this is fine.

Gábor Hojtsy’s picture

Status: Reviewed & tested by the community » Fixed

Code looks good, reviews are positive, so committed! Thanks.

Anonymous’s picture

Status: Fixed » Closed (fixed)

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