I guess the title says it all

CommentFileSizeAuthor
#64 xmlsitemap-add-files-to-sitemap-1461670-64.patch16.92 KBJelle_S
#63 interdiff-62-63.txt2.26 KBJelle_S
#63 xmlsitemap-add-files-to-sitemap-1461670-63.patch20.12 KBJelle_S
#62 interdiff-57-62.txt11.82 KBJelle_S
#62 xmlsitemap-add-files-to-sitemap-1461670-62.patch20.04 KBJelle_S
#57 interdiff-1461670--54-56.txt802 bytesbrockfanning
#57 xmlsitemap-add-files-to-sitemap-1461670-56.patch16.68 KBbrockfanning
#56 xmlsitemap-add-files-to-sitemap-1461670-55.patch16.67 KBbrockfanning
#56 interdiff-1461670--54-55.txt801 bytesbrockfanning
#54 xmlsitemap-add-files-to-sitemap-1461670-54.patch16.64 KBbrockfanning
#54 interdiff-1461670--52-54.txt2.27 KBbrockfanning
#52 xmlsitemap-add-files-to-sitemap-1461670-52.patch16.45 KBJelle_S
#51 interdiff-1461670-50-51.txt873 bytesJelle_S
#51 xmlsitemap-add-files-to-sitemap-1461670-51.patch16.45 KBJelle_S
#50 interdiff-1461670-48-50.txt1.75 KBbrockfanning
#50 xmlsitemap-add-files-to-sitemap-1461670-50.patch15.97 KBbrockfanning
#48 interdiff-1461670-47-48.txt1.78 KBbrockfanning
#48 xmlsitemap-add-files-to-sitemap-1461670-48.patch14.22 KBbrockfanning
#45 xmlsitemap-add-files-to-sitemap-1461670-45.patch16.72 KBwildfeed
#45 interdiff-1461670-41-45.txt2.24 KBwildfeed
#43 xmlsitemap-add-files-to-sitemap-1461670-43.patch14.83 KBwildfeed
#43 interdiff-1461670-41-43.txt712 byteswildfeed
#41 xmlsitemap-entity-integration-1461670-41.patch14.51 KBnicola85
#40 xmlsitemap-entity-integration-1461670-40.patch15.02 KBnicola85
#38 xmlsitemap-entity-integration-1461670-38.patch15.08 KBnicola85
#33 xmlsitemap-entity-integration-1461670-33.patch14.3 KBosopolar
#29 xmlsitemap-entity-integration-1461670-29.patch14.23 KBosopolar
#29 xmlsitemap-23-29.diff.txt4.94 KBosopolar
#26 xmlsitemap-entity-integration-1461670-26.patch14.23 KBosopolar
#26 xmlsitemap-23-26.diff4.94 KBosopolar
#23 default-entity-integration-1461670-23.patch11.14 KBMatthijs
#22 default-entity-integration-1461670-22.patch11.14 KBMatthijs
#21 default-entity-integration-1461670-21.patch11.08 KBMatthijs
#19 default-entity-integration-1461670-19.patch9.77 KBMatthijs
#44 interdiff-1461670-41-44.txt3.02 KBwildfeed
#44 xmlsitemap-add-files-to-sitemap-1461670-44.patch17.5 KBwildfeed
#47 interdiff_1461670-41-47.txt6.54 KBwildfeed
#47 xmlsitemap-add-files-to-sitemap-1461670-47.patch13.31 KBwildfeed
Support from Acquia helps fund testing for Drupal Acquia logo

Comments

Anonymous’s picture

Version: 7.x-2.0-rc1 » 7.x-2.x-dev
Status: Active » Postponed (maintainer needs more info)

I guess not. You need to be more specific with your request. What do you mean by "Entity Type"?

fmizzell’s picture

Sorry, I guess I am an entity guy, I thought Entity Type had a very specific definition. Node, User, Taxonomy Term, Taxonomy Vocabulary and Comment are examples of entity types in core, but anyone can create an entity type for D7. Profile 2, Relation, Commerce, all those module create (or define might be a better word) their own entity types. Some of these entity types have defined ways in which uris are created for their entities, and XML Sitemap should be able to work for those entities that have a uri defined. I hope that is clearer.

Anonymous’s picture

