If I clone an environment the bundles selected to be indexed (or excluded if we flip the logic) should be copied too.

CommentFileSizeAuthor
#22 1790894-21-D6x-3x.patch13.95 KBpwolanin
#21 1790894-20-7.x.patch14.52 KBpwolanin
#19 1790894-19-7.x.patch12.81 KBpwolanin
#17 1790894-17-7.x.patch11.39 KBpwolanin
#16 1790894-15.patch9.31 KBnick_vh
#13 1790894-13.patch9.31 KBnick_vh
#11 1790894-11-7.x.patch9.3 KBpwolanin
#6 1790894-6-7.x.patch9.09 KBnick_vh
#4 1790894-3-D6x-3x.patch6.97 KBpwolanin
#4 1790894-3-7.x.patch4.14 KBpwolanin
#3 1790894-2.patch4.09 KBpwolanin
#1 1790894-1.patch3.86 KBpwolanin

Comments

pwolanin’s picture

pwolanin commented
Priority: Normal » Major
Status: Active » Needs review
StatusFileSize
new 1790894-1.patch3.86 KB

It seems it also doesn't update them on save, nor when a node type is changed, so a bit incomplete in general.

Also - we we're clearing the environment cache when a node type changed.

Status: Needs review » Needs work

The last submitted patch, 1790894-1.patch, failed testing.

pwolanin’s picture

pwolanin commented
Status: Needs work » Needs review
StatusFileSize
new 1790894-2.patch4.09 KB

oops - left the cache clearing out of the end of the save function.

pwolanin’s picture

pwolanin commented
StatusFileSize
new 1790894-3-7.x.patch4.14 KB
new 1790894-3-D6x-3x.patch6.97 KB

With some added doxygen and D6 backport.

nick_vh’s picture

nick_vh
he/him
Location Ghent
commented

I'm not sure what you mean with not updating them on save? I just tried it and looks like it works on save?

Could you be a bit more explanatory?

nick_vh’s picture

nick_vh
he/him
Location Ghent
commented
StatusFileSize
new 1790894-6-7.x.patch9.09 KB

Revised version with some clutter removed and tests added to check the bundle array in the environment and the cloned environment.

nick_vh’s picture

nick_vh
he/him
Location Ghent
commented 
+++ b/tests/apachesolr_base.testundefined
@@ -188,11 +181,10 @@ class DrupalSolrOfflineEnvironmentWebTestCase extends DrupalWebTestCase {
     // Load it, to fill our cache.
+    apachesolr_environment_clear_cache();
     apachesolr_environment_load('solr_test');
     $mode = apachesolr_environment_variable_get('solr_test', 'apachesolr_read_only', APACHESOLR_READ_WRITE);
     $this->assertEqual($mode, APACHESOLR_READ_ONLY, t('Environment succesfully changed to read only'));

I'm not sure yet why this is failing if I don't clear the cache here

pwolanin’s picture

pwolanin commented
Status: Needs review » Needs work

You have to remember the 2 different threads, remote vs local. This change is wrong:

     $this->drupalPost($this->getUrl(), $edit, t('Save'));
     $this->assertResponse(200);
-    drupal_static_reset('apachesolr_load_all_environments');
-    drupal_static_reset('apachesolr_get_solr');
pwolanin’s picture

pwolanin commented
Status: Needs work » Needs review
nick_vh’s picture

nick_vh
he/him
Location Ghent
commented

If apachesolr_environment_save() calls apachesolr_environment_clear_cache(), I don't see why I should do it again in the test (and the tests confirm that).

Please elaborate?

pwolanin’s picture

pwolanin commented
StatusFileSize
new 1790894-11-7.x.patch9.3 KB

If you are doing an edit via http POST, then the local thread's static variable cache is not cleared. I wasn't looking at the full context of the test, but if you do a load inside the test code you'd still have the old values.

I don't know why the test failed to run - here's the patch again w/ just a minor comment change.

Status: Needs review » Needs work

The last submitted patch, 1790894-11-7.x.patch, failed testing.

nick_vh’s picture

nick_vh
he/him
Location Ghent
commented
StatusFileSize
new 1790894-13.patch9.31 KB

I must have uploaded an old version before. Anyway, corrected one here.

I did not inherit the code comment because you added a lot of spaces (for the sake of the test I think?)

nick_vh’s picture

nick_vh
he/him
Location Ghent
commented
Status: Needs work » Needs review

Status: Needs review » Needs work

The last submitted patch, 1790894-13.patch, failed testing.

nick_vh’s picture

nick_vh
he/him
Location Ghent
commented
Status: Needs work » Needs review
StatusFileSize
new 1790894-15.patch9.31 KB

doh, juggling too much at once - changed the wrong line.

pwolanin’s picture

pwolanin commented
StatusFileSize
new 1790894-17-7.x.patch11.39 KB

I think this approach is better.

nick_vh’s picture

nick_vh
he/him
Location Ghent
commented
Status: Needs review » Needs work 
+++ b/tests/apachesolr_base.testundefined
@@ -138,15 +167,13 @@ class DrupalSolrOfflineEnvironmentWebTestCase extends DrupalWebTestCase {
+   *	Asserts that we can clone a search environment

You still have those spaces :)

pwolanin’s picture

pwolanin commented
Status: Needs work » Needs review
StatusFileSize
new 1790894-19-7.x.patch12.81 KB

lots lof tabs. fixed.

nick_vh’s picture

nick_vh
he/him
Location Ghent
commented
Status: Needs review » Reviewed & tested by the community

good to go

pwolanin’s picture

pwolanin commented
Status: Reviewed & tested by the community » Needs review
StatusFileSize
new 1790894-20-7.x.patch14.52 KB

With some more test cleanup and making the naming of the new function consistent with 6.x

pwolanin’s picture

pwolanin commented
Version: 7.x-1.x-dev » 6.x-3.x-dev
StatusFileSize
new 1790894-21-D6x-3x.patch13.95 KB

committing #21, here's a 6.x version.

nick_vh’s picture

nick_vh
he/him
Location Ghent
commented
Status: Needs review » Reviewed & tested by the community

Looks good, go ahead and commit

pwolanin’s picture

pwolanin commented
Status: Reviewed & tested by the community » Fixed

Status: Fixed » Closed (fixed)

Automatically closed -- issue fixed for 2 weeks with no activity.