Currently xmlsitemap uses only the node language to generate URLs and ignores nodes that have been translated with entity translation.

Files: 
CommentFileSizeAuthor
#93 interdiff-1481798-90-93.txt1.47 KBkalistos
#93 add_support_for_entity_translation-1481798-93.patch13.32 KBkalistos
#90 interdiff-1481798-76-90.txt3.53 KBkalistos
#90 add_support_for_entity_translation-1481798-90.patch13.14 KBkalistos
#76 interdiff-1481798-71-76.txt1.71 KBsergei_brill
#76 add_support_for_entity_translation-1481798-76.patch12.96 KBsergei_brill
#71 interdiff-1481798-64-71.txt1.33 KBvasike
#71 add_support_for_entity_translation-1481798-71.patch11.88 KBvasike
#68 updated-patch.patch11.76 KBLangley
#64 add_support_for_entity_translation-1481798-64.patch11.49 KBDeFr
PASSED: [[SimpleTest]]: [MySQL] 525 pass(es). View
#52 interdiff-1481798-47-52-do-not-test.txt2.34 KBKristen Pol
#52 xmlsitemap-add_support_for_entity_translation-1481798-52.patch13.62 KBKristen Pol
PASSED: [[SimpleTest]]: [MySQL] 525 pass(es). View
#47 interdff-1481798-46-47.txt515 bytessergei_brill
#47 add_support_for_entity_translation-1481798-47.patch11.63 KBsergei_brill
PASSED: [[SimpleTest]]: [MySQL] 525 pass(es). View
#35 interdiff-1481798-34-35-do-not-test.diff1.48 KBdas-peter
#46 add_support_for_entity_translation-1481798-46.patch11.62 KBDeFr
PASSED: [[SimpleTest]]: [MySQL] 525 pass(es). View
#46 interdiff-1481798-45-46.txt5.27 KBDeFr
#46 interdiff-1481798-35-45.txt1017 bytesDeFr

Comments

das-peter’s picture

I just came across this issue. Atm. for me it looks like to support this we've to alter the database schema. Currently the xmlsitemap table has id and type as key and a row can have a single language.
But since with entity translation the id / type combination will be shared for multiple languages we loose the unique identifier.
And I'm not sure if the "simple" introduction of language is really sustainable.

Btw. for me this issue is related to following other issues:
#1461670: Generalize to work with any Entity Type
#891696: Consistently use link, bundle, and entity loaders everywhere

All of them are mainly about to better integrate with the "core" entity system.

marcoka’s picture

romualdbrisson’s picture

Here is a patch for a simple solution on this issue.
You have to rebuild links.

hefterbrumi’s picture

This doesnt work for me

muschpusch’s picture

i started working on entity translation support over here: #1370394: Set correct language for url()

hefterbrumi’s picture

great news indeed

e.escribano’s picture

I needed the implementation for entity_translation myself so I made a patch that includes also support for taxonomy terms.

EDIT:

Didn't notice that the alter in the install file gave me problems when a node was saved. It is not a complete patch yet.

e.escribano’s picture

matsbla’s picture

titouille’s picture

#7 seems to be broken with last xmlsitemap version. follow code must be edited in xmlsitemap_link_save function (in xmlsitemap.module) :

  // Save the link and allow other modules to respond to the link being saved.
  if ($existing) {
-    drupal_write_record('xmlsitemap', $link, array('type', 'id'));
+    drupal_write_record('xmlsitemap', $link, array('type', 'id', 'language'));
    module_invoke_all('xmlsitemap_link_update', $link, $context);
  }
  else {
    drupal_write_record('xmlsitemap', $link);
    module_invoke_all('xmlsitemap_link_insert', $link, $context);
  }

Because patch add language as primary key, drupal_write_record must pass 'language' as primary key to work correctly.

das-peter’s picture

Status: Active » Needs review
FileSize
7.34 KB
7.98 KB
PASSED: [[SimpleTest]]: [MySQL] 525 pass(es). View

Re-rolled #7, incorporated #10 and centralized the translation handling.
New there's xmlsitemap_get_entity_languages() which fetches the available languages.
This new function just returns the languages of published translations. It also uses the translation handlers to get the information instead accessing the data directly - this should be safer.
And last but not least it just handles entities with entity_translation enabled.

J-Lee’s picture

#11 is working for me with entity translated nodes/fields.
Thank you.

J-Lee’s picture

It seems, I was to fast at #12 ....
If found that nodes without translations are added to the sitemap too.

I have two languages (en and de) with a node which is only available in en.
At the sitemap there is an entry for /en/[node-alias] which is ok.
But there is an entry for /de/node/[nid] too.

I'm using entity translation for title-field and body.

ph08n1x’s picture

#11 worked for me too, thank you so much! :D

in regards to showing languages that don't have translations, luckily all my pages are translated in all three of my languages... hmmm mabe there is a way to check the state of node if it is published for that language...?

ph08n1x’s picture

Hi J-Lee perhaps this will help you (sorry about not supplying a patch but super busy today!):

