Feature/2820072 phpcs eslint (#11)

* automated fixes

* fix phpcs Drupal warnings

* fix drupal best practice warnings

* fix eslint

* fix schema

* depend on breakpoint_js_settings

.github/PULL_REQUEST_TEMPLATE.md

@@ -0,0 +1,10 @@
+Make sure these boxes are checked before submitting your pull request - thank you!
+
+- [ ] All coding styles are fulfilled. ([How to check for cs issues?](https://github.com/BurdaMagazinOrg/thunder-dev-tools/blob/master/README.md#code-style-guidelines))
+- [ ] All tests are running locally. ([How to run the test?](https://github.com/BurdaMagazinOrg/thunder-distribution/blob/8.x-1.x/docs/development.md#how-to-run-the-tests))
+- [ ] Necessary update hooks are provided.
+- [ ] User roles have correct access for new introduced permission.
+- [ ] Every thunder module has a README.md in its root. Follow [this guidelines](https://www.drupal.org/node/2181737), but we don't need every topic.
+- [ ] Code is covered with well-balanced amount of inline comments.
+
+If you are really awesome, then your feature is covered by additional tests. Well done!

.travis.yml

@@ -0,0 +1,124 @@
+# @file
+# .travis.yml - Drupal for Travis CI Integration
+#
+# Template provided by https://github.com/LionsAd/drupal_ti.
+
+language: php
+
+sudo: false
+
+php:
+  - 7
+  - 5.6
+
+addons:
+  apt:
+    packages:
+      - nodejs
+branches:
+  only:
+    - /^8\.([0-9]+|x)\-[0-9]+\.([0-9]+|x)$/
+
+env:
+  global:
+    # add composer's global bin directory to the path
+    # see: https://github.com/drush-ops/drush#install---composer
+    - PATH="$PATH:$HOME/.composer/vendor/bin"
+
+    # Configuration variables.
+    - DRUPAL_TI_MODULE_NAME="ad_integration"
+    - DRUPAL_TI_SIMPLETEST_GROUP=$DRUPAL_TI_MODULE_NAME
+
+    # Define runners and environment vars to include before and after the
+    # main runners / environment vars.
+    #- DRUPAL_TI_SCRIPT_DIR_BEFORE="./drupal_ti/before"
+    #- DRUPAL_TI_SCRIPT_DIR_AFTER="./drupal_ti/after"
+
+    # The environment to use, supported are: drupal-7, drupal-8
+    - DRUPAL_TI_ENVIRONMENT="drupal-8"
+
+    # The installation profile to use:
+    #- DRUPAL_TI_INSTALL_PROFILE="testing"
+
+    # Drupal specific variables.
+    - DRUPAL_TI_DB="drupal_travis_db"
+    - DRUPAL_TI_DB_URL="mysql://root:@127.0.0.1/drupal_travis_db"
+    # Note: Do not add a trailing slash here.
+    - DRUPAL_TI_WEBSERVER_URL="http://127.0.0.1"
+    - DRUPAL_TI_WEBSERVER_PORT="8080"
+
+    # Simpletest specific commandline arguments, the DRUPAL_TI_SIMPLETEST_GROUP is appended at the end.
+    - DRUPAL_TI_SIMPLETEST_ARGS="--verbose --color --concurrency 4 --url $DRUPAL_TI_WEBSERVER_URL:$DRUPAL_TI_WEBSERVER_PORT"
+
+    # === Behat specific variables.
+    # This is relative to $TRAVIS_BUILD_DIR
+    - DRUPAL_TI_BEHAT_DIR="./tests/behat"
+    # These arguments are passed to the bin/behat command.
+    - DRUPAL_TI_BEHAT_ARGS=""
+    # Specify the filename of the behat.yml with the $DRUPAL_TI_DRUPAL_DIR variables.
+    - DRUPAL_TI_BEHAT_YML="behat.yml.dist"
+    # This is used to setup Xvfb.
+    - DRUPAL_TI_BEHAT_SCREENSIZE_COLOR="1280x1024x16"
+    # The version of selenium that should be used.
+    - DRUPAL_TI_BEHAT_SELENIUM_VERSION="2.48.2"
+    # Set DRUPAL_TI_BEHAT_DRIVER to "selenium" to use "firefox" or "chrome" here.
+    - DRUPAL_TI_BEHAT_DRIVER="phantomjs"
+    - DRUPAL_TI_BEHAT_BROWSER="firefox"
+
+    # PHPUnit specific commandline arguments.
+    - DRUPAL_TI_PHPUNIT_ARGS="--verbose --debug"
+    # Specifying the phpunit-core src/ directory is useful when e.g. a vendor/
+    # directory is present in the module directory, which phpunit would then
+    # try to find tests in. This option is relative to $TRAVIS_BUILD_DIR.
+    #- DRUPAL_TI_PHPUNIT_CORE_SRC_DIRECTORY="./tests/src"
+
+    # Code coverage via coveralls.io
+    - DRUPAL_TI_COVERAGE="satooshi/php-coveralls:0.6.*"
+    # This needs to match your .coveralls.yml file.
+    - DRUPAL_TI_COVERAGE_FILE="build/logs/clover.xml"
+
+    # Debug options
+    #- DRUPAL_TI_DEBUG="-x -v"
+    # Set to "all" to output all files, set to e.g. "xvfb selenium" or "selenium",
+    # etc. to only output those channels.
+    #- DRUPAL_TI_DEBUG_FILE_OUTPUT="selenium xvfb webserver"
+
+  matrix:
+    # [[[ SELECT ANY OR MORE OPTIONS ]]]
+    #- DRUPAL_TI_RUNNERS="phpunit"
+    #- DRUPAL_TI_RUNNERS="simpletest"
+    #- DRUPAL_TI_RUNNERS="behat"
+    #- DRUPAL_TI_RUNNERS="phpunit simpletest behat"
+    # Use phpunit-core to test modules with phpunit with Drupal 8 core.
+    - DRUPAL_TI_RUNNERS="phpunit-core"
+
+mysql:
+  database: drupal_travis_db
+  username: root
+  encoding: utf8
+
+before_install:
+  - composer self-update
+
+  # Code Checks
+  - bash -x -e ./scripts/travis/test-source-code.sh
+
+  # Continue with Drupal Tests
+  - cd ./tests
+  - composer global require "lionsad/drupal_ti:1.*"
+  - drupal-ti before_install
+
+install:
+  - drupal-ti install
+
+before_script:
+  - drupal-ti before_script
+
+script:
+  - drupal-ti script
+
+after_script:
+  - drupal-ti after_script
+
+notifications:
+  email: false

README.md

@@ -4,5 +4,5 @@ This modules aims to be a base module for the integration of different ad provid
 
 ## Usage
 After installing the module visit /admin/config/system/ads to configure it. 
-First select the ad provider, currently ForwardAd Group (fag) and orbyd are possible selections. When selection "fag" you have to provide a container tag url and the used fag ad engine you want to use.
+First select the ad provider, currently ForwardAd Group (fag) and orbyd are possible selections. When selection "fag" you have to provide a container tag url and the fag ad engine you want to use.
 

ad_integration.info.yml

@@ -6,3 +6,4 @@ core: 8.x
 
 dependencies:
   - token
+  - breakpoint_js_settings

config/schema/ad_integration.schema.yml

@@ -1,5 +1,5 @@
 ad_integration.settings:
-  type: mapping
+  type: config_object
   label: 'Ad integration settings'
   mapping:
     ad_provider:

js/adHelper.js

@@ -19,20 +19,20 @@ function getDeviceType() {
   return 'desktop';
 }
 
-function deviceIsMobile() {
+window.deviceIsMobile = function () {
   'use strict';
 
   return (getDeviceType() === 'smartphone');
-}
+};
 
-function deviceIsTablet() {
+window.deviceIsTablet = function () {
   'use strict';
 
   return (getDeviceType() === 'tablet');
-}
+};
 
-function deviceIsDesktop() {
+window.deviceIsDesktop = function () {
   'use strict';
 
   return (getDeviceType() === 'desktop');
-}
+};

js/adIntegration.js

@@ -17,8 +17,10 @@
     }
 
     for (var slot_html_id in settings.AdvertisingSlots) {
-      var tag = settings.AdvertisingSlots[slot_html_id][adsc_device];
-      $('#' + slot_html_id).html('<script>' + TFM.Tag.getAdTag(tag, slot_html_id) + ';</script>');
+      if (settings.AdvertisingSlots.hasOwnProperty(slot_html_id)) {
+        var tag = settings.AdvertisingSlots[slot_html_id][window.adsc_device];
+        $('#' + slot_html_id).html('<script>' + window.TFM.Tag.getAdTag(tag, slot_html_id) + ';</script>');
+      }
     }
   }
 

scripts/travis/test-source-code.sh

@@ -0,0 +1,23 @@
+# remove xdebug to make php execute faster
+phpenv config-rm xdebug.ini
+
+# globally require drupal coder for code tests
+composer global require drupal/coder
+
+# run phpcs
+phpcs --config-set installed_paths ~/.composer/vendor/drupal/coder/coder_sniffer
+phpcs --standard=Drupal --report=summary -p .
+phpcs --standard=DrupalPractice --report=summary -p .
+
+# JS ESLint checking
+mv ~/.nvm ~/.nvm-backup
+git clone https://github.com/creationix/nvm.git ~/.nvm
+(cd ~/.nvm && git checkout `git describe --abbrev=0 --tags`)
+set -x
+source ~/.nvm/nvm.sh
+set +x
+nvm install 4
+npm install -g eslint
+eslint .
+rm -rf ~/.nvm
+mv ~/.nvm-backup ~/.nvm

src/AdIntegration.php

@@ -3,10 +3,6 @@
 namespace Drupal\ad_integration;
 
 use Drupal\Core\Config\ConfigFactoryInterface;
-use Drupal\Core\Entity\EntityTypeManagerInterface;
-use Drupal\Core\Entity\Query\QueryFactory;
-use Drupal\Core\Path\PathMatcher;
-use Drupal\Core\Routing\CurrentRouteMatch;
 use Drupal\Core\Utility\Token;
 
 /**

src/AdIntegrationInterface.php

@@ -44,7 +44,7 @@ public function getAdEngine();
    *   ['entity'] ContentEntityInterface - instance of entity.
    *   ['term'] TermInterface - instance of term.
    *
-   * @return string The ad unit 1.
+   * @return string
    *   The ad unit 1.
    */
   public function getAdUnit1($data = array());

src/AdIntegrationLookup.php

@@ -18,7 +18,7 @@
  */
 class AdIntegrationLookup implements AdIntegrationLookupInterface {
 
-  const supportedEntityParameters = ['node', 'taxonomy_term'];
+  const SUPPORTED_ENTITY_PARAMETERS = ['node', 'taxonomy_term'];
 
   protected $currentRouteMatch;
   protected $config;
@@ -28,8 +28,11 @@ class AdIntegrationLookup implements AdIntegrationLookupInterface {
    * AdIntegrationLookup constructor.
    *
    * @param \Drupal\Core\Routing\RouteMatchInterface $currentRouteMatch
+   *   The route matcher.
    * @param \Drupal\Core\Config\ConfigFactoryInterface $configFactory
+   *   The configuration.
    * @param \Drupal\Core\Entity\EntityTypeManagerInterface $entityTypeManager
+   *   The entity type manager.
    */
   public function __construct(RouteMatchInterface $currentRouteMatch, ConfigFactoryInterface $configFactory, EntityTypeManagerInterface $entityTypeManager) {
     $this->currentRouteMatch = $currentRouteMatch;
@@ -59,7 +62,8 @@ public function byCurrentRoute($name, $termsOnly = FALSE) {
    * @param string $name
    *   The name of the Ad property to look up.
    * @param \Drupal\Core\Routing\RouteMatchInterface $routeMatch
-   *   The route matching the entity (node, term) on which to look up properties.
+   *   The route matching the entity (node, term) on which to look up
+   *   properties.
    * @param bool $termsOnly
    *   If set to TRUE, skips lookup on node settings.
    *
@@ -69,7 +73,7 @@ public function byCurrentRoute($name, $termsOnly = FALSE) {
   public function byRoute($name, RouteMatchInterface $routeMatch, $termsOnly = FALSE) {
     $entity = NULL;
 
-    foreach (static::supportedEntityParameters as $parameter) {
+    foreach (static::SUPPORTED_ENTITY_PARAMETERS as $parameter) {
       if ($entity = $routeMatch->getParameter($parameter)) {
         if (is_numeric($entity)) {
           $entity = Node::load($entity);
@@ -145,9 +149,8 @@ protected function searchEntity($name, ContentEntityInterface $entity, $termsOnl
         }
       }
 
-
       // Check for fallback categories if no ad_integration_setting is found.
-      if (!isset($termOverride) && $fieldType === 'entity_reference' && $fieldDefinition->getSetting('target_type') === 'taxonomy_term') {
+      if ($fieldType === 'entity_reference' && $fieldDefinition->getSetting('target_type') === 'taxonomy_term') {
         $fieldName = $fieldDefinition->getName();
         if ($tid = $entity->$fieldName->target_id) {
           if ($term = Term::load($tid)) {
@@ -167,9 +170,15 @@ protected function searchEntity($name, ContentEntityInterface $entity, $termsOnl
   }
 
   /**
+   * Search for a property value in a term.
+   *
    * @param string $name
+   *   The property name.
    * @param \Drupal\taxonomy\Entity\TermInterface $term
+   *   The term in which to search for the value.
+   *
    * @return string|null
+   *   The found value.
    */
   protected function searchTerm($name, TermInterface $term) {
     foreach ($term->getFieldDefinitions() as $fieldDefinition) {
@@ -192,11 +201,17 @@ protected function searchTerm($name, TermInterface $term) {
   }
 
   /**
-   * @param $name
-   * @param $fieldDefinition
-   * @param $entity
+   * Retrieve overriden setting.
+   *
+   * @param string $name
+   *   The name of the setting.
+   * @param FieldDefinitionInterface $fieldDefinition
+   *   The Ad Settings field.
+   * @param ContentEntityInterface $entity
+   *   The entity to search for overrides in.
    *
    * @return string|null
+   *   The overridden value of null if none is found.
    */
   protected function getOverriddenAdSetting($name, FieldDefinitionInterface $fieldDefinition, ContentEntityInterface $entity) {
     if ($fieldDefinition->getType() === 'ad_integration_settings') {
@@ -220,4 +235,5 @@ protected function getOverriddenAdSetting($name, FieldDefinitionInterface $field
   private function defaults($name) {
     return $this->config->get($name . '_default');
   }
-}
+
+}

src/AdIntegrationLookupInterface.php

@@ -32,7 +32,8 @@ public function byCurrentRoute($name, $termsOnly = FALSE);
    * @param string $name
    *   The name of the Ad property to look up.
    * @param \Drupal\Core\Routing\RouteMatchInterface $routeMatch
-   *   The route matching the entity (node, term) on which to look up properties.
+   *   The route matching the entity (node, term) on which to look up
+   *   properties.
    * @param bool $termsOnly
    *   If set to TRUE, skips lookup on node settings.
    *
@@ -68,4 +69,5 @@ public function byEntity($name, ContentEntityInterface $entity, $termsOnly = FAL
    *   The property value.
    */
   public function byTerm($name, TermInterface $term);
-}
+
+}