Support from Acquia helps fund testing for Drupal Acquia logo

Comments

drewish’s picture

FileSize
69.92 KB

forms now use pull model.

drewish’s picture

FileSize
70.04 KB

added dependencies .info files

DayShallCome’s picture

Is there a plan to switch CVS to 5.0 in the near future? Or is that a far way off?

Jaza’s picture

@DayShallCome: 5.0 upgrade is not that far off. At the moment, the main thing that needs to be done before the upgrade is the tying up of a few more bugs. Also, some of the modules in the /contrib directory cannot be upgraded until their dependent external modules (i.e. views, pathauto) are upgraded to 5.0. And it would be a help to category if CCK was upgraded to 5.0 soon (although strictly speaking, category is not dependent on CCK).

drewish’s picture

uh, are you speaking about Drupal in general or just about your module? CCK has been 5.0 compatible for a while. sure there are still some bugs but it works... category on the other hand could be ready to go.

bdragon’s picture

Status: Needs review » Needs work
FileSize
74.47 KB

Working on bringing this up to date again. This is a work in progress.

Made a start at 5.0izing the menu. There's more arg() to check though. (I double checked category.module, but I still need to check other arg().

Also added hook_simpletest in prepration for having proper testing. (Mainly because I didn't bother to take it back out before rolling the patch.

Only category.module is tested, things are likely broken due to the menu changes.

I also envision a problem with directly querying $node->type (The content-types-in-core stuff, to be exact...)

bdragon’s picture

Now that I think of it, just plain ripping out hardcoded category_cat and category_cont and converting to content-types-in-core would make things a bit simpler and cleaner.

Jaza’s picture

@bdragon: I think that the category-cat and category-cont node types should still be provided, and that being module-defined node types, they should still be defined through hook_node_info() (if they're enabled, of course). The new node type system in 5.0 supports a clear distinction between user-defined and module-defined node types, and the category-cat and category-cont node types definitely fall into the latter category.

bdragon’s picture

But there's all this code to add the ability to enable/disable category_cat and category_cont if people wish to use CCK. By converting the two types to user types, the users can just delete the content types if they want to use their own.

(See what happened to the Story and Page modules ;)

bdragon’s picture

(I meant with the first sentence that there's all this code that just plain isn't needed in 5, as we have content types built in.)

bdragon’s picture

FileSize
75.31 KB

Content type settings forms work different than they used to.

Need to write a category_update() to fix up the variables, as I had to change the naming convention:

Due to node types being automatically appended in the content type settings page in Drupal 5, any "category_nodeapi_(content type)_containers" variables are now "category_nodeapi_containers_(content type)" instead.

bdragon’s picture

FileSize
75.84 KB

Something like this should do nicely...

(Only change is to rename some variables during update 9 and add some comments.)

bdragon’s picture

FileSize
76.2 KB

fix an additional instance of variable_get('category_nodeapi_'. $type .'_containers')...

bdragon’s picture

FileSize
76.2 KB

Whoops, screwed up one of the hunks. This one won't cause patch to bomb.

sym’s picture

Patch installed cleanly, but there are a few errors.

1) category legacy menu item displays under admin, not under admin > Content management > category
2) The taxonomy wrapper shows as installed even though it isn't.
3) There is a CCK error on the the add category and container pages. I'm not sure if this is to do with CCK or category, but I've not seen it before I installed category with this patch.

Apart from this, is there anything else stopping it going in?

Permanently Undecided’s picture

Subscribing to this issue

forngren’s picture

subscribe

DayShallCome’s picture

Patch seems fairly solid to me. It going in soon?

Jaza’s picture

FileSize
154.91 KB

Drewish and Bdragon, thanks for your work on getting the ball rolling with this, and for taking the initiative to start work on the 5.0 upgrade patch for category. I've finally gotten around to reviewing the patch, and to working on it myself, with the aim of bringing it closer to readiness for being committed to HEAD.

Updated patch attached. Some of the things that I've fixed up so far:

  1. All modules in the category package have .info files (with proper dependencies defined).
  2. All hook_link() implementations now return structured links (this was already mostly done, but I finished it off).
  3. The 'category settings' page is moved from 'admin/settings/category' to 'admin/content/category/settings' (so now everything category-module-related is centralised under 'admin/content/category').
  4. Changed some drupal_set_html_head() calls to drupal_add_css().
  5. New book.info.copyme and taxonomy.info.copyme files in the '/wrappers' directory.
  6. Updated the wrapper install/uninstall script to copy over the wrapper .info files for book and taxonomy.
  7. Changed hook_nodeapi('view') to add structured elements to $node->content, instead of the crappy 4.7 method of appending output straight on to $node->body.
  8. Fixed up links and URL paths that have 'category_cat' or 'category_cont', to instead have 'category-cat' or 'category-cont' (remember, in 5.0 node type names have underscores, but those underscores are converted to dashes when used in URLs).
  9. Fixed up most t() calls to use proper placeholders (may still need more work).
  10. Changed RSS output to use the new drupal_add_feed() system.
  11. Fixed up a whole bunch of menu paths to conform to the new 5.0 admin menu hierarchy.
  12. Updated the category API functions, and the book and taxonomy wrappers, to be consistent with changes to the core book and taxonomy API functions for 5.0.

