Hi all,
The Ubercart catalog uses an extra path and alias for it's catalog pages. Extremely useful modules like XML sitemap have difficulties finding those paths and aliases and use the obsolete (in my case anyway) standard term pages. hook_term_path() can change that easily. Could this be implemented in a next release or should this also be in a contrib?
function uc_catalog_term_path($term) {
// $term looks like it doesn't have a $term->name
$term = taxonomy_get_term($term->tid);
return uc_catalog_path($term);
}
with big thanks to Kiam.
http://drupal.org/node/425068
Thanks
Mark
| Comment | File | Size | Author |
|---|---|---|---|
| #28 | 425984-uc_catalog_term_path.patch | 447 bytes | longwave |
| #18 | ubercart-425984-17.patch | 446 bytes | fuerst |
| #17 | ubercart-425984-17.patch | 446 bytes | fuerst |
| #14 | ubercart-425984-14.patch | 419 bytes | fuerst |
| #13 | ubercart-425984-13.patch | 416 bytes | fuerst |
Comments
Comment #1
splash112 commentedSorry,
It works, expcept if a term is updated. Then it will insert a category/catalog/etc in the Ubercart catalog menu...
grrr
Comment #2
rszrama commentedSo... is your patch necessary or not?
Comment #3
splash112 commentedHi Ryan,
Necessary maybe, but for now incomplete.
It messes up the Ubercart catalog menu on updates (and probably inserts).
Need to dig a little deeper in the code. Probably has something to do with the double paths used in the catlog module instead of changing one.
Mark
Comment #4
rszrama commentedAlrighty. I don't like the title of the "needs more info" status, because I'm not just sidelining your request, but I want it to indicate that research is still in progress. : )
Comment #5
heyyo commentedAnything new on this issue ?
I really would like to include the catalog terms in my sitemap.xml and not the taxonomy terms
Comment #6
splash112 commentedSadly, not to my knowledge.
The term paths are easily disabled though, through the vocabulary settings.
Another way could be to use views instead of the uc_catalog module.
Comment #7
sean-gnu commentedHi
Here is a modified uc_catalog to address this issue.
I've removed all path_alias hooks and function calls and hooked into taxonomy_term_path rather.
Path_auto no longer duplicates links (i.e. no more taxonomy/term/% and catalog/% - only catalog/%)
XML sitemap now appears to work as expected.
Some modules with hard-coded taxonomy/term/% paths instead of taxonomy_term_path calls will still not work.
Check your vocab table that the module field is set to uc_catolog.
As per usual it requires further testing !!!
I hope someone finds it useful ...
S.
Comment #8
splash112 commentedNice!
Will try and test it soon!
Comment #9
tr commented@sean-gnu: Could you please post a patch? Zipping up the entire module is not useful at all, as it is an unknown version of uc_catalog containing unknown and undocumented changes and possibly leaving out fixes that have been added since you downloaded your copy of uc_catalog. More to the point, without a patch there's no way to evaluate your contribution and get it included into the next version of Ubercart.
Comment #10
sean-gnu commented@TR - sorry - never used patch files before. Good time to start !
Here is the patch file ...
Comment #11
tr commentedGood start. I haven't evaluated the patch yet, but one thing in there caught my eye - your correction of a misspelling 'catlaog' in an update function. For future reference, update functions shouldn't be changed. The correct way to fix this mistake is to add a new update function. The reason for that is to make sure that all people who have run update_5(), for instance, end up with the exact same changes to their database. I took the liberty of committing a fix for this in the -dev version of Ubercart.
Comment #12
sean-gnu commentedHi - thanks for the pointers on the update functions and for committing the fix for that in the correct place.
Regarding update scripts :
I think for this patch an update script may need to be created to force the field 'module' in the vocabulary table to be 'uc_catalog'. I have modded the install and uninstall to accommodate this, but for someone upgrading it can potentially slip past. There will probably also need a check in the admin page when selecting the vocab to use for the catalog.
I will look into this if the patch looks good.
For now anyone trying this patch may need to manually check the value of that field using phpmyadmin etc.
Comment #13
fuerst commentedThe patch in #10 does much more than this issue was created for. Shouldn't be another issue created for this?
Anyway, here is a patch which just adds the missing hook_term_path() to uc_catalog. This makes modules like http://drupal.org/project/edit_term to be useful for the Ubercart Catalog terms. The missing
$term->nameas mentioned in the original issue posting is not needed as far as I can see therefor I did not include a call totaxonomy_get_term().Hopefully that patch get included into the next release since it makes living of Ubercart product managers easier.
Comment #14
fuerst commentedPatch still necessary in 2.6
Comment #16
fuerst commented#15: Looks like an error in the test bot: First attempty to apply the patch file using -p1 failed, second attempt using -p0 succeeded with status = 0. Nevertheless PIFR thinks it Encountered error on [apply]. Filed it in the issue queue, see #1246354: Test failed with "Unable to apply patch"
Comment #17
fuerst commentedAha - PIFR does not longer accept -p0. Recreated patch with -p1.
Comment #18
fuerst commentedAha - PIFR does not longer accept -p0. Recreated patch with -p1.
Comment #20
fuerst commentedSimpleTest failed which seems not to be connected to the patch.
Comment #21
longwaveTestbot cannot currently handle dependencies, but Token is a required dependency in Ubercart 6.x-2.x, so all tests on that branch will fail :(
Comment #22
tr commentedShould be done in D7 too. hook_term_path() has been removed in D7 and replaced by hook_url_outbound_alter(). See http://drupal.org/update/modules/6/7#hook_url_outbound_alter for usage example.
Comment #23
tr commentedNow that the testbot is finally working, let's try it again!
#18: ubercart-425984-17.patch queued for re-testing.
Comment #25
dimitriseng commentedI would be great to get this in D7, any plans for this to happen? Thanks.
Comment #26
dimitriseng commentedHi, just checking if there is any progress in getting this in D7... Thank you!!
Comment #27
longwaveMarked #1513328: Catalog paths create duplicate product listing pages as duplicate
Comment #28
longwaveNot sure this is worth changing now in 6.x, and should be solved in 7.x by #1909542: Replace hook_link_alter with hook_preprocess_link, but I also don't see why the patch in #18 doesn't apply, so let's see what testbot thinks of this.
Comment #29
longwaveLooking at it again, I think this is now won't fix for 6.x, as we risk breaking too many existing sites versus any minor benefit that this gives. The patch above will still work if anyone needs this.
Comment #30
agittins commentedNot sure if I am understanding this at it's full depth but for the record I think I've worked around this issue. The main bits for me was that xmlsitemap was pointing at the catalog via taxonomy paths instead of at the uc_catalog view, and also that it listed categories in the sitemap which had no products. To fix this:
It seems counter-intuitive, but the idea is that taxonomy links get pointed into the uc_catalog but we don't list them in the sitemap because it will include empty ones. Then we use taxonomy_menu to publish those links into xmlsitemap but without the empties.
It is definitely a bit of a kludge, but that's the thing with uc building a non-drupal menu for navigation I guess. As it happens I have also hacked taxonomy and uc_catalog to return actual 404 responses for empty terms to stop google complaining about (and indexing) the ones that do show up.