Fix #890 Enable search solr #906

Saphyel · 2017-05-24T14:18:32Z

Fixes #890
Is blocking #916

Changes proposed in this PR

Enable it locally
Enable it in Plat.sh
Fix the annoyance of ESU

pvhee

@Saphyel search doesn't seem to be working (or itsn't enabled properly) on plat.sh?

Sample search: https://pr-906-6ew4uwq-tx3mbsqmxtu74.eu.platform.sh/search?text=bake

Saphyel · 2017-05-24T14:47:43Z

@pvhee is not working properly in plat.sh but I'm fixing it

I also upgraded it to 6.3 that accepts multiple cores (maybe something good for the future?)

pvhee · 2017-05-25T13:21:28Z

@Saphyel can we separate module updates in their own PRs please? will make it easier, and I don't think this work requires you to update the modules?

Saphyel · 2017-05-25T13:39:55Z

@pvhee indeed this requires it because unfortunately I can't update the config easily... it requires some fields (that should not require), doesn't show the proper values, etc...

I guess because we were using a dev version this is expected.

About split this PR, why ? update those modules now is minimal in the PR and is related with this task.

If we separate this into 2 I have to take care of 2 PR and make sure that works fine in the both sites and if one fails we might need to revert 2 PR.. I don't want to over complicate this issue more than needed.

pvhee · 2017-05-25T14:57:38Z

@Saphyel we could consider running Docker in Travis. This will make the setup in Travis cleaner and more on par with local dev?

Saphyel · 2017-05-25T15:00:33Z

@pvhee we use serveral images so if Travis support a network of containers it will possible or if we use an external address to connect with that services.

pvhee · 2017-05-25T15:25:08Z

@Saphyel docker-compose is supported by Travis, so we could use this. Will add to a new ticket, that we can handle before finishing off solr integration here if this would make our life with Travis easier.

pvhee · 2017-05-25T15:26:22Z

^^ #912

Saphyel · 2017-05-31T16:09:33Z

@pvhee @pradip3271 ready to be merge! (Travis is having issues at the moment so better wait a bit)

pvhee · 2017-05-31T16:59:21Z