Status: Postponed (maintainer needs more info) » Closed (won't fix)

Most things in Drupal are nodes with specific content types. Those are already all covered. So is taxonomy, user and menu. There is a custom links module to cover the unusual. There is already a request for views and some other content. Adding another feature request for anything that might be content in a general manner will not take place since the unknown cannot be programmed for.

fmizzell’s picture

Status: Closed (won't fix) » Active

@earnie Entity Types are not unknowns, neither is whether they have a path or not. Completely matching the exact functionality that XML Sitemap offers for nodes with any entity could not be possible right now with what core offers, but IMHO XML Sitemap could do a lot better with the information available. Plus there might be other people like me willing to do the work, so if you don't mind I would appreciate if you could leave the issue open just a little bit longer, as I don't believe the request is out of the scope of this module.

Anonymous’s picture

Status: Active » Postponed (maintainer needs more info)

I can see this as a possible add on module but if it isn't available via core functions then it isn't going to happen for xmlsitemap. The only possible request would be an additional hook to help the add on module and that depends on how much weight the patch brings with regard to processing time.

The major scope of this module is to provide a sitemap.xml rendering of links to nodes in a manner that is fast and efficient for 100's of 1000's of nodes using the core API. If any request has the potential to reduce "fast and efficient" then it will not be considered as part of this module. This module does already provide hooks for you to use.

Can you prove that your concept for a feature will not add processing time or memory use to xmlsitemap?

fmizzell’s picture

That's fair... I will take a closer look at the code and the kind of changes that would be necessary for the add on module to work, and then we can go from there.

Dave Reid’s picture

So yeah, the plan is to eventually have a version that is entity-agnostic since it makes sense. However my experience implementing those types of modules so far in Drupal 7 has proven horribly frustrating (especially with respect to determining which entity types are 'publicly visible' e.g. Pathauto, Meta tags, etc) - #1332058: Need a way to distinguish "public/final" URIs for entities from admin and redirected ones. For example, comment module implements the entity URI callback when it doesn't actually have it's own pages - they just redirect to node pages. Commerce products implement entity URI pointing to the admin URL of the products, not the public-facing URL. So it's a combined matter of frustration with D7 and not being spread too thin between modules.

We're also missing a unified way to alter the 'bundle' forms for entities (e.g. node_type_form, taxonomy_form_vocabulary, etc).

Ideally this rewrite would happen on a 7.x-3.x branch which makes me queasy since 7.x-2.x isn't technically "finished" yet, but it would help clean things up a bit.

kevinquillen’s picture

So there's no way to get an entity like Product or Profile2 into an XML Sitemap???

makangus’s picture

I am trying to figure out how to get some custom entities into XML Sitemap, but I am not sure if I will end up making it general enough for all Entity Type.

makangus’s picture

After writing some code to integrate my custom entity into XML Sitemap, as well as writing a basic module to integrate Profile2 with XML sitemap. I am not sure if it's possible at all at this moment to write something entity-agnostic. I don't even see a simple way to figure out if an entity has an unique ID. I am thinking maybe the better approach, at least for the time being, is to provide a simple hook for modules with custom entities to easily integrate with xmlsitemap. Currently we need to connect to about 8 hooks to get a custom entity into xmlsitemap.

@kevinquillen, here is my first attempt to integrate Profile2 with XML sitemap. http://drupal.org/sandbox/makangus/1783144

bisonbleu’s picture

@makangus, what is the status of your profile2_xmlsitemap project?

That's exactly what I need for a project I'm developing and where users profiles (daycares) need to be added to the xmlsitemap so that they can be indexed by search engines.

makangus’s picture

@bisonbleu, I am not doing much with profile2_xmlsitemap but it does work. You might be able to achieve the same results with the xmlsitemap_user module that comes with xmlsitemap, so I would recommend trying that first. If you do end up using my sandbox module though, feel free to file any bugs.

kevinquillen’s picture

It has to be a case by case basis due to how entities are implemented through EntityAPI - that's what I have found. You could make some workarounds but it is a bit of work.

bisonbleu’s picture

The xmlsitemap_user module is not very useful for my uses case because I use the separate profile page option. User links in xmlsitemap point to user accounts where all you can see is the following.

History

Member for
x month y weeks

kevinquillen’s picture

That's why there's no unified solution, yet.

If you work with the xmlsitemap_profile2 you can adapt it to do what you need.

makangus’s picture

@bisonbleu, you can try my sandbox module and see how that works out for you, but let's move this conversation about profile2 to the issue queue there http://drupal.org/project/issues/1783144.

Let's keep this thread about XMLSitemap for ALL Entity Type.

acbramley’s picture

The apachesolr module does this really nicely and instantly provides any entity type that is set as "indexable" in it's config form. This is sourced from hook_apachesolr_entity_info_alter which is invoked in apachesolr_entity_info_alter and by simply setting $entity_info['my_entity']['indexable'] = TRUE; it is exposed to the index config form.

Could a similar methodology not be implemented for xmlsitemap?

acbramley’s picture

Thanks @makangus for the sandbox, I used it to implement xmlsitemap functionality for my entity type. Copy-paste-find-replace FTW :D

Matthijs’s picture

Issue summary: View changes
Status: Postponed (maintainer needs more info) » Needs review
FileSize
9.77 KB

I've written a patch - using makangus code - to add this feature for all Entity API powered entities.

Please note that an uri and access callback are required. And one of the uri methods must be overridden when entity_class_uri() is used as URI callback.

Matthijs

Status: Needs review » Needs work

The last submitted patch, 19: default-entity-integration-1461670-19.patch, failed testing.

Matthijs’s picture

Status: Needs work » Needs review
FileSize
11.08 KB

Fixed "apply patch" issue and some other remaining bugs...

Matthijs’s picture

FileSize
11.14 KB

One of the select queries was missing the field to select...

Matthijs’s picture

FileSize
11.14 KB

Another quick fix, the bundle should be empty...

tobiberlin’s picture

Hi...

I tried your patch to get my custom entities into xml sitemap but when I generated the sitemap the following the batch was interrupted and the following messages appeared in drupal logs:

Warning: Missing argument 2 for xmlsitemap_entity_xmlsitemap_process_entity_links(), called in sites/all/modules/xmlsitemap/xmlsitemap.generate.inc on line 456 and defined in xmlsitemap_entity_xmlsitemap_process_entity_links() (Line 212 of /sites/all/modules/xmlsitemap/xmlsitemap_entity/xmlsitemap_entity.module).

Notice: Undefined variable: entity_type in xmlsitemap_entity_xmlsitemap_process_entity_links() (Line 213 of /sites/all/modules/xmlsitemap/xmlsitemap_entity/xmlsitemap_entity.module).
	
Notice: Undefined index: controller class in entity_get_controller() (Line 7836 of /includes/common.inc).

Additionally I find no place where I can define the standard xml settings for the entity type. Here you can see the relevant parts of my entity hook info:

      'admin ui' => array(
        'path' => 'admin/path/entity',
        'menu wildcard' => '%entity',
        'file' => 'customentitiy.admin.inc',
      ),
     'xmlsitemap' => TRUE,

Maybe I miss something... on my admin page I just see a list of my entities and the link to add a new entity which is quite the standard I think?!

Barry_Fisher’s picture

Thanks to @matthijs for the patches!

I found a similar issue mentioned in #24

I didn't get the Missing Argument error, but was unsure how to set the status (inclusion) and priority of each bundle.

I've done some digging.

In my case I'm using ECK, so there is nothing in the patch in #23 which provides a per bundle setting for custom (or ECK) entities- or at least from what I could see.

I got the entities to be included by adding a hook_entity_info_alter() in my custom module like this:

/**
 * Implements hook_entity_info_alter().
 */
function MY_MODULE_entity_info_alter(&$entity_info) {
  $entity_info['product']['xmlsitemap'] = TRUE;
}

(if you're creating own custom entities (not ECK) you should be able to add this to your hook_entity_info())

This allowed for the xmlsitemap inclusion in the xmlsitemap_entity module (as provided in #23), but there was still an additional step required. The main xmlsitemap module looks for the bundle settings (normally set in the admin UI). In xmlsitemap_link_bundle_load() it looks for your custom bundle settings in the variable table, much like it does for node. However, given that there is no UI to set this variable (in the case of ECK entities) I had to add these lines to my settings.php:

$conf['xmlsitemap_settings_product_widget'] = array(
  'status' => '1',
  'priority' => '1.0',
);

...where 'product' is the ECK entity type and 'widget' is the bundle.

Flush caches and everything worked as expected.

Note: I'm not concerned with individual entity priorities, so these steps aren't a complete fix for ECK. I might be able to do this by looking at the way xmlsitemap_entity_form_alter() is hooking into the entity form. The conditions in this function won't match those of ECK's entity forms, so some work would be required there is that's what you need to achieve.

Hope this helps somebody.

osopolar’s picture

I use this for field-collection-items. It seems to work, but as field-collections don't have an edit bundle form (as for example nodes have), there is no place to set the status and priority settings. It might be done in the field settings, but this is maybe to complicated for a generic approach. I set the variable xmlsitemap_settings_field_collection_item_{field-collection-name} manually (via hook_update_N() implementation in a custom module). Maybe the xmlsitemap_entity module should get it's own admin page?

I haven't got the errors from #23, it's not clear why argument 2 (variable $entity that contains the entity-type) would be missing there. Probably the patch was not applied completely as the $entity variable is used in different places of the same function and it absence would have caused more errors. The patch in #23 adds $entity as second parameter to the 'process callback', the only place where xmlsitemap_entity_xmlsitemap_process_entity_links() gets called in that function.

I was wondering about the function xmlsitemap_link_queue_process() in xmlsitemap.module. There is not second parameter (entity-type) for the 'process callback'. It couldn't be added that easy, as there is another function, xmlsitemap_menu_xmlsitemap_process_menu_links() which receives a second parameter, although it is not set in xmlsitemap_link_queue_process(). Anyway we could not just add $data['type'], which seems to be the entity-type as a second parameter to the callback, as it would get misinterpret in xmlsitemap_menu_xmlsitemap_process_menu_links().

Therefore I changed the second parameter for all xmlsitemap_*_xmlsitemap_process_*_links() callbacks (only the one above mentioned and the new one xmlsitemap_entity_xmlsitemap_process_entity_links() into a data array which may contain different information.

I haven't yet tested the sitemap for xmlsitemap_menu, but I hope the changes won't affect it.

Status: Needs review » Needs work

The last submitted patch, 26: xmlsitemap-entity-integration-1461670-26.patch, failed testing.

The last submitted patch, 26: xmlsitemap-23-26.diff, failed testing.

osopolar’s picture

Status: Needs work » Needs review
FileSize
4.94 KB
14.23 KB
osopolar’s picture

Status: Needs review » Needs work

I get following warning when importing redirects with the Path redirect import module (having the File Entity module installed):

Notice: Undefined property: stdClass::$pid in xmlsitemap_entity_entity_delete() (line 146 of [...]/xmlsitemap/xmlsitemap_entity/xmlsitemap_entity.module).

There entity to be deleted is the (temporary) redirect csv file temporary://redirects.csv.

Would it be okay to test for pid like in the following code snipped?

  if (!empty($info['xmlsitemap']['use entity module']) && isset($entity->pid)) {
    xmlsitemap_link_delete($entity_type, $entity->pid);
  }

This would be fine for the temporary file, as it isn't be part of the sitemap anyway. I am not sure if that is fine in general for all other entities (like other file entities which might be part of the sitemap).

joachim’s picture

Category: Feature request » Bug report

I am getting this error too, when deleting a file.

I don't know what the pid property is meant to be, but files don't have it. It's not something that xmlsitemap sets (there is no other mention of it anywhere in the module's code).

Lots of other entities don't have it either, but 'use entity module' is set on entity types that satisfy fairly general criteria:

    // Check if this type of entity can be indexed.
    $indexable = (isset($info['entity keys']['id']) && isset($info['uri callback']) && isset($info['access callback']));

That's going to be a lot of entity types!

Is pid perhaps a typo?

joachim’s picture

Yup, looking at the signature of xmlsitemap_link_delete(), this is a typo.

The fix is to use entity_extract_ids() to get the ID before calling the function.

osopolar’s picture

Status: Needs work » Needs review
FileSize
14.3 KB

Patch changed as suggested in #32:

   if (!empty($info['xmlsitemap']['use entity module'])) {
-    xmlsitemap_link_delete($entity_type, $entity->pid);
+    list($entity_id) = entity_extract_ids($entity_type, $entity);
+    xmlsitemap_link_delete($entity_type, $entity_id);
   }
screon’s picture

Can anyone confirm the latest patch works for custom entities built with ECK?

JeroenT’s picture

+++ b/xmlsitemap_entity/xmlsitemap_entity.api.php
@@ -0,0 +1,27 @@
+ * Describes the hooks provided by the Workflow module.

Workflow should be replaced with XML sitemap entity.

I enabled the module and got the following error:
Warning: Invalid argument supplied for foreach() in element_children() (line 6540 of includes/common.inc).

milos.kroulik’s picture

Would it be possible to add some instruction to the README? The patch applied fine, but I wasn't able to make it work. My custom entities were apparently indexed fine, but not added to the sitemap.

jmdorian’s picture

@milos.kroulik
I've added some code to my module in form_alter hook (you can use specific, I've added to general):

if ($form_id == 'eck__bundle__edit_form') {
    module_load_include('inc', 'xmlsitemap', 'xmlsitemap.admin');
    xmlsitemap_add_link_bundle_settings($form, $form_state, $form['entity_type']['#value']->name, $form['bundle']['#value']->name);
  }

This code adds a fields to a form and submit function, that's why you don't need do nothing else to get it works.

nicola85’s picture

Status: Needs review » Needs work

The last submitted patch, 38: xmlsitemap-entity-integration-1461670-38.patch, failed testing. View results

nicola85’s picture

nicola85’s picture

ggmacasaet’s picture

Need some help on this since i've already patched the codes that we have here but the custom entity that i have is still not yet visible. I already have it indexed and available but its not going to VISIBLE thus not showing on the XML sitemap.

wildfeed’s picture

Patch in #41 worked well but did not support adding files to the sitemap. In this patch I added add a form element to the file-type edit form. By editing a file-type and setting the "Inclusion" item to "Include" files are added to sitemap.xml when cron runs.

wildfeed’s picture

Re rolled the patch in #43 - which was based on the patch in #41. This updated code adds a new module, xmlsitemap_file which supports adding files to the sitemap. The module also appends "/download" to the file path before adding the link to sitemap.xml - this way a crawler can actually access the contents of a .pdf file.

wildfeed’s picture

This patch adds a module to the patch in #41. It adds support for adding files to sitemap.xml and allows for "/download" to be added to url so a text file can be displayed in the browser and indexed.

brockfanning’s picture

wildfeed’s picture

Status: Needs work » Needs review
FileSize
13.31 KB
6.54 KB

This patch adds a module (xmlsitemap_file) to the patch in #41.

XML sitemap file supports adding files to sitemap.xml and allows for "/download" to be added to link url (optional) so a text file can be displayed in the browser and indexed.

XML sitemap file passes "timestamp" as "changed property" so $entity->timestamp is used for "Last modification date"

patch fixes issue in comment #35 - replaces reference to "Workflow module" with "XML sitemap entity module"

Please review

brockfanning’s picture

This update adds the per-entity override form to file entities, so files can be individually included/excluded. It also adds some code that seems to be needed in order for the entity edit form to remember the overrides for an entity. Interdiff attached.

General notes: The xmlsitemap_file module added in recent patches seems like a good example of leveraging the xmlsitemap_entity module for integration with another contrib module. I notice that ECK (entity construction kit) needs a bit of help as well, in getting the various form elements added. Maybe a similar module (xmlsitemap_eck?) could be added in this patch? Field Collection is mentioned above also, so might be another candidate for a module.

It could be argued that these contrib integrations should be in separate patches, and that only the xmlsitemap_entity module should be added here. I would support that, except that it's not clear what the xmlsitemap_entity module should do by itself. If it's only an "API module", then that should be made clear, and maybe an example implementation provided (like the xmlsitemap_file from recent patches).

Jelle_S’s picture

Status: Needs review » Needs work

Looking at the interdiff from #41 to #47 (in comment #47) the second argument to the process callback (array('entity_type' => $entity)) was removed again, which causes it to fail indexing for me. Subsequent patch in #48 doesn't seem to fix that.

Also, xmlsitemap_entity_query_alter() contains a fix for EntityFieldQueries for users (because of #1054168: EntityFieldQuery fails for entities that have no bundle). As long as #1054168: EntityFieldQuery fails for entities that have no bundle is not fixed, maybe we should add a fix for entities without bundles in general?

brockfanning’s picture

Thanks for pointing that out @Jelle_S about the argument issue - I'm getting the same thing. Here's an attempt to add that code back in.

Your idea about entities without bundles seems reasonable to me. I don't have time at the moment to try implementing that, but it does seem like something that should be done in this issue.

Jelle_S’s picture

Status: Needs work » Needs review
FileSize
16.45 KB
873 bytes

My stab at it.

Jelle_S’s picture

Rerolled against latest dev.

ThuleNB’s picture

I installed the latest dev version and recognized some problems:

  • The sitemap urls result in a 404 page not found
  • I have two languages installed on my page but I can't choose a language for the sitemaps.
  • I have no UI to choose the entities that I want to include in the sitemap
  • Field collections are not included in the sitemap

Running cron, deleting cache and rebuilding sitemaps do not resolve this.

brockfanning’s picture

Status: Needs review » Needs work
FileSize
2.27 KB
16.64 KB

Regarding the file entity integration: After further testing in production I believe that a change I added to this patch in #48 was probably unnecessary. Instead a simpler form alteration is need for a specific use-case involving some common file-related modules (file_entity, media, media_bulk_upload, and multiform). So in the interests of keeping unnecessary stuff out of the patch, here is an update to make those changes.

In regards to the review by @ThuleNB: This is great feedback. It sounds like this patch does NOT provide complete integration with field_collection entities. Getting feedback like this on a variety of entity types will probably be needed to truly "generalize" this module.

@ThuleNB: I don't have much experience using field collections as stand-alone entities -- they do get their own URLs? I always thought field collections were purely for attaching to other entities, but sounds like I'm mistaken there?

General note: Obviously also, this patch is really in need of some tests.

Jelle_S’s picture

@brockfanning: That is the way I understood as wel, but from the Field Collection project page:

A field collection is internally represented as an entity, which is embedded in the host entity. Thus, if desired field collections may be viewed and edited separately too.

So yes, they do get their own urls.

See also field_collection_field_get_path() and FieldCollectionItemEntity::defaultUri().

brockfanning’s picture

@Jelle_S: A minor tweak to the entity query alter, to avoid a PHP warning that happens in my Drupal install.

brockfanning’s picture

Arg, fixing a typo.

Jelle_S’s picture

Hm, weird, seems like entityConditions should never be a string... See https://api.drupal.org/api/drupal/includes%21entity.inc/function/EntityF...

Wich means we should not only check the value, but also the operator...

Which got me thinking. Maybe we shouldn't be fixing this here... It seems this is in place to fix the uses of EntityFieldQuery in xmlsitemap itself. Maybe we should fix it there (used in xmlsitemap.drush.inc, xmlsitemap.generate.inc, xmlsitemap.module). We could just do the same check there before adding the bundle condition. Maybe that's something to fix in a different patch though? Since a similar hook is already in place for the user entity.

Jelle_S’s picture

brockfanning’s picture

@Jelle_S: Yes I think I'm leaning that way too. Probably the best this patch can do is to lay the (potentially incomplete) groundwork for support of other entity types, and then maybe to include complete support for some common/popular entity types (like the currently included xmlsitemap_file). If that entity query alter is mainly for support of user entities, then it may belong in an "xmlsitemap_user" submodule?

Jelle_S’s picture

Well it's for bundle-less entity types, of which user is a core example. But we can't special case it for user since https://www.drupal.org/project/user_bundle exists. That is why I created the patch over at #3011651: Fix EntityFieldQuery uses. It checks the entity info to see if the entity type we're querying for has bundles. It's a generic approach that should work for all entities (even when they're altered like User Bundle does). That patch also removes the hardcoded special case for user entities.

Jelle_S’s picture

Did some code cleanup (trailing spaces, comment punctuation, code extraction to a separate function, and some early returns to limit if-nesting). If or when #3011651: Fix EntityFieldQuery uses get committed we can remove xmlsitemap_entity_entity_query_alter().

Jelle_S’s picture

Fixed some copy paste errors.

Jelle_S’s picture

FileSize
16.92 KB

Patch contained changes from the other issue...