Moeshe,

Have made a drupal 6 port of the code you created on the 10th June 2008.

It is just a straight port with nothing else done to it.

One issue we did run into was MENU_MAX_PARTS is set to 7 in Drupal 6 and this causes the add term piece not to work unless we increase it.

I was wondering if we can change the menu path to add_term instead of add/term for the menu item below 'node/%node/og/vocab/terms/%taxonomy_vocabulary/add/term'

If we can then it will make the module work without any changes to core. I have put in a bug request to core already to determine if it can be altered but i am not sure if it will.

The code for the d6 port can be found at

http://apollo.openband.net/drupal-modules/og_vocab/og_vocab.tar.gz

Please let me know if you need diff's against the d5 version or if the above will suffice.

Comments

moshe weitzman’s picture

Status: Active » Needs review

Hi Darren. Thanks for this. The tarball format is fine ... Yes, feel free to change the url structure as you proposed. Increasing that constant is not a good idea since it forces the menu callback system to work much harder.

I will try to review and commit this soon.

Your feedback on D6 OG is much appreciated. I have been meaning to release an alpha for some time, but just have one annoying Views bug to figure out.

darren.ferguson’s picture

Moshe

Yes, understood, i actually work for Evgeny Kaplun so i have been following your progress with OG because the work i am doing for him is tied into the OG piece.

Glad you are working on the OG piece it is a really nice addition to Drupal and the pieces i am tying into make it very helpful for me.
Looking forward to the Alpha

darren.ferguson’s picture

StatusFileSize
new10.57 KB

Moshe,

Altered the menu structure so that it would not go over the MENU_MAX_PARTS limit.

Tar ball attached is D6 working copy.

moshe weitzman’s picture

Status: Needs review » Needs work

Thanks. I have commited this code to a new DRUPAL-6--1 branch. Lets make future patches from there ...

I noticed the following issues. Hope someone can tackle them. Should be straightforward:

  • og_vocab_form_alter - use individual functions for each form. see http://drupal.org/node/144132#form-id-alter
  • alter of 'taxonomy_form_vocabulary' not working. use og_get_types() function instead of variable.
  • alter of 'taxonomy_form_term' not working. use og_get_types() function instead of variable.
  • i am pretty sure that hook_menu() is not called for each page View so you can't use a dynamic variable like $user in hook_menu(). Remove that and hard code global $user in the 'determine access' function. i think the current approach actually passes the same user object every time. yikes. i have not tested this though.
  • if (function_exists('drupal_set_content')) no longer needed. hook_init() always runs after bootstrap in D6
  • various links use old l() parameters. example: l(t('add vocabulary'), "node/$gid/og/vocab/add/vocabulary", array(), drupal_get_destination());
  • Remove: drupal_set_message('Returning False');
  • require_once drupal_get_path('module', 'taxonomy') . '/taxonomy.admin.inc';. Use module_load_include()
  • why do we require php = 5.1? is it easy to work on PHP4?
darren.ferguson’s picture

Moshe

Thanks for the feedback regarding the module.

Will re-implement those changes per the information and will attach the patch file.

darren.ferguson’s picture

StatusFileSize
new4.38 KB

Moshe, took everything you said above into consideration and re-did the module.

* form alter moved 2 of them to their own functions. the third is still there because it is dynamically built.

* used the og_get_types, had to put 'omit' as the parameter passed to the function in order to make it not complain.

* hook_menu() yes, that would have been an issue, resolved and the check function now utilizes it via global $user.

* Removed the check for the function per your instruction.

* fixed the links that were still using the old API

* Removed the debug message

* This function was no longer required since the module now uses file and file path in the menu so just removed it completely.

* Put version for PHP to 4 since it will work with that. No need to have it as version 5.

CVS diff patch against Drupal-6--1 is attached.

Please let me know if there are any further issues with this.

moshe weitzman’s picture

Status: Needs work » Fixed

Committed with the following changes:

- og_get_types('omit') does not return any node types. 'omitted' is the right string here. i'm open to suggestions on a clearer API here.
- the form alters for both vocab and term forms did not work because the form structure has changed
- term form alter lost this code: $vocab = taxonomy_get_vocabulary($form['vid']['#value']); also, that function is now taxonomy_vocabulary_load()
- added the needed .info change

this still isn't 100% tested but the obvious stuff is working.

darren.ferguson’s picture

Moshe,

Thanks for the changes. I must have taken that piece out with the taxonomy_vocabulary_load.

I think that was all the changes needed to make the module work on D6.

Will commit to our tester group for testing if any issues will submit patches.

Thanks for checking the code and helping with the pieces i missed.

Anonymous’s picture

Status: Fixed » Closed (fixed)

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

zirvap’s picture

Status: Closed (fixed) » Active

If the port to 6.x is finished, shouldn't there be a 6.x version for download on the project page? Or am I missing something obvious?

I assume I can just download the files from http://cvs.drupal.org/viewvc.py/drupal/contributions/modules/og_vocab/?p..., but if there's a committed version it should be on the project page as well.

moshe weitzman’s picture

Status: Active » Closed (fixed)

I just added the dev snapshot download. Sorry about that.