compare:
/**
* Implements hook_node_type_delete().
*/
function apachesolr_node_type_delete($info) {
module_load_include('inc', 'apachesolr', 'apachesolr.index');
apachesolr_index_mark_for_reindex($env_id, $info->type);
}
to:
function apachesolr_index_mark_for_reindex($env_id, $entity_type = NULL) {
foreach (entity_get_info() as $type => $entity_info) {
...
We are passing the node type (i.e. bundle) in instead of the entity type, and we need to delete all content of that entity_type + bundle not reindex.
| Comment | File | Size | Author |
|---|---|---|---|
| #12 | hook_node_type_delete-1522174-12.patch | 1.62 KB | killua99 |
| #10 | hook_node_type_delete-1522174-10.patch | 1.23 KB | killua99 |
| #4 | 1522174-4.patch | 1.02 KB | pwolanin |
Comments
Comment #1
pwolanin commentedoh, also - $env_id is not defined in the function.
Comment #2
pwolanin commentedAlso, seems like the functionality in function apachesolr_index_node_bundles_changed() should be generic not node specific? why should each entity type have to implement this code which maintains the indexing info?
Comment #3
pwolanin commentedHmm, I see that {node}.status is a field that doesn't necessarily have a corresponding field in other entity types.
Comment #4
pwolanin commentedHere's a first pass at a patch, though not tested yet.
Comment #5
nick_vhI feel we should write some simpletest for this one also, so we can keep confirming that deleting node types actually remove all the information from our tables and index?
Comment #6
pwolanin commentedYes, probably so.
Comment #7
nick_vhLooking good, but we need to add tests, marking as needs work
Comment #8
nick_vhI've been thinking about creating tests, but it is a little bit far fetched. The normal drupal tests should already be sufficient enough to test the deletions of a node type, but copying the code from the node test to test a node type deletion is not what I would call a core-competence of the solr suite?
This plus the fact that we need a working solr (or at least the index page visible) makes me conclude that it is not worth the effort of making tests for this case
I manually tested this scenario (create content type, check content type for indexing, delete content type, verify if the checkbox and data has been removed)
Comment #9
nick_vhCommitted to D7
Comment #10
killua99 commentedBackported ...
Comment #12
killua99 commentedThis patch contain also this issue:
#1630222-2: Fatal error: Cannot use string offset as an array in /sites/all/modules/contrib/apachesolr/apachesolr.module on line 1698
So its easy to review, and solve 2 problems.
Comment #13
nick_vhFuture wise : 1 problem per issue, you confused me a bit!
Committed patch (only the above fix, and removed the "corrected" spacing issue) to 6.x-3.x
Comment #14
nick_vhComment #15
nick_vh