@@ -1629,7 +1629,7 @@ function xmlsitemap_get_entity_languages($entity_type, $entity, $default_languag
         $languages = array();
         $translations = $handler->getTranslations();
         foreach ($translations->data as $translation_lang => $translation_data) {
-          if (!empty($translation_data['status'])) {
+          if (!empty($translation_data['status']) && $translation_data['status'] == 1) {
             $languages[$translation_lang] = $translation_lang;
           }
         }

I saw that the if statement only checks if status is empty it does not check if status is equal to 1 (i.e. is the entity published).

Although I'm not sure if that is checking if the entity translation is published or if it's just checking if the default language node is published... Give it a go tho :D

das-peter’s picture

@ph08n1x empty() should be fine:

No warning is generated if the variable does not exist. That means empty() is essentially the concise equivalent to !isset($var) || $var == false.
....
The following things are considered to be empty:

"" (an empty string)
0 (0 as an integer)
0.0 (0 as a float)
"0" (0 as a string)
NULL
FALSE
array() (an empty array)
$var; (a variable declared, but without a value)

This means 1, TRUE, 'published' etc. are considered as published. And as far as I know status is here an int or bool.
Edit: Fixed formatting

J-Lee’s picture

I found the issue from #12 ...
Because I used the node_translation_sitemap (from #9) before, I have multible 'node_translated_de' entries at the 'type' column in the xmlsitemap table.

I had to trunkate the xmlsitemap table (without the first row, which is the front page) and rebuilding the links.

Now it is working.

@ph08n1x: Thank you for your suggestion.
Unpublished translations are not displayed, so #15 is not needed, I think. But it will be fine if someone can confirming it.

Edit: das-peter has confirmed it a few minutes before :)

MXT’s picture

Sorry guys,

a banal question: applying patch in #11 is enough or I have to install XML_sitemap_internationalization submodule also?

Thank you very much

J-Lee’s picture

XML sitemap internationalization adds one sitemap for each language. E.g. www.example.com/en/sitemap.xml and www.example.com/de/sitemap.xml. If you only want one sitemap with multiple languages, patch #11 is enough.

DeFr’s picture

Status: Needs review » Needs work

Moving to NW, because there's a few things that seems wrong with regards to the schema handling.

This is adding a xmlsitemap_node_update_6202 update function, to tweak the xmlsitemap table. That update function number seems off for a Drupal 7 module (the update will still run because the last update in that file is 6201, so every Drupal 7 site out there have a schema_version of 6201 for xmlsitemap_node), but more importantly, xmlsitemap_node isn't the mode that provides the xmlsitemap table in the first place.

I think instead, the module should add an update function to xmlsitemap.install, and also tweak the default primary key defined in xmlsitemap_install to handle new installations.

das-peter’s picture

Status: Needs work » Needs review
FileSize
1.23 KB
8.18 KB
PASSED: [[SimpleTest]]: [MySQL] 525 pass(es). View

Address points from #20.

J-Lee’s picture

Patch #21 is working on a clean Drupal 7.37 installation with latest title- and entity_translation module.
Thank you das-peter.

DeFr’s picture

FileSize
9.4 KB
PASSED: [[SimpleTest]]: [MySQL] 525 pass(es). View
3.19 KB

Thanks das-peter, the schema part does look a lot better now. Sadly found a bunch of others issues while testing various things that I had not in the first review :-(

First problem is due to the change in the $existing in xmlsitemap_link_save being conditional on entity translation activation, while the primary key and the drupal_write_record are not. That means that if you don't have entity translation enabled and the entity language changes, you won't get any update for this sitemap link anymore.

(To reproduce that issue: install Drupal core, its bundled Locale and Translation modules, install XML Sitemap and XML Sitemap Node, add another language and change the article bundle multilingual settings from Disabled to Enabled while adding that content type to your site map. Then add an article node in English, save it, and change the node language to something else… No update to the xmlsitemap table anymore)

Attached patch tries to fix the problem by always using the same key while checking for existence and updating, which fix that problem, but I'm not sure that's completely correct either.

I think that's the only problem for people not using entity translation ; moving to problems specific to entity translation now:

  1. There's an implicit assumption that having the Entity Translation enabled means that it'll be used to managed the entities. That's not mandatory though, and should be checked with entity_translation_enabled otherwise you get stale links in the table ; added that to the patch.
  2. There's a problem with translation deletion ; right now there's nothing to delete the corresponding link from the sitemap.
  3. Finding that lead me to xmlsitemap_link_delete, which should also probably be tweaked to accept an optional language parameter now that this is part of the primary key ? As it is now, it's misleading, because its name implies that it'll delete a single link, while in fact it can deletes a bunch of them, and the code is adding a link_delete_mutliple call which will target a single link
  4. _xmlsitemap_check_changed_link is also doing a custom query to load a link that's not taking the language into account
  5. At that point I think we need a module maintainer to chime in, to have an idea of how invasive the acceptable changes can be. There's a lot of code in there that expects the primary key to be (type, id), and changing them all is going to be time consuming.

maxplus’s picture

Hi,

First of all: thanks for the great work done already!

I have just tested the patch from #23 and I'm getting this error:

Fatal error: Cannot redeclare xmlsitemap_node_entity_translation_delete() (previously declared in /.../sites/all/modules/xmlsitemap/xmlsitemap_node/xmlsitemap_node.module:90) in /.../sites/all/modules/xmlsitemap/xmlsitemap_taxonomy/xmlsitemap_taxonomy.module on line 158

Then I tested the patch from #21, and that runs fine.
=> Now I have three correct xml-sitemaps, one for every language (Dutch, French and English)

Using Drupal 7.36, XML sitemap 7.x-2.2+3-dev and Entity Translation 7.x-1.0-beta4

DeFr’s picture

FileSize
804 bytes
9.4 KB
PASSED: [[SimpleTest]]: [MySQL] 525 pass(es). View

The fatal error in #24 was due to a bad copy & paste between xmlsitemap_node and xmlsitemap_taxonomy, sorry for that. Attached fixed patch + interdiff.

gstout’s picture

Latest patch worked great. We should get this into a dev release.

Neograph734’s picture

The patch appears to be working nicely, and applied flawless. It would indeed be nice if this could get committed. Should we test more or is this RTBC?

colan’s picture

Priority: Normal » Major
Issue tags: +i18n, +Release blocker

I'll be testing this very shortly. Seems to apply to the latest dev.

colan’s picture

Issue tags: -Release blocker

Removing tag.

colan’s picture

Status: Needs review » Reviewed & tested by the community

Code looks good and works well. It also appears to solve the problem described in #1370394: Set correct language for url(). Here are the steps I took to get this up & running:

  1. Enable the XML Sitemap Internationalization submodule.
  2. Apply the patch from #25.
  3. Delete the default sitemap.
  4. Add a new sitemap for each language.

Can anyone comment on #2220641: Merge this function back to xmlsitemap module?? Now that we have this code, is that module obsolete?

colan’s picture

Status: Reviewed & tested by the community » Needs review
FileSize
11.03 KB
PASSED: [[SimpleTest]]: [MySQL] 525 pass(es). View

Whenever the XML Sitemap Internationalization submodule (xmlsitemap_i18n) is enabled, sitemaps will now be created automatically for each language. This patch is the same as the previous one, except that I've added xmlsitemap_i18n_install().

colan’s picture

Title: xmlsitemap doesn't work with nodes translated with entity translation (and title_field) » Add support for Entity Translation & Title modules

Better title.

Neograph734’s picture

Status: Needs review » Needs work

Colan, your patch seems to be working nice. There is one thing I am not completely sure about;

If I enable the patched module while i18n is configured and in place, there are two sitemaps created for the english (default language). One named English and one named Default. The sitemaps for the other languages are fine.
Though this seems harmless and one of both can easily be deleted, it might be causing problems when the system tries to write the same file twice.

When I changed the i18n default language, the sitemap default language followed as expected. Therefor I suppose we should attempt to remove the default language, rather then to prevent the English (or other language) from being created. If one was to change the i18n default language later the same problem would occur with duplicate languages and English would even be missing.

You think you could add that to the patch?

colan’s picture

Status: Needs work » Needs review
FileSize
11.48 KB
PASSED: [[SimpleTest]]: [MySQL] 525 pass(es). View

Ask and you shall receive! Added a DB call to delete the default sitemap so it doesn't cause any confusion. It didn't seem to conflict with the new default language sitemap, but I agree, it's better to remove it.

das-peter’s picture

FileSize
1.48 KB
10.72 KB
PASSED: [[SimpleTest]]: [MySQL] 525 pass(es). View

I think this looks pretty good now.
Just gave it another visual review and just found some minor coding style nit-picks.
Patch updated.
As I'm one of the people fiddling with the patch I probably shouldn't RTBC this :)

colan’s picture

Thanks das-peter. I noticed those, but forgot to fix them. I better not RTBC this either. :)

For anyone testing this, all you should have to do is:

  1. Apply the patch.
  2. Enable the XML Sitemap Internationalization module (xmlsitemap_i18n).
  3. Run cron one or more times (some links are generated each cron run).
Dave Reid’s picture

Status: Needs review » Needs work

Some overall review:

  • I think it's kind of strange that we're encouraging people to install xmlsitemap_i18n when they may not actually want to be using the i18n module (and only entity_translation)? Do we want to provide support for per-language sitemaps in the base module if locale.module is enabled?
  • We should add an optional langcode parameter to xmlsitemap_link_delete() instead of using xmlsitemap_link_delete_multiple(). Then we could just implement xmlsitemap_entity_translation_delete() instead of requiring each submodule to implement the hook.
  • Need a change record for 'language' supporting an array of langcodes instead of just a single langcode string. Functions like xmlsitemap_node_create_link() are public APIs and looks like this change would break existing implementations. Maybe it would be better to *add* $link['languages'] for multiple-language-per-link support instead.
Neograph734’s picture

Dave, pleaese be aware that though there is some overlap, entity translation is no replacement for i18n as stated on the module page. It is just another way of translating content. Language selectors and other aspects of i18n are still required to actually use entity translation. So I assume the i18n requirement is not much of an issue.

The other points are worth thinking about.

colan’s picture

To make it clear that xmlsitemap_i18n isn't only for the i18n module (I was confused about that myself when I first got into this issue), how about if we change the module description from Enables multilingual XML sitemaps to Enables XML sitemaps for multiple languages via the Locale module, used by Internationalization and Entity Translation.

I'm fine with moving this stuff into the base module - not sure why it was split off in the first place.

Dave Reid’s picture

Simple, Entity Translation didn't exist yet. And without i18n there was no way to only have certain language content on certain domains, and not all content always on every domain.

MXT’s picture

XML_sitemap_internationalization submodule should not be mandatory.

I'm not using it in my use case (see #18 and #19).

nlambert’s picture

#35 works for me

Maybe out of scope for this issue, but one thing that seems to be missing is a way to add a language neutral sitemap that could point to language specific sitemaps. Or maybe the only way to do this is to actually put a sitemap.xml in the Drupal root.

E.g.:
/sitemap.xml # With links to language specific sitemaps:

/fr/sitemap.xml
/pt/sitemap.xml
/en/sitemap.xml
...

vimokkhadipa’s picture

#35 works for me too
I applied the patch # 35, and it worked perfectly.
xmlsitemap Versión: 7.x-2.2+3-dev

pipicom’s picture

#35 works for me also
xmlsitemap version: 7.x-2.2

sergei_brill’s picture

The patch works well for me except one thing. After I applied the patch, I tried to disable xmlsitemap_i18n and enable it again. I got below error message:
exception 'PDOException' with message 'SQLSTATE[23000]: Integrity constraint violation: 1062 Duplicate entry '3QuYVMoAVDdMUmwuEdFhwSFxLtcBNDrxIy91p4VzGdY' for key 'PRIMARY'' in [error]
/home/sergei/web/tours/site/includes/database/database.inc:2171
Stack trace:
#0 /home/sergei/web/tours/site/includes/database/database.inc(2171): PDOStatement->execute(Array)
#1 /home/sergei/web/tours/site/includes/database/database.inc(683): DatabaseStatementBase->execute(Array, Array)
#2 /home/sergei/web/tours/site/includes/database/mysql/query.inc(36): DatabaseConnection->query('INSERT INTO {xm...', Array, Array)
#3 /home/sergei/web/tours/site/sites/all/modules/contrib/xmlsitemap/xmlsitemap_i18n/xmlsitemap_i18n.install(35): InsertQuery_mysql->execute()
#4 [internal function]: xmlsitemap_i18n_install()
#5 /home/sergei/web/tours/site/includes/module.inc(905): call_user_func_array('xmlsitemap_i18n...', Array)
#6 /home/sergei/web/tours/site/includes/module.inc(477): module_invoke('xmlsitemap_i18n', 'install')
#7 /home/sergei/.composer/vendor/drush/drush/commands/core/drupal/environment_7.inc(143): module_enable(Array)
#8 /home/sergei/.composer/vendor/drush/drush/commands/pm/pm.drush.inc(1120): drush_module_enable(Array)
#9 [internal function]: drush_pm_enable('xmlsitemap_i18n')
#10 /home/sergei/.composer/vendor/drush/drush/includes/command.inc(359): call_user_func_array('drush_pm_enable', Array)
#11 /home/sergei/.composer/vendor/drush/drush/includes/command.inc(210): _drush_invoke_hooks(Array, Array)
#12 [internal function]: drush_command('xmlsitemap_i18n')
#13 /home/sergei/.composer/vendor/drush/drush/includes/command.inc(178): call_user_func_array('drush_command', Array)
#14 /home/sergei/.composer/vendor/drush/drush/lib/Drush/Boot/BaseBoot.php(62): drush_dispatch(Array)
#15 /home/sergei/.composer/vendor/drush/drush/drush.php(70): Drush\Boot\BaseBoot->bootstrap_and_dispatch()
#16 /home/sergei/.composer/vendor/drush/drush/drush.php(11): drush_main()
#17 {main}

I guess hook_uninstall should be included to xmlsitemap_i18n.install to remove contexts?

DeFr’s picture

Status: Needs work » Needs review
FileSize
1017 bytes
5.27 KB
11.62 KB
PASSED: [[SimpleTest]]: [MySQL] 525 pass(es). View

Worked on the review comments in #37 to try to keep moving this forward :-) Attaching interdiff and patch, plus missing interdiff for #45 that was posted in between.

In details, response to the various points of #37 below:

  1. I think changing xmlsitemap_i18n module description is the way to go ; not sure about moving that to XMLSiteMap itself. Moving it to the core with the corresponding install hook, then MXT use case in #18 (have one site map with all links in all language) become trickier (the default sitemap would get deleted, replaced by site map per language. You'd then have to delete those sitemaps, and re-create a new one without context)
  2. Changed that ; not completely sure about it though, because untill this patch, each module providing links for XMLSitemap was responsible of deleting them as needed, except for bundle renames (e.g. there's no hook_entity_delete in xmlsitemap.module)
  3. Makes sense to minimize the BC break. As an additional bonus, this allow caller to distinguish the source language and the set of available translations if they want to do so.
sergei_brill’s picture

FileSize
11.63 KB
PASSED: [[SimpleTest]]: [MySQL] 525 pass(es). View
515 bytes

Small fix for last patch

DeFr’s picture

@sergei_brill: Thanks for that and sorry for that typo :-)

Sakhmed’s picture

I have two languages enabled on my site English and Russian. Have several nodes that were originally created in English and then translated into Russian. Applied patch #47. Patch has been applied smoothly. Have two sitemaps for Russian and English. I can see all translated nodes in English sitemap but no translated nodes in Russian sitemap.

Using xmlsitemap-7.x-2.2+3-dev

Update

By some reasons translated nodes show up in actual xml file located in public directory. They are not showing in the sitemap that is available for viewing via drupal admin interface

sergei_brill’s picture

@Sakhmed Make sure you did database update (/update.php or drush updb). And try to rebuild sitemaps here /admin/config/search/xmlsitemap/rebuild

Kristen Pol’s picture

Works great! Thanks for the hard work. I found one minor typo:

+++ b/xmlsitemap.module
@@ -663,11 +673,17 @@ function xmlsitemap_link_update_multiple($updates = array(), $conditions = array
+ *  An optionnal language code for the link that should be deleted.

optionnal => optional

Kristen Pol’s picture

FileSize
13.62 KB
PASSED: [[SimpleTest]]: [MySQL] 525 pass(es). View
2.34 KB

I found an issue where the sitemaps were not being updated via cron when entity translation was done but the source node was untouched. The sitemaps would only get updated correctly when running the rebuild links option manually. To reproduce this issue:

  1. Apply patch from #47
  2. Enable 2 languages and configure xmlsitemap, xmlsitemap_node, and xmlsitemap_i18n for the 2 languages, e.g. English and Spanish
  3. Enable and configure entity_translation for a content type, e.g. article
  4. Create node for that content type in one language, e.g. English
  5. Run cron
  6. Check the sitemap for that language => it should have the node listed
  7. For that node, click "Translate" tab
  8. Click "add" for the other language, e.g. Spanish
  9. Translate the node and save
  10. Run cron
  11. Check the sitemap for that language => the node will be missing

I have updated the patch from #47 and added some code to xmlsitemap_i18n_query_xmlsitemap_generate_alter that will check if entity_translation is enabled and, if so, will use the entity_translation table for the query to find the nodes that need to be rebuilt. This seems like the logical place to add it but, if there are other suggestions, let me know.

I also fixed the minor typo noted in my previous comment (#51). Thanks!

phanosd’s picture

Hey all,

Recently had the issue where only "source" language nodes would appear on my sitemap. Patch #35 fixed this for me.

I have since however come across an undocumented issuewherever I try to create a new taxonomy term, or add translation to ANY taxonomy. Using xmlsitemap: 7.x-2.2+3-dev patch #35, 6 languages enabled

PDOException: SQLSTATE[42000]: Syntax error or access violation: 1064 You have an error in your SQL syntax; check the manual that corresponds to your MySQL server version for the right syntax to use near ' 'en-cy', 'en-uk', 'en-sa', 'en-tn', 'en-my', 'en-ca', 'ru-ru', 'es-ar', 'es-mx'' at line 1: SELECT loc, access, status, lastmod, priority, changefreq, changecount, language FROM {xmlsitemap} WHERE type = :type AND id = :id AND language = :language_0, :language_1, :language_2, :language_3, :language_4, :language_5, :language_6, :language_7, :language_8, :language_9, :language_10, :language_11, :language_12, :language_13, :language_14, :language_15 LIMIT 0, 1; Array ( [:type] => taxonomy_term [:id] => 7 [:language_0] => en [:language_1] => en-cy [:language_2] => en-uk [:language_3] => en-sa [:language_4] => en-tn [:language_5] => en-my [:language_6] => en-ca [:language_7] => ru-ru [:language_8] => es-ar [:language_9] => es-mx [:language_10] => es-es [:language_11] => ar-sa [:language_12] => en-ng [:language_13] => en-id [:language_14] => id-id [:language_15] => pl-pl ) in xmlsitemap_link_save() (line 611 of /var/www/wwwtest.hiwayfx.com/sites/all/modules/xmlsitemap/xmlsitemap.module).

*EDIT*

Went to sitemap xlm module and deselected "node" from inclusion in sitemap. Rebuild links, problem has disappeared.

PascalAnimateur’s picture

I just tested #52 and it adds the original URL of the (untranslated) page to the translated sitemap.
For example, I create the "Test page" in english (/en/pages/test-page) and translate it to french as "Page de test" (/fr/pages/page-de-test), and my french sitemap has /en/pages/test-page instead of /fr/pages/page-de-test

Any ideas?

PascalAnimateur’s picture

Update: It seems the check for is_array($link['language']) in function xmlsitemap_node_node_update always returns FALSE ... perhaps it should check for index 'languages', which is an array?

PascalAnimateur’s picture

FileSize
11.45 KB
PASSED: [[SimpleTest]]: [MySQL] 525 pass(es). View

Update (again): It seems the problem is with the generation of the sitemap itself, not the individual links being inserted / updated in the xmlsitemap table.

By adding a language condition in function xmlsitemap_generate_chunk, my sitemaps only contain links in the proper language! (see line 175 in xmlsitemap.generate.inc)

Here's patch #52 re-rolled with this additional line...

DeFr’s picture

FileSize
11.49 KB
FAILED: [[SimpleTest]]: [MySQL] Invalid PHP syntax in sites/default/modules/xmlsitemap/xmlsitemap_taxonomy/xmlsitemap_taxonomy.module. View
3.7 KB

The _node_update and _taxonomy_term update do seems off, seems like I didn't convert them properly when switching to the 'languages' key in #46 ; sorry about that. Uploading a patch that fix that.

I think that explains part of the problem described in #52. Not sure about the query_alter introduced in there though: I see that they're a port of the strict mode below, but adding some node specific case to handle Entity Translation that aims to be fully generic seems unexpected.

I also think that this query_alter is the cause of the problem @PascalAnimateur encountered.

So, what I'm attaching is #47 + the typo fix mentionned in #50 + the fix to the update calls.

@Kristen Pol, @PascalAnimateur: Can you please test and confirm if this works for you ?

Status: Needs review » Needs work

The last submitted patch, 57: add_support_for_entity_translation-1481798-57.patch, failed testing.

DeFr’s picture

FileSize
11.49 KB
PASSED: [[SimpleTest]]: [MySQL] 525 pass(es). View
626 bytes

Forgot to git add the latest change before creating patches…

That one should be better.

DeFr’s picture

Status: Needs work » Needs review
chrisdarke42@gmail.com’s picture

#35 works for me too, I tried #52 and #56 but they were generating extra entries in the sitemaps for the wrong languages. Admittedly, I have not tested the taxonomy issue but if you don't need that, #35 seems to be good

DeFr’s picture

@chris.darke: Could you please try #59 ?

PascalAnimateur’s picture

@DeFr: I tested your patch and the french sitemap entry is not created for the node translation when I create it from the original english node.

Adding the query condition from my patch probably won't fix that, as I don't see the 'fr' entry for that translation in the database.

DeFr’s picture

FileSize
11.49 KB
PASSED: [[SimpleTest]]: [MySQL] 525 pass(es). View
1.18 KB

Took the time to really test the patch I'm attaching this time, and everything seems OK. Running cron after adding a translation makes the link appear in the correct sitemap, deleting the translation makes it disappear, tested for node and taxonomy term. Sorry for the obvious typo in the patch above.

@PascalAnimateur If you can please confirm, it'd be more than welcome :-)

PascalAnimateur’s picture

Status: Needs review » Reviewed & tested by the community

@DeFr : Your patch works great for node content and taxonomy terms!
Marking this as RTBC, although more people should probably inspect / test this further.

bwaindwain’s picture

Yay! Patch #64 fixes xmlsitemaps with nodes and menus for my test site in 3 languages. Thx @DeFr

Neograph734’s picture

Seems to be working for me as well. Two languages and both site maps contain what they should contain.

Thanks!

Langley’s picture

FileSize
11.76 KB

Thanks for the patch it works great, except when trying to add taxonomy to sitemaps. The patch contains

xmlsitemap_taxonomy_taxonomy_term_update($term);

which should probably be

xmlsitemap_taxonomy_term_update($term);

?

Status: Reviewed & tested by the community » Needs work

The last submitted patch, 68: updated-patch.patch, failed testing.

Skin’s picture

Hello,
with #64, after applying the patch and after running update.php I have this error if I try to rebuild the sitemaps:

An AJAX HTTP error occurred. HTTP Result Code: 500 Debugging information follows. Path: /site/batch?id=399&op=do StatusText: Internal Server Error ResponseText:

If i try with #68 I'm unable to apply the patch:

git apply -v updated-patch.patch
updated-patch.patch:10: trailing whitespace.
'primary key' => array('id', 'type', 'language'),
updated-patch.patch:18: trailing whitespace.

updated-patch.patch:19: trailing whitespace.
/**
updated-patch.patch:20: trailing whitespace.
* Add new primary key including the language.
updated-patch.patch:21: trailing whitespace.
*/
fatal: corrupt patch at line 33

vasike’s picture

Status: Needs work » Needs review
FileSize
11.88 KB
1.33 KB

There is an update for the #64 patch.
It seems the previous patches do not do anything about about for languages for entities not yet translated having "Enable language fallback" option set.
So we have these pages available for other languages than source/original but they do not get in/out of sitemaps.

This update also includes the fix about using xmlsitemap_taxonomy_taxonomy_term_update($term) in xmlsitemap_taxonomy_xmlsitemap_process_taxonomy_term_links() (see #68).

Skin’s picture

I reinstalled 7.x-2.x-dev and applied #64 and after #71, but now I don't see taxonomy terms in the localized sitemaps, they appear only in the main language.

joachim’s picture

Patch works great, though existing nodes that aren't showing in the sitemap aren't fixed by this until they've been edited.

jurgenhaas’s picture

Status: Needs review » Reviewed & tested by the community

Tested it and it works fine for me. In terms of updating all nodes, I have written a simple drush command doing this:

  foreach (node_load_multiple(FALSE) as $node) {
    xmlsitemap_node_node_update($node);
  }
joachim’s picture

Cool! I hadn't managed to work out the right API to call in this module to update its data.

Though that's not going to scale for large sites. Ideally, it should be a hook_update_N() which uses the batch functionality to process the nodes in chunks.

Also, we could do to process only the nodes that need it maybe? I reckon this should give us them:

SELECT * FROM `entity_translation`
WHERE entity_type = 'node'
AND source != `language` AND source != ''
GROUP BY `entity_id`

That's all the nodes that have a translation in addition to their source language.

sergei_brill’s picture

The last patch works great. But I needed to add an additional option for my case. I have original site in English and localized version of the site for Australia. On the site I have main content type which references to other content type (multiple entityreference field). Nodes of the second content type have no own pages (at least users don't see them). I don't want to translate each item of the second content type, but I don't want to show all items of first content type on AU version and don't want add them to sitemap.xml. Also I don't want translate bean blocks on the AU version. So, I need Language fallback to be enabled, but I don't want add untranslated content to sitemap.xml. Therefore I added additional variable xmlsitemap_language_fallback which could be configured on xmlsitemap settings page.

sergei_brill’s picture

Status: Reviewed & tested by the community » Needs review
DuneBL’s picture

Patch 71 doesn't work for me... after applying it, I get errors when rebuilding the map

jsacksick’s picture

I applied the patch #76 and it seems to work pretty well.
@DuneBL you need to run the database updates.

aschiwi’s picture

I successfully used the patch in #76. I have two languages enabled and add a few node types and vocabularies to the sitemap. There are about 200 entries per sitemap, so it's quite a small use case.

Thanks to everyone who helped with this patch <3

DuneBL’s picture

@jsacksick Thanks for the tip!
This time I have used the #76 patch and didn't got the errors.
Unfortunately, the sitemap is only made by the nodes in the original language, not the translated ones.

An example is better:

Node1: Created in FR, translated (field translation) in NL
=>/fr/node1 Will show up in the FR Sitemap but /nl/node1 will not appears in the NL Sitemap

The opposite is also true
Node2: Created in NL, translated in FR
=>/fr/node2 Will NOT shows up in the FR Sitemap but /nl/node2 will appears in the NL Sitemap

yazzybe’s picture

I successfully applied patch #76 against version 2.3

  • Entity translated nodes are included in the correct language XML sitemap.
  • Language neutral nodes are all included in the default XML sitemap of the default language (they should only be listed in the default sitemap to prevent pushing duplicate content to Google of non-translated nodes).

PS: applying the patch manually was a pain. Hope to see this fix committed in a next release or so.

To me this looks RTBC.

suit4’s picture

Category: Feature request » Bug report

After upgrading a page (with entity translations) to the xmlsitemap 7.x-2.3, I got errors editing and resaving nodes.

Error message was:
PDOException: SQLSTATE[23000]: Integrity constraint violation: 1062 Duplicate entry '272-node-en' for key 'PRIMARY': UPDATE {xmlsitemap} SET subtype=:db_update_placeholder_0, loc=:db_update_placeholder_1, language=:db_update_placeholder_2, access=:db_update_placeholder_3, status=:db_update_placeholder_4, status_override=:db_update_placeholder_5, lastmod=:db_update_placeholder_6, priority=:db_update_placeholder_7, priority_override=:db_update_placeholder_8, changefreq=:db_update_placeholder_9, changecount=:db_update_placeholder_10 WHERE (type = :db_condition_placeholder_0) AND (id = :db_condition_placeholder_1) ; Array ( [:db_update_placeholder_0] => product [:db_update_placeholder_1] => node/272 [:db_update_placeholder_2] => en [:db_update_placeholder_3] => 1 [:db_update_placeholder_4] => 1 [:db_update_placeholder_5] => 0 [:db_update_placeholder_6] => 1464261898 [:db_update_placeholder_7] => 0.5 [:db_update_placeholder_8] => 0 [:db_update_placeholder_9] => 0 [:db_update_placeholder_10] => 0 [:db_condition_placeholder_0] => node [:db_condition_placeholder_1] => 272 ) in drupal_write_record() (line 7336 of /var/www/vhosts/olfasense.com/httpdocs/includes/common.inc).

Like yazzybe I applied patch #76 against 7.x-2.3 which fixed the constraint problem.

Applying the patch was easy using
patch -p 1 < add_support_for_entity_translation-1481798-76.patch

othermachines’s picture

#76 works well for me, thanks very much. Like @sergei_brill, I also don't want untranslated content added, so I'm pleased to have this additional setting.

After applying the patch be sure to run update,php (or drush updb).

milopca’s picture

Assigned: Unassigned » milopca
milopca’s picture

Assigned: milopca » Unassigned
tourtools’s picture

#76 works like a charm, good for a commit

jamsilver’s picture

Status: Needs review » Reviewed & tested by the community

#76 works well

The last submitted patch, 35: interdiff-1481798-34-35-do-not-test.diff, failed testing.

kalistos’s picture

I tried patch #76 and found some problems with it.
Disabling "This translation is published" flag for some translation doesn't remove node from Xmlsitemap with cache clearing.

So I have updated patch and interdiff.

kalistos’s picture

Status: Reviewed & tested by the community » Needs review
gonssal’s picture

Status: Needs review » Reviewed & tested by the community

Just tested #90 and everything seems to work fine. Update hook for PK worked too.

Drupal Commerce site with Entity Translation and fairly big number of products and related taxonomies. Indexed pages, blog posts, menu items and of course, products.

Moving to RTBC again.

kalistos’s picture

I have found that my previous patch doesn't work correctly with programmaticaly update. Source of this issue is in function xmlsitemap_link_load from function xmlsitemap_node_create_link must take $language variable also, if we need to add entity translation support by correct way. So I think we need to rework all places using xmlsitemap_link_load function with adding it.

But now I'm uploading patch that fixes this issue, and it looks it works correctly. But it isn't the best way.

Status: Reviewed & tested by the community » Needs work

The last submitted patch, 93: add_support_for_entity_translation-1481798-93.patch, failed testing.

candelas’s picture

Patch #76 works perfect. Thanks a lot. I have uninstalled xmlsitemap, apply the patch and then install and configure. Now I have the maps in 4 languages using the node and menus modules.

Skin’s picture

Any news? Is this bug still present?

saile’s picture

Tried the patches but when rebuilding the sitemap I get mysql contraint errors
Er is een AJAX HTTP fout opgetreden. HTTP-resultaatcode: 500 Debug informatie volgt. Pad: /batch?id=297&op=do Statustekst: Service unavailable (with message) Antwoordtekst: PDOException: SQLSTATE[23000]: Integrity constraint violation: 1062 Duplicate entry '23636-node' for key 'PRIMARY': INSERT INTO {xmlsitemap} (id, type, subtype, loc, language, access, status, status_override, lastmod, priority, priority_override, changefreq, changecount) VALUES (:db_insert_placeholder_0, :db_insert_placeholder_1, :db_insert_placeholder_2, :db_insert_placeholder_3, :db_insert_placeholder_4, :db_insert_placeholder_5, :db_insert_placeholder_6, :db_insert_placeholder_7, :db_insert_placeholder_8, :db_insert_placeholder_9, :db_insert_placeholder_10, :db_insert_placeholder_11, :db_insert_placeholder_12); Array ( [:db_insert_placeholder_0] => 23636 [:db_insert_placeholder_1] => node [:db_insert_placeholder_2] => veld [:db_insert_placeholder_3] => node/23636 [:db_insert_placeholder_4] => fr [:db_insert_placeholder_5] => 1 [:db_insert_placeholder_6] => 1 [:db_insert_placeholder_7] => 0 [:db_insert_placeholder_8] => 1483980082 [:db_insert_placeholder_9] => 0.5 [:db_insert_placeholder_10] => 0 [:db_insert_placeholder_11] => 0 [:db_insert_placeholder_12] => 0 ) in drupal_write_record() (regel 7376 van /var/www/***/includes/common.inc).

jovial_prash’s picture

#76 is working fine.
Please follow steps given below so as to create xmlsitemaps for multiple enabled languages separately.

1. Apply the patch.
2. To fix PDOException issue please run 'xmlsitemap_update_7204' hook_update.
2. enable 'xmlsitemap_i18n' module.
3. this will create lists of multiple sitemaps (depending on number of languages of the site) at XMLSITEMAP configuration page. If it fails to create on its own then you can add XMLSITEMAPS on your own in multiple enabled languages.
4. Edit required content 'include xml sitemap and save'.
5. refresh sitemap config page-----you will get those edited content (number) in links column.
6. sitemap links will go to 404 initially to fix that 'edit and save again those sitemaps'.
7. xml sitemaps should work now.

jovial_prash’s picture

Status: Needs work » Reviewed & tested by the community
ThuleNB’s picture

Are there any plans to committ this patch soon?

fmmribeiro’s picture

i used patch #90 but i found problem.
If I need a specific setting for a node, the setting doesn't get saved.

Neograph734’s picture

Status: Reviewed & tested by the community » Needs work

This should not have been marked as RTBC. From #93 we already knew there were still issues with the proposed patch in #90 and the newer patch fails testing.

Reverting to needs work. The patch is not yet complete.

dev.patrick’s picture

Tried #76 and this seems to resolve issue. Any plans to commit this patch?

cluke009’s picture

Tried the #93 patch, ran updates and it worked for me. This on a fairly complex production site and testing went smoothly. What exactly is holding up this patch?

alexei.orlov’s picture

#90 or #93 works fine for me, except for vocabularies with 'Localize' translation mode: links from these kind of vocabularies are no further generated. Not applying the patch changes to xmlsitemap_taxonomy.module make it works again. I guess that changes to this module should check the translation mode of the vocabularies...