This is still a work-in-progress. Will be posting another patch soon.

Jaza’s picture

Status: Needs work » Needs review
FileSize
190.95 KB

Updated patch. Changes:

  1. New hook_enable() implementation that automatically installs the book and taxonomy wrappers when category is enabled (only if the core book and taxonomy modules are already enabled).
  2. Finished upgrading and testing category_legacy(). I've tested taxonomy import with the patch applied, and it works about as well as it does in 4.7 (that is, not perfectly, but pretty good).
  3. Added the new core jQuery 'tableselect' functionality to the tables in the category_legacy UI.
  4. Finished upgrading and testing category_bulkedit(). Despite the claims by some that multistep forms are vastly improved in 5.0, they are still quite a major pain. The multistep bulk editing form is cleaner than it was, but still needs plenty of cruft (some of it new cruft) in order to work in 5.0. Anyway, bulk editing of categories and containers works pretty well with the patch applied.
  5. Added the new core jQuery 'tableselect' functionality to the tables in the category_bulkedit multistep form.
  6. Installed the 5.0 versions of pathauto and views, and tested category_pathauto and category_views. They seem to work fine with this patch applied.

I think that's pretty much everything that's needed to upgrade the whole category package to 5.0. Testing and reviews welcome. I'd like to get a bit of testing by other people before I commit the upgrade. Apart from the things listed in this thread, the category module should function pretty much the same as it does in 4.7. This is by and large a maintenance upgrade, not a new-features upgrade. New features for 5.0 can be submitted as separate issues later on.

DayShallCome’s picture

Thanks for the hard work, Jaza (and others). I'll patch and test sometime today or tomorrow.

Permanently Undecided’s picture

I tried the update with a test site I'm working on, with almost 300 categories and a complex hierarchy. Here's my update experience (e.g. not on a clean install):

  1. patch installs cleanly
  2. I run update.php; I go to the modules page. All modules show up and are enabled. To be on the safe side, I submit the module page again. I clean the browser cache and the database cache to be even more on the safe side
  3. I go to the main admin page, and here I start noticing something's wrong: there are two categories items listed, one pointing to admin/content/category and one to admin/content/taxonomy
  4. I don't know what's going on, so I go on to admin/content/category and I notice some weird output above the main bar of the Garland theme (something like NULL NULL NULL NULL etc.). I click on the outline and the categories all seem to be there, but trying to actually visit a category page only gives me the title. I start being very worried, but then I suddenly figure it could be a problem with the wrappers, so...
  5. ...I go under the settings and re-install the wrapper: going back to the main admin page, there's now only the correct category listing (for admin/content/category)
  6. since the wrapper was uninstalled, I figure something has gone wrong with my category settings in the update, and indeed all category settings except 'Category transform settings' have been reset in the update, so I put them all manually back to what they were. Since the settings were reset, by default category and containers content types were not provided, so nothing on my site worked anymore (since I used both content types)
  7. I now try browsing my site again, and all seems to be working fine on categories pages. Then I go to a CCK node where I embedded a list view with category elements for a node: instead of seeing the listed elements as I did before, I now get 'array'. Going to the edit page of the View in question, I notice that according to the View everything is fine (namely, it recognizes correctly all the containers/categories), so I suppose it's something related to Views module and not Category module and that the category_views module works fine
  8. next I try to submit a node to see if category_pathauto is working, but I notice another problem instead. The node I'm trying to add is a CCK content type that I have just re-enabled to be a category after the update lost the settings, but instead of getting the category settings as I did before the update, it's now asking me 'Container information'. So I go back to the category settings page and the 'Content type settings' does list 'category' and under 'Category transform settings' the content type in question is listed only under 'Node types that can be transformed into categories'. I clean the browser cache, and the database cache as well, but the content type now thinks it can be a container when I only allowed it to be a category. It worked fine before the update, needless to say.
  9. next I try to add a different CCK content type to see if category_pathauto is working. I submit the node and the path isn't generated. I go to the pathauto settings page and I notice that my settings (e.g. pre-update) are listed under the 'Node path settings', but that there is now a 'Category path settings' field group that wasn't there before. Since the default path pattern listed is '[vocab]/[catpath]' I think that the taxonomy module is interfering with the category pathauto generation. Nothing I tried worked.