@Saphyel Travis should also fail if errors are detected in the Drupal-Solr connection, see e.g.

 360  31/May 16:45  search_a  error     Drupal\search_api_solr\SearchApiSolrExc 
                    pi                  eption: Solr HTTP error: OK (500)       
                                                                                
                                                                                
                                                                                
                                        Error 500                               
                                        {metadata={error-class=org.apache.solr. 
                                        common.SolrException,root-error-class=o 
                                        rg.apache.solr.core.Solr

pvhee · 2017-06-01T14:30:41Z

tests/behat/features/search.feature

@@ -1,3 +1,4 @@
+@not-on-travis
 Feature: Search


@Saphyel we need to find a way in which we can test search on Travis, as this is a critical part of the application. Any ideas?

I think I found the issue, is related with the bootstrap of solr on travis.

Drupal is trying to use Solr before than Solr is ready

pvhee

Fix search testing on Travis

Saphyel · 2017-06-01T16:02:58Z

@adamclark-dev do you mind to take a look to the whole PR?

pvhee · 2017-06-02T08:08:14Z

ci/travis/run-tests.sh

@@ -8,6 +8,6 @@ cd ..
 # Check the config match with the info.yml
 #phing config:check
 # Run behat tests
-vendor/bin/behat
+vendor/bin/behat --tags=~not-on-travis


@Saphyel is the not-on-travis tag still relevant now that we use docker on Travis, so we should be able to run all tests anyways?

I removed all of them, so there is not harm

So why are you keeping the not-on-travis tag? Remove as well, keeping things consistent.

pvhee · 2017-06-02T08:09:30Z

modules/cr_solr/config/install/block.block.seven_exposedformsearchpage_1.yml

+  provider: views
+  label_display: '0'
+  views_label: ''
+visibility: {  }


@Saphyel upgrade path to consider?

what do you want to add in the upgrade path? In the comic relief site works fine

Aren't you proposing a new config? How will sites get this automatically included via their upgrade path? Also consider rnd.com. For a full discussion pls refer to #926

Then I have a question @pvhee shall I use our old way or should this be block until we have a decision about the thunder implementation?

I don't think we're ready to fully roll out config updates in the update hooks, but I want us to consider this. Shouldn't block this PR though

pvhee · 2017-06-02T08:10:33Z

tests/behat/features/search.feature

@@ -20,4 +19,4 @@ Feature: Search
  @default-content
  Scenario: Search in the DB Vitkauskas
    Given I am on "search?text=Vitkauskas"
-    Then I should see the link "YPlan partners with Comic Relief"
+    Then I should see the link "YPlan partners with"


@Saphyel I think we would benefit of writing additional tests for searching media, could this be part of this PR or do you prefer to tackle in a new issue?

I prefer other issue because QA can handle that

@Saphyel #928

* Fix #897 Remove modernizr * 862 remove copying gem from composer * 862 update sass grunt path * 862 add default img update twig style * 862 update pl version npm * 862 remove spaces twig * Fix #897 Enable modernizr from node * fixes #910 article, partner page to run in local (#910) * fixes #910 for article, partner page to run in local * #910 Reformat code * #912 Use docker-compose to run Travis build * 863 - Initial module build to display articles in block by taxonomy type * 863 - Fix incorrectly named template and add default value to admin form * 863 - Add installation configuration for feature articles block * 863 - Reformat code to match drupal code style * 863 - Remove superfluous class from feature articles template * 863 - Add article link to feature article nodes * 863 - Add behat tests for feature articles * #912 Add cache * 912 Fix Media feature test * #917 Exclude broken stuff * 912 Travis cache not work as expected * 912 Fix Media tests * 863 - Migrate custom feature articles logic to allow for feature articles to be displayed as custom block or paragraph * 863 - Reformat code to match drupal 8 guidelines * 863 - Add feature article dependencies to module * 863 - add dockblocks to feature articles module * 863 - Remove custom block, migrate template to run off of teasers and add config to module * 863 - Remove uuid from feature article config items * 863 - Fix feature article background colour to use colours function * 863 - Add missing config dependencies to feature articles module * 863 - Add background colour class to feature articles module * 863 - Add default content for feature articles module * 823 - Update feature articles test url * 863 - Remove tests on non existent regions for feature articles behat tests * 863 - Update pattern lab version for feature articles * 863 - Add newline to feature articles test * 863 fix layout * 884 update meta icon block config (#900) * 884 update block config * 884 Add admin theme metablock config * 863 - Add title and update default content to include titles * 863 - max 3 items * 863 - separate layout from styles * 863 fix logo campaign position * 863 - update bhat test * 863 - add new line github warning * 863 - add new line github warning * 863 recovering old campaign styles * 863 - Remove config hash and uuid from exported config * 863 minor fix mobile logo size * 863 - Fix featured articles to be filtered by display date * 863 - Fix tests due to ordering changes in feature articles module * 865 - exclude articles from being categorised as all (#925) * 865 - Update config to exclude articles and add update script * 865 - Add tests to test exclude from aggregator views feature * 863 - Correct incorrectly remove .gitignore extension * 865 - Remove un-necessary field from configuration, breaking install. * 865 - Remove un-necessary test as is covered by news articles test * 865 - Add missing line endings to files * 865 - Fix behat test path for exclusion test * 865 - Add missing config_devel to articles module * 865 - Rename exclude aggregator fields to match naming conventions * 865 - Refference new field-article-exclude-aggr field name in behat tests * fix 930 * 863 - Update cr.install to fix missing function brace * Fix #890 Enable search solr (#906) * 890 Enable Solr on platform.sh * 890 Enable cr_solr on local * 890 Fix ESU issue * 890 Fix more tests * 890 Fix tags on tests * 890 Remove tag on travis * 924 Quote block config * 924 Single msg config * 924 Story config * 863 - Fix issue with field not being allowed in twig due to missing #type (#936) * 867 - Add views_ajax_history module * 942 Adding xls, xlsx file types to downloadables (#946) * 943 - update names and description of sing msg padding option (#947) * 943 - update names and description of sing msg padding option * 937 Add custom node link * 937 Remove readmore link from content wall display * 949 - Change configuration to hide from feature articles paragraph type. (#950) * 949 - Change configuration to hide from feature articles paragraph type. * 949 - Add javascript tag to test to fix failing test * 949 - Move add publish up to fix feature article exclude test. * 867 - Add scroll into view for submit button on behat test * 949 - Reinstall article configuration as part of install and fix incorrect module configuration * 949 - Add new line toy cr_article configuration.

Saphyel self-assigned this May 24, 2017

Saphyel requested a review from pvhee May 24, 2017 14:28

pvhee suggested changes May 24, 2017

View reviewed changes

Saphyel force-pushed the 890 branch 3 times, most recently from 16c5a89 to 57d65e6 Compare May 25, 2017 11:03

Saphyel force-pushed the 890 branch from fae7205 to b7c2f1b Compare May 25, 2017 13:36

Saphyel force-pushed the 890 branch from b7c2f1b to 6e8305a Compare May 25, 2017 14:20

Saphyel force-pushed the 890 branch 5 times, most recently from 2a0be96 to 4646cef Compare May 31, 2017 13:06

Saphyel requested a review from pradip3271 May 31, 2017 15:50

Saphyel added the Status: Needs Review label May 31, 2017

pvhee added Status: Needs Work and removed Status: Needs Review labels May 31, 2017

pvhee requested a review from adamclark-dev May 31, 2017 17:00

Saphyel force-pushed the 890 branch 3 times, most recently from 7e59f5f to e5c4a74 Compare June 1, 2017 13:42

Saphyel force-pushed the 890 branch 2 times, most recently from a156d07 to fcdedb8 Compare June 1, 2017 14:22

pvhee reviewed Jun 1, 2017

View reviewed changes

pvhee suggested changes Jun 1, 2017

View reviewed changes

Saphyel force-pushed the 890 branch 4 times, most recently from 979295e to b59df89 Compare June 1, 2017 15:50

Saphyel added Status: Needs Review and removed Status: Needs Work labels Jun 1, 2017

pvhee reviewed Jun 2, 2017

View reviewed changes

adamclark-dev approved these changes Jun 2, 2017

View reviewed changes

pvhee mentioned this pull request Jun 2, 2017

Write Behat tests around Media search #928

Closed

Saphyel added 6 commits June 2, 2017 12:02

890 Enable Solr on platform.sh

651afa0

890 Enable cr_solr on local

abe292a

890 Fix ESU issue

52f14b8

890 Fix more tests

8023ced

890 Fix tags on tests

aaa6850

890 Remove tag on travis

ee3e16c

Saphyel force-pushed the 890 branch from aaa6ca3 to ee3e16c Compare June 2, 2017 11:02

Saphyel removed the request for review from pradip3271 June 2, 2017 11:22

pvhee approved these changes Jun 2, 2017

View reviewed changes

pvhee merged commit 60f0245 into develop Jun 2, 2017

pvhee deleted the 890 branch June 2, 2017 13:42

bimsonz mentioned this pull request Jun 8, 2017

1.31.1 #963

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Fix #890 Enable search solr #906

Fix #890 Enable search solr #906

Saphyel commented May 24, 2017 •

edited

pvhee left a comment

Saphyel commented May 24, 2017 •

edited

pvhee commented May 25, 2017 •

edited

Saphyel commented May 25, 2017 •

edited

pvhee commented May 25, 2017

Saphyel commented May 25, 2017

pvhee commented May 25, 2017

pvhee commented May 25, 2017

Saphyel commented May 31, 2017

pvhee commented May 31, 2017

pvhee Jun 1, 2017

Saphyel Jun 1, 2017 •

edited

pvhee left a comment

Saphyel commented Jun 1, 2017

pvhee Jun 2, 2017

Saphyel Jun 2, 2017

pvhee Jun 2, 2017

pvhee Jun 2, 2017

Saphyel Jun 2, 2017

pvhee Jun 2, 2017

Saphyel Jun 2, 2017

pvhee Jun 2, 2017

pvhee Jun 2, 2017

Saphyel Jun 2, 2017

pvhee Jun 2, 2017

Fix #890 Enable search solr #906

Fix #890 Enable search solr #906

Conversation

Saphyel commented May 24, 2017 • edited

Changes proposed in this PR

pvhee left a comment

Choose a reason for hiding this comment

Saphyel commented May 24, 2017 • edited

pvhee commented May 25, 2017 • edited

Saphyel commented May 25, 2017 • edited

pvhee commented May 25, 2017

Saphyel commented May 25, 2017

pvhee commented May 25, 2017

pvhee commented May 25, 2017

Saphyel commented May 31, 2017

pvhee commented May 31, 2017

Choose a reason for hiding this comment

Saphyel Jun 1, 2017 • edited

Choose a reason for hiding this comment

pvhee left a comment

Choose a reason for hiding this comment

Saphyel commented Jun 1, 2017

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Saphyel commented May 24, 2017 •

edited

Saphyel commented May 24, 2017 •

edited

pvhee commented May 25, 2017 •

edited

Saphyel commented May 25, 2017 •

edited

Saphyel Jun 1, 2017 •

edited