Closed (fixed)
Project:
Category
Version:
6.x-2.0-beta3
Component:
Code
Priority:
Normal
Category:
Task
Assigned:
Reporter:
Created:
7 Feb 2009 at 13:20 UTC
Updated:
19 Aug 2009 at 05:20 UTC
Jump to comment: Most recent file
If you want to do this, then please assign this issue to yourself, and post updates and patches in this thread (not in the main d6 upgrade thread). Volunteer(s) wanted! Thanks.
| Comment | File | Size | Author |
|---|---|---|---|
| #11 | cat-views-d6-4.patch | 18.45 KB | JirkaRybka |
| #11 | category_views.tar_.gz | 3.7 KB | JirkaRybka |
| #8 | cat-views-d6-3.patch | 18.33 KB | JirkaRybka |
| #8 | category_views.tar_.gz | 3.68 KB | JirkaRybka |
| #4 | cat-views-d6-2.patch | 17.56 KB | JirkaRybka |
Comments
Comment #1
JirkaRybka commentedNote to myself: I need to keep an eye on this, as category_views is running on my site (to be upgraded soon). Not assigning (yet) due to lack of time to work on this.
Comment #2
JirkaRybka commentedI started my attempts on this. Let's see how far can I get...
Comment #3
JirkaRybka commentedOK, I reached a version, that works quite well as far as I tested it. But I didn't test everything yet (install/uninstall in particular), so leaving at "Needs work" for now.
This was a bit difficult, as both Category and Views are heavily changed on 6.x, and both lack documentation on some important points.
hook_nodeapi() is completely rewritten, since the 5.x version was dependent on several functions now gone, hook_category() is also gone, so I changed that into hook_form_alter(), install/uninstall now use Schema API, the hook_schema() implementation was somehow present in CVS (perhaps some past bdragon's effort?), but needed update.
Attaching the patch. Please note the first chunk, which goes to category.module, where the category_views specific code got changed since 5.x, making little sense now (list of nodes in the category got passed, instead of needed category node itself - this functionality replaces the display of category as whole, not single nodes in it), so the chunk removes that change. The last chunk clears the category_views.schema file found in CVS - this file should be removed entirely, it's a leftover from early stage of Schema API, not supported in any released Drupal version.
Attaching also a tarball, because I'm a bit unsure how it really is in CVS (accessing only manually through web-based interface), and also for the sake of readability. But the tarball doesn't contain the first chunk from the patch!
TODO: Consider, whether this module needs to be extended to also expose Category tables to Views. I've seen a lot of requests in the queue over time, I manage my own hack since 4.7, but I haven't it complete for all fields etc. Not sure now, so postponing this part.
Comment #4
JirkaRybka commentedUpdated patch (again note, that the tarball is just for reference, lacks one hunk from the patch!).
Changes since previous patch:
- Updated one link in a form field description, pointing to new View submission form (don't hurt your eyes, reviewers - there's no change to the actual string, just path in replacement argument).
- Fixed problem with $behavior checking: This value comes via variable_get(), with default value being zero (useful for empty() tests), and actual flags are 'category' or 'container' (this is inherited from the rest of Category package). However, if the default value (i.e. a non-category regular node) come to a test like
if ($behavior == 'container') {, it's surprisingly considered equal. Default for $behavior is integer 0, and so the string gets type-casted to integer too, evaluating to zero (not a valid number), and so becoming equal to the default. To be safe, I changed the operators to strict '===' (going to check how is it in other parts of Category package, might need a cleanup too).- Everything tested now. Works fine, I'm pretty confident now.
BTW I ran into a problem with Views embed+argument+exposed filter situation, just like on 5.x (my fix posted at #156130: Exposed filters: URL processing on embeds), but I managed to implement a safe workaround here, so my patch is not affected.
Comment #5
JirkaRybka commentedForgot to mention - the TODO above is not really valid, as exposing the tables to Views is entirely different task (and old issue, possibly even existing somewhere in the queue), with no impact on the 5.x->6.x port of existing code. Just ignore the mention above.
Comment #6
JirkaRybka commentedJust a bit of extra information on the marginal ideas mentioned:
- The $behavior everywhere else in the category package is tested for being empty first, so that's OK. My approach with === operator works fine too, both are valid IMO.
- Exposing the tables to Views is not that important now, and may be omitted altogether IMO. Now, with both Views and the Taxonomy wrapper being improved (i.e. Taxonomy tables maintained more reliably, and Views offering more stuff to do with them), all works flawless for me via Taxonomy. (I'm dropping my old partial implementation now.)
Comment #7
JirkaRybka commentedFurther testing of my site shows, that there's more to be changed along with Views 1 to Views 2 transition. Changes are done, patch to be rolled soon. Don't commit yet ;-)
Comment #8
JirkaRybka commentedOK, new patch attached (also hopefully rolled against the correct HEAD - I'm only accessing CVS via web based interface...)
To explain what happened here, a bit of background: To show list of available views on the form, and store the selection, 5.x code of category_views simply just grabbed records directly from {view_view} database table, and stored the numeric 'vid' identifier. It worked (I'm running the module myself without problems), and although Views 6.x-2.x claim to accept view *name* in views_embed_view() [plus it's subparts], the old method still works (apparently there's some sort of a fallback in Views). So in my previous patch, I only just updated the table name to {views_view}, and we're finished. Or I thought so!
Now that I converted all my views to Views2, I followed a suggestion in their documentation: To easily transfer these complicated views between testing and production site, and reduce database-stored data, export them as php-code definitions, and put into site-specific module's hook_views_default_views() implementation. They even offer a special module Views Exporter for that, and it writes all the code for you. Difficult to resist :)
Unfortunately, once I did this, my views vanished from category_views, as they were no more stored in database. Basically this means, that the {views_view} table is only just one of possible storages, not the only. And with all that Views' suggestions and tools out there, it's quite possible that the needed view is often stored elsewhere. I don't know how it was on 5.x, but now this is a problem.
So, I reworked the category_views_get_views() function, dropping the query, and using Views API instead, so that we get really all usable views, regardless the storage. This is a little tricky, as I can't see any exactly fitting function in Views module - best candidates are views_get_all_views() and views_get_applicable_views(). The former returns all stuff including disabled views etc., so I used the latter. It searches for a given setting being present (I use 'handler' which is always present), and additionally filters out all the disabled and similar stuff. Then I filter away displays other than 'default' (the one used for embedding), otherwise there would be duplicates. As a little bonus, also tags are added to the form options now.
Next problem is, that non-database views don't have any numeric ID ('vid' is just empty), so we need to store names of views (the right identificator in Views2). So all the other changes from previous patch are just to change the numeric 'view_id' column in {category_views} table into string 'view_name', and make it to pass through all the code that way. On upgrade, I just dropped the old column and added a new one, because it doesn't make any sense to keep old (numeric) data in there. On Views1(5.x)->Views2(6.x) migration, all views must be re-created or converted anyway, so the IDs are (mostly) not kept anyway. *some* view may show up by old numeric ID, but the chance of a correct hit is low, and it may vanish later like in my case, so keeping old data would be more of a harm IMO. Resetting to no view is better than showing mostly wrong ones.
Patch attached, plus tarball for reference and readability (beware - the tarball doesn't have a needed one-line fix to category.module).
Comment #9
JirkaRybka commented#157560: category_views with default views indicates, that the name vs. ID of view (and default views) problem existed on 5.x, too. There's even a nearly identical fix on that issue. It would be a perfect duplicate, except that it's a 5.x feature request, as oposed to 6.x task.
Comment #10
JirkaRybka commentedGoing to check status of this code after I rolled the patch at #484624: Fix all broken category/container previews, and add category_display defaults. The only suspection is persistence of settings in form across previews of container node.
Comment #11
JirkaRybka commentedUpdated patch, container previews now working (it's only possible to test that part with another patch applied: #484624: Fix all broken category/container previews, and add category_display defaults). Otherwise see above.
I can't see anything more to do here.
Comment #12
Donsov commentedtell me how to make category_menu that he filtered with views
Comment #13
Jaza commentedCommitted to HEAD. Thanks! See:
#158598: Category port for Drupal 6.x