This is all the testing I did. The really upsetting thing was finding that my site wasn't working at all because the default category and container type were not enabled, so I'd suggest enabling them or somebody else will have a shock. Thanks for the hard work!

Permanently Undecided’s picture

After some more testing, I can say that (in relation to the points listed above):

7. Okay, I tested and retested with Views and the View Theme Wizard and checked my templates again and again, and according to Views I should get a set of category links, when instead I get the output 'array'. If you need more info, I created a List View and set it to display Terms for a certain container, with the container's name as label (e.g: Product: pens). This worked great before the update, now it doesn't anymore. According to Views everything is fine, so is this is a category_views module error?

8. I have successfully managed to get the CCK content type to realize it's a category and not a container by going to admin/content/types and editing the content type settings. During the update the setting 'Category behavior': category setting got unset, so this is why I got the weird behavior reported above. After resetting that option, the content type worked fine as a category again.

Permanently Undecided’s picture

More on 7):

With a quick <code><?php print_r($node); ?>, I found something interesting:

    [taxonomy] => Array
        (
            [category_cat_39] => Array
                (
                    [title] => Art
                    [href] => node/39
                    [attributes] => Array
                        (
                            [rel] => tag
                            [title] => 
                        )

                )

        )

    [category] => Array
        (
            [39] => stdClass Object
                (
                    [nid] => 39
                    [node_id] => 280
                    [cid] => 39
                    [cnid] => 4
                    [description] => 
                    [weight] => 0
                    [depth] => 0
                    [title] => Art
                )

Could this difference in structure (and the mere presence of both taxonomy and category) be the cause of the 'Array' output in the List View?

Permanently Undecided’s picture

More info:

7) is definitely a Views.module problem and not a category_views problem, because I just tried a List View with only taxonomy enabled on a fresh install, and the terms were not displayed. So I think we can assume that category_views is working fine

Also:

10) while trying to fix 7, I decided to run a bulk update on my test site with many categories, and everything worked without a single problem.

Jaza’s picture

Status: Needs review » Reviewed & tested by the community
FileSize
194.36 KB

Thanks for your testing, PermanentlyUndecided. It seems that the majority of the problems you experienced were due to the 4.7 -> 5.0 upgrade not working smoothly. Previously, I didn't test upgrading a 4.7 site with category to 5.0. I just tested on a fresh install of 5.0, and I assumed that the update script (already written by the people who started this thread) was all done.

I have now tested the upgrade path, and I saw that the update script wasn't at all ready. Variables were getting incorrectly renamed. Node type names were getting incorrectly renamed. Queries were written with incorrect syntax. It was barely working. No wonder your testing turned up so many problems.

Attached patch fixes the upgrade path for category and for category_transform. Should all be smooth sailing from 4.7 to 5.0 now. More testing is welcome, but I am setting to RTBC, and I am planning to commit this patch within the next few days, unless more major bugs are found with it.

Jaza’s picture

New patch that fixes the hook_activeselect implementation to work with the 5.0-patched version of activeselect (see http://drupal.org/node/104478 for the activeselect 5.0 upgrade patch). This hook implementation still needs some work. Seems to work pretty well for the 'add/edit category' pages, but not so well for complex distant-parent categorised node forms.

Permanently Undecided’s picture

I installed a backup of the 4.7.4 version of my test site, and tried the update again using the patch provided in #26, and this time I only had to reinstall the taxonomy wrapper, everything else worked smoothly. Great job, Jaza, thanks! I've never used activeselect module, so I wouldn't know if it works properly or not with the patch in #27, sorry I can't help.

One curiosity, related to the 4.7->5.0 upgrade. In 4.7, this is what a category node looks like, with category_display set up accordingly:

body field (of the category node)

list of subcategories 

prev, next and up links

node listing

In 5.0 this is instead what the same category node looks like, in any theme (i.e. not a theme issue):

list of subcategories 

prev, next and up links

body field (of the category node)

node listing

Is this normal, by design, or something due to the upgrade? I really preferred the body of the category showing up before the list of subcategories and the cat-display links, rather than afterwards, since I generally use the body to explain what the category is about, which is nice to know before the subcategories are listed. If it's by design, can it be overriden in a theme to get the old display back?

Jaza’s picture

Status: Reviewed & tested by the community » Fixed
FileSize
194.84 KB

Awesome! Thanks for all the hard work on developing, testing, and reviewing this patch, guys. Attached patch committed to HEAD. I am about to create a new 5.0 branch for the category module.

Permanently Undecided’s picture

You rock, Jaza! Just tried the release version and everything works smoothly, problem in #28 solved as well. Thank you so much!

Anonymous’s picture

Status: Fixed » Closed (fixed)