Posted by Frieder on May 23, 2008 at 8:35am
| Project: | XML sitemap |
| Version: | 7.x-2.x-dev |
| Component: | xmlsitemap |
| Category: | feature request |
| Priority: | normal |
| Assigned: | Darren Oh |
| Status: | closed (fixed) |
Issue Summary
XML Sitemap should use aliased Drupal URLs like http://example.com/blog/2008-04-02/my-entry instead of http://example.com/node/76
Patch is not mine! It is from ehsan.akhgari, see here:
http://drupal.org/node/157533#comment-851922
I didnt try to apply the patch, but adding the two lines to xmlsitemap.module works fine for me.
Comments
#1
Here is a version of the patch which should apply cleanly on CVS. The code is basically the same, but I have adjusted the paths (thanks to siliconmeadow).
#2
Patch seems to work for me.
#3
The reason we use xmlsitemap_url() instead of url() is that performing separate database queries for thousands of links causes poor performance. xmlsitemap_url() needs to be given the correct alias rather than looking it up.
#4
On a 6.2 production site, we decided to autogenerate a sitemap xml file, and we got everything in one query:
SELECT u.dst, n.changed FROM {node} AS n INNER JOIN (select distinct(src) as src, dst from {url_alias}) AS u ON (concat('node/', n.nid) = u.src)Once we execute that and have a record in a $record variable, getting the full URL is as simple as:
$url = 'http://www.example.com/' . $record['dst'];Perhaps that might help you?
Dave
#5
That's what the current code does. However, if the dst was not retrieved, it doesn't work.
#6
The problem is in xmlsitemap_node.install. pid must be allowed to be NULL.
#7
Ah, I think I understand. The reason that works for us is because we have pathauto turned on and so everything has an alias, and we don't have to worry about some nodes not having them. However, that should be fixable with an IFNULL() statement in the query, no?
#8
No. xmlsitemap_node saves the pid and uses it to retrieve the alias. The xmlsitemap_node_enable() adds the pid to any link that has a null pid. However, xmlsitemap_node_schema() sets all pids to 0 initially.
#9
#10
I'm not sure I understand what needs to be done here any more. Could someone please elaborate? I'm kind of new to Drupal development.
#11
Just so we're all thinking the same thing, I think Darren is referring to the following code from xmlsitemap_node.install:
'pid' => array('description' => t('The id of the path.'),
'type' => 'int',
'unsigned' => TRUE,
'not null' => FALSE,
'default' => 0,
),
I note that 'not null' is set to false. This presumably means that nulls are allowed (which is what Darren is saying needs to happen). So I concur with ehsan -- I don't understand what needs to be done either.
#12
If NULLs are allowed, the only problem is the default value.
#13
Ok, great. I've attached a patch (in which we can see the beauty of the 6.x schema API), but I have a question: should previous_comment or priority_override and the like also default to NULL?
#14
#15
#16
OK, the patch from wayland worked fine for me, but to enable aliases for users and taxonomy I had to do the same to:
xmlsitemap/xmlsitemap_user/xmlsitemap_user.install
xmlsitemap/xmlsitemap_term/xmlsitemap_term.install
I changed the 'pid' field in mysql table like for xmlsitemap_node:
'pid' => array(
'description' => t('The id of the path.'),
'type' => 'int',
'unsigned' => TRUE,
'not null' => FALSE,
'default' => NULL,
),
Now my xmlsitemap is working with aliases, YEAH!
Please can someone make a proper patch, i don't know how to use CVS and do patches for Drupal, maybe someone can explain me or better, give me a link to the documentation where I can learn how to make patches for Drupal modules. (I am working under Linux, so this should be easy).
Next step should be to release a xmlsitemap without the bug from here: http://drupal.org/node/261511 and working aliases :)
#17
@Frieder:
The basic CVS intro: http://drupal.org/handbook/cvs/introduction
The intro for module maintainers: http://drupal.org/handbook/cvs/quickstart
You won't have a CVS login, so just check stuff out as anonymous; that won't let you commit your changes, but it will allow you to make patches.
If you're using a command line CVS, you probably want:
cvs -z6 -d:pserver:anonymous:anonymous@cvs.drupal.org:/cvs/drupal-contrib checkout contributions/modules/xmlsitemap
Then, you make your changes. Then you go to the contributions/modules directory, and run "cvs diff xmlsitemap > xmlsitemap_patch.txt" or something like that.
Oh, and until the attached patch actually functions 100%, it should be marked "needs work" or "needs review".
HTH,
#18
I'll finish this.
#19
Patch needs testing. Uninstall XML Sitemap before testing.
#20
I have tested this patch and can verify that installation of the modules occurred without a hitch, and full URL aliases are included in the sitemap.
Not sure if I should be the one changing the status (to reviewed and tested by the community) or not. But for me it all works as intended.
#21
Anyone can change the status. Thank you.
#22
Fixed in CVS commit 120222.
#23
Automatically closed -- issue fixed for two weeks with no activity.
#24
I'm using 9th Jul dev version but the problem still appears:
#25
You must uninstall the module before installing the update.