git commit -m 'Issue #2875538 by vytch, rli, nikunjkotecha, gbyte.co:...

git commit -m 'Issue #2875538 by vytch, rli, nikunjkotecha, gbyte.co: Changefreq implementation missing'

README.md

@@ -56,10 +56,10 @@ be configured on a per-bundle basis, e.g.
 
 When including an entity type or bundle into the sitemap, the priority setting
 can be set which will set the 'priority' parameter for all entities of that
-type. See https://en.wikipedia.org/wiki/Sitemaps to learn more about this
-parameter.
+type. Same goes for the 'changefreq' setting. See https://en.wikipedia.org/wiki/Sitemaps to learn more about these
+parameters.
 
-Inclusion and priority settings of bundles can be overridden on a per-entity
+Inclusion settings of bundles can be overridden on a per-entity
 basis. Just head over to a bundle instance edit form (e.g. node/1/edit) to
 override its sitemap settings.
 

config/install/simple_sitemap.custom.yml

@@ -2,3 +2,4 @@ links:
   -
     path: '/'
     priority: '1.0'
+    changefreq: 'daily'

config/schema/simple_sitemap.schema.yml

@@ -38,6 +38,9 @@ simple_sitemap.bundle_settings.*.*:
     priority:
       label: 'Priority'
       type: string
+    changefreq:
+      label: 'Change frequency'
+      type: string
 
 simple_sitemap.custom:
   label: 'Custom links'
@@ -55,3 +58,6 @@ simple_sitemap.custom:
           priority:
             label: 'Priority'
             type: string
+          changefreq:
+            label: 'Change frequency'
+            type: string

js/simple_sitemap.fieldsetSummaries.js

@@ -13,6 +13,7 @@
         if ($(context).find('#edit-simple-sitemap-index-content-1').is(':checked')) {
           vals.push(Drupal.t('Included in sitemap'));
           vals.push(Drupal.t('Priority') + ' ' + $('#edit-simple-sitemap-priority option:selected', context).text());
+          vals.push(Drupal.t('Change frequency') + ' ' + $('#edit-simple-sitemap-changefreq option:selected', context).text());
         }
         else {
           vals.push(Drupal.t('Excluded from sitemap'));

js/simple_sitemap.form.js

@@ -12,28 +12,32 @@
       // On load: Hide the 'Regenerate sitemap' field to only display it if settings have changed.
       $('.form-item-simple-sitemap-regenerate-now').hide();
 
-      // On load: Show or hide 'priority' setting dependant on 'enabled' setting.
+      // On load: Show or hide settings dependant on 'enabled' setting.
       if ($('#edit-simple-sitemap-index-content-1').is(':checked')) {
         $('.form-item-simple-sitemap-priority').show();
+        $('.form-item-simple-sitemap-changefreq').show();
       }
       else {
         $('.form-item-simple-sitemap-priority').hide();
+        $('.form-item-simple-sitemap-changefreq').hide();
       }
 
-      // On change: Show or hide 'priority' setting dependant on 'enabled' setting.
+      // On change: Show or hide settings dependant on 'enabled' setting.
       $("#edit-simple-sitemap-index-content").change(function() {
         if ($('#edit-simple-sitemap-index-content-1').is(':checked')) {
           $('.form-item-simple-sitemap-priority').show();
+          $('.form-item-simple-sitemap-changefreq').show();
         }
         else {
           $('.form-item-simple-sitemap-priority').hide();
+          $('.form-item-simple-sitemap-changefreq').hide();
         }
         // Show 'Regenerate sitemap' field if 'enabled' setting has changed.
         $('.form-item-simple-sitemap-regenerate-now').show();
       });
 
-      // Show 'Regenerate sitemap' field if 'priority' setting has changed.
-      $("#edit-simple-sitemap-priority").change(function() {
+      // Show 'Regenerate sitemap' field if settings have changed.
+      $("#edit-simple-sitemap-priority, #edit-simple-sitemap-changefreq").change(function() {
         $('.form-item-simple-sitemap-regenerate-now').show();
       });
     }

js/simple_sitemap.sitemapEntities.js

@@ -34,31 +34,36 @@
         });
       });
 
-      // Show priority settings if atomic entity enabled on form load.
+      // Show settings if atomic entity enabled on form load.
       $.each(atomicEntities, function(index, value) {
         var enabledId = '#edit-' + value + '-enabled';
         var priorityId = '.form-item-' + value + '-simple-sitemap-priority';
+        var changefreqId = '.form-item-' + value + '-simple-sitemap-changefreq';
 
-        // On load: Show or hide priority setting dependent on 'enabled' checkbox.
+        // On load: Show or hide settings dependent on 'enabled' checkbox.
         if ($(enabledId).is(':checked')) {
           $(priorityId).show();
+          $(changefreqId).show();
         }
         else {
           $(priorityId).hide();
+          $(changefreqId).hide();
         }
 
-        // On change: Show or hide priority setting dependent on 'enabled' checkbox.
+        // On change: Show or hide settings dependent on 'enabled' checkbox.
         $(enabledId).change(function() {
           if ($(enabledId).is(':checked')) {
             $(priorityId).show();
+            $(changefreqId).show();
           }
           else {
             $(priorityId).hide();
+            $(changefreqId).hide();
           }
         });
 
-        // Show 'Regenerate sitemap' field if 'priority' setting has changed.
-        $(priorityId).change(function() {
+        // Show 'Regenerate sitemap' field if settings have changed.
+        $(priorityId, changefreqId).change(function() {
           $('.form-item-simple-sitemap-regenerate-now').show();
         });
       });

simple_sitemap.module

@@ -108,16 +108,19 @@ function simple_sitemap_entity_form_submit($form, FormStateInterface &$form_stat
      */
     $generator = \Drupal::service('simple_sitemap.generator');
 
+    $settings = [
+      'index' => $values['simple_sitemap_index_content'],
+      'priority' => $values['simple_sitemap_priority'],
+      'changefreq' => $values['simple_sitemap_changefreq'],
+    ];
+
     switch ($f->getEntityCategory()) {
 
       case 'bundle':
         $generator->setBundleSettings(
           $f->getEntityTypeId(),
           !empty($f->getBundleName()) ? $f->getBundleName() : $f->getFormEntityId(),
-          [
-            'index' => $values['simple_sitemap_index_content'],
-            'priority' => $values['simple_sitemap_priority']
-          ]
+          $settings
         );
         break;
 
@@ -125,10 +128,7 @@ function simple_sitemap_entity_form_submit($form, FormStateInterface &$form_stat
         $generator->setEntityInstanceSettings(
           $f->getEntityTypeId(),
           !empty($f->getInstanceId()) ? $f->getInstanceId() : $f->getFormEntityId(),
-          [
-            'index' => $values['simple_sitemap_index_content'],
-            'priority' => $values['simple_sitemap_priority']
-          ]
+          $settings
         );
         break;
     }

simple_sitemap.permissions.yml

 administer sitemap settings:
   title: 'Administer sitemap settings'
-  description: 'Administer Simple XML sitemap settings, alter inclusion and priority of content and generate the sitemap on demand.'
+  description: 'Administer Simple XML sitemap settings, alter inclusion, priority and changefreq of content and generate the sitemap on demand.'
   restrict access: false

src/Batch/CustomUrlGenerator.php

@@ -46,6 +46,7 @@ class CustomUrlGenerator extends UrlGeneratorBase implements UrlGeneratorInterfa
         'lastmod' => method_exists($entity, 'getChangedTime')
           ? date_iso8601($entity->getChangedTime()) : NULL,
         'priority' => isset($custom_path['priority']) ? $custom_path['priority'] : NULL,
+        'changefreq' => !empty($custom_path['changefreq']) ? $custom_path['changefreq'] : NULL,
       ];
       if (NULL !== $entity) {
         $path_data['entity_info'] = [

src/Batch/EntityUrlGenerator.php

@@ -61,7 +61,8 @@ class EntityUrlGenerator extends UrlGeneratorBase implements UrlGeneratorInterfa
         ],
         'lastmod' => method_exists($entity, 'getChangedTime')
           ? date_iso8601($entity->getChangedTime()) : NULL,
-        'priority' => $entity_settings['priority'],
+        'priority' => isset($entity_settings['priority']) ? $entity_settings['priority'] : NULL,
+        'changefreq' => !empty($entity_settings['changefreq']) ? $entity_settings['changefreq'] : NULL,
       ];
       $this->addUrlVariants($url_object, $path_data, $entity);
     }

src/Form/FormHelper.php

@@ -66,9 +66,20 @@ class FormHelper {
     'register',
   ];
 
+  protected static $changefreqSelectValues = [
+    'always',
+    'hourly',
+    'daily',
+    'weekly',
+    'monthly',
+    'yearly',
+    'never',
+  ];
+
   protected static $valuesToCheck = [
     'simple_sitemap_index_content',
     'simple_sitemap_priority',
+    'simple_sitemap_changefreq',
     'simple_sitemap_regenerate_now',
   ];
 
@@ -225,6 +236,7 @@ class FormHelper {
     }
     $index = isset($settings['index']) ? $settings['index'] : 0;
     $priority = isset($settings['priority']) ? $settings['priority'] : self::PRIORITY_DEFAULT;
+    $changefreq = !empty($settings['changefreq']) ? $settings['changefreq'] : '';
     $bundle_name = !empty($this->getBundleName()) ? $this->getBundleName() : $this->t('undefined');
 
     if (!$multiple) {
@@ -243,10 +255,13 @@ class FormHelper {
 
     if ($this->getEntityCategory() == 'instance') {
       $priority_description = $this->t('The priority this @bundle entity will have in the eyes of search engine bots.', ['@bundle' => $bundle_name]);
+      $changefreq_description = $this->t('The frequency with which this @bundle entity changes. Search engine bots may take this as an indication of how often to index it.', ['@bundle' => $bundle_name]);
     }
     else {
       $priority_description = $this->t('The priority entities of this type will have in the eyes of search engine bots.');
+      $changefreq_description = $this->t('The frequency with which entities of this type change. Search engine bots may take this as an indication of how often to index them.');
     }
+
     $form_fragment[$prefix . 'simple_sitemap_priority'] = [
       '#type' => 'select',
       '#title' => $this->t('Priority'),
@@ -254,9 +269,23 @@ class FormHelper {
       '#default_value' => $priority,
       '#options' => $this->getPrioritySelectValues(),
     ];
+
     if ($this->getEntityCategory() == 'instance' && isset($bundle_settings['priority'])) {
       $form_fragment[$prefix . 'simple_sitemap_priority']['#options'][$this->formatPriority($bundle_settings['priority'])] .= ' (' . $this->t('Default') . ')';
     }
+
+    $form_fragment[$prefix . 'simple_sitemap_changefreq'] = [
+      '#type' => 'select',
+      '#title' => $this->t('Change frequency'),
+      '#description' => $changefreq_description,
+      '#default_value' => $changefreq,
+      '#options' => $this->getChangefreqSelectValues(),
+    ];
+
+    if ($this->getEntityCategory() == 'instance' && isset($bundle_settings['changefreq'])) {
+      $form_fragment[$prefix . 'simple_sitemap_changefreq']['#options'][$bundle_settings['changefreq']] .= ' (' . $this->t('Default') . ')';
+    }
+
     return $this;
   }
 
@@ -377,6 +406,7 @@ class FormHelper {
    * Gets the values needed to display the priority dropdown setting.
    *
    * @return array
+   *   Select options.
    */
   public function getPrioritySelectValues() {
     $options = [];
@@ -387,6 +417,27 @@ class FormHelper {
     return $options;
   }
 
+  /**
+   * Gets the values needed to display the changefreq dropdown setting.
+   *
+   * @return array
+   *   Select options.
+   */
+  public function getChangefreqSelectValues() {
+    $options = ['' => t('- Not specified -')];
+    foreach (self::$changefreqSelectValues as $setting) {
+      $options[$setting] = t($setting);
+    }
+    return $options;
+  }
+
+  /**
+   * @return array
+   */
+  public static function getChangefreqOptions() {
+    return self::$changefreqSelectValues;
+  }
+
   /**
    * @param string $priority
    * @return string
@@ -402,4 +453,12 @@ class FormHelper {
   public static function isValidPriority($priority) {
     return is_numeric($priority) && $priority >= 0 && $priority <= 1;
   }
+
+  /**
+   * @param string $changefreq
+   * @return bool
+   */
+  public static function isValidChangefreq($changefreq) {
+    return in_array($changefreq, self::$changefreqSelectValues);
+  }
 }

src/Form/SimplesitemapCustomLinksForm.php

@@ -33,7 +33,7 @@ class SimplesitemapCustomLinksForm extends SimplesitemapFormBase {
       '#type' => 'textarea',
       '#title' => $this->t('Relative Drupal paths'),
       '#default_value' => $this->customLinksToString($this->generator->getCustomLinks()),
-      '#description' => $this->t("Please specify drupal internal (relative) paths, one per line. Do not forget to prepend the paths with a '/'. You can optionally add a priority (0.0 - 1.0) by appending it to the path after a space. The home page with the highest priority would be <em>/ 1.0</em>, the contact page with the default priority would be <em>/contact 0.5</em>."),
+      '#description' => $this->t("Please specify drupal internal (relative) paths, one per line. Do not forget to prepend the paths with a '/'.<br/>Optionally link priority <em>(0.0 - 1.0)</em> can be added by appending it after a space.<br/> Optionally link change frequency <em>(always / hourly / daily / weekly / monthly / yearly / never)</em> can be added by appending it after a space.<br/><br/><strong>Examples:</strong><br/><em>/ 1.0 daily</em> -> home page with the highest priority and daily change frequency<br/><em>/contact</em> -> contact page with the default priority and no change frequency information."),
     ];
 
     $this->formHelper->displayRegenerateNow($form['simple_sitemap_custom']);
@@ -46,7 +46,13 @@ class SimplesitemapCustomLinksForm extends SimplesitemapFormBase {
    */
   public function validateForm(array &$form, FormStateInterface $form_state) {
     foreach ($this->stringToCustomLinks($form_state->getValue('custom_links')) as $i => $link_config) {
-      $placeholders = ['@line' => ++$i, '@path' => $link_config['path'], '@priority' => isset($link_config['priority']) ? $link_config['priority'] : ''];
+      $placeholders = [
+        '@line' => ++$i,
+        '@path' => $link_config['path'],
+        '@priority' => isset($link_config['priority']) ? $link_config['priority'] : '',
+        '@changefreq' => isset($link_config['changefreq']) ? $link_config['changefreq'] : '',
+        '@changefreq_options' => implode(', ', FormHelper::getChangefreqOptions()),
+      ];
 
       // Checking if internal path exists.
       if (!$this->pathValidator->isValid($link_config['path'])
@@ -65,6 +71,11 @@ class SimplesitemapCustomLinksForm extends SimplesitemapFormBase {
       if (isset($link_config['priority']) && !FormHelper::isValidPriority($link_config['priority'])) {
         $form_state->setErrorByName('', $this->t("<strong>Line @line</strong>: The priority setting <em>@priority</em> for path <em>@path</em> is incorrect. Set the priority from 0.0 to 1.0.", $placeholders));
       }
+
+      // Making sure changefreq is formatted correctly.
+      if (isset($link_config['changefreq']) && !FormHelper::isValidChangefreq($link_config['changefreq'])) {
+        $form_state->setErrorByName('', $this->t("<strong>Line @line</strong>: The changefreq setting <em>@changefreq</em> for path <em>@path</em> is incorrect. The following are the correct values: <em>@changefreq_options</em>.", $placeholders));
+      }
     }
   }
 
@@ -99,10 +110,26 @@ class SimplesitemapCustomLinksForm extends SimplesitemapFormBase {
 
     $custom_links = [];
     foreach ($custom_links_string_lines as $i => &$line) {
-      $link_settings = explode(' ', $line, 2);
+      $link_settings = explode(' ', $line);
       $custom_links[$i]['path'] = $link_settings[0];
-      if (isset($link_settings[1]) && $link_settings[1] != '') {
+
+      // If two arguments are provided for a link, assume the first to be
+      // priority, the second to be changefreq.
+      if (!empty($link_settings[1]) && !empty($link_settings[2])) {
         $custom_links[$i]['priority'] = $link_settings[1];
+        $custom_links[$i]['changefreq'] = $link_settings[2];
+      }
+      else {
+        // If one argument is provided for a link, guess if it is priority or
+        // changefreq.
+        if (!empty($link_settings[1])) {
+          if (is_numeric($link_settings[1])) {
+            $custom_links[$i]['priority'] = $link_settings[1];
+          }
+          else {
+            $custom_links[$i]['changefreq'] = $link_settings[1];
+          }
+        }
       }
     }
     return $custom_links;
@@ -115,9 +142,13 @@ class SimplesitemapCustomLinksForm extends SimplesitemapFormBase {
   protected function customLinksToString(array $links) {
     $setting_string = '';
     foreach ($links as $custom_link) {
+      $setting_string .= $custom_link['path'];
       $setting_string .= isset($custom_link['priority'])
-        ? $custom_link['path'] . ' ' . $this->formHelper->formatPriority($custom_link['priority'])
-        : $custom_link['path'];
+        ? ' ' . $this->formHelper->formatPriority($custom_link['priority'])
+        : '';
+      $setting_string .= isset($custom_link['changefreq'])
+        ? ' ' . $custom_link['changefreq']
+        : '';
       $setting_string .= "\r\n";
     }
     return $setting_string;

src/Form/SimplesitemapEntitiesForm.php

@@ -96,6 +96,7 @@ class SimplesitemapEntitiesForm extends SimplesitemapFormBase {
             $this->generator->setBundleSettings($entity_type_id, $entity_type_id, [
               'index' => TRUE,
               'priority' => $values[$entity_type_id . '_simple_sitemap_priority'],
+              'changefreq' => $values[$entity_type_id . '_simple_sitemap_changefreq'],
             ]);
           }
         }

src/Simplesitemap.php

@@ -49,8 +49,13 @@ class Simplesitemap {
    * @var array
    */
   protected static $allowed_link_settings = [
-    'entity' => ['index', 'priority'],
-    'custom' => ['priority'],
+    'entity' => ['index', 'priority', 'changefreq'],
+    'custom' => ['priority', 'changefreq'],
+  ];
+
+  protected static $link_setting_defaults = [
+    'index' => 0,
+    'priority' => 0.5,
   ];
 
   /**
@@ -284,7 +289,7 @@ class Simplesitemap {
    *   Name of the bundle. NULL if entity type has no bundles.
    * @param array $settings
    *   An array of sitemap settings for this bundle/entity type.
-   *   Example: ['index' => TRUE, 'priority' => 0.5].
+   *   Example: ['index' => TRUE, 'priority' => 0.5, 'changefreq' => 'never'].
    *
    * @return $this
    */
@@ -367,21 +372,40 @@ class Simplesitemap {
   public function getBundleSettings($entity_type_id = NULL, $bundle_name = NULL) {
     if (NULL !== $entity_type_id) {
       $bundle_name = empty($bundle_name) ? $entity_type_id : $bundle_name;
-      $settings = $this->configFactory
+      $bundle_settings = $this->configFactory
         ->get("simple_sitemap.bundle_settings.$entity_type_id.$bundle_name")
         ->get();
-      $bundle_settings = !empty($settings) ? $settings : FALSE;
+      return !empty($bundle_settings)
+        ? $this->supplementDefaultSettings('entity', $bundle_settings)
+        : FALSE;
     }
     else {
       $config_names = $this->configFactory->listAll("simple_sitemap.bundle_settings.");
-      $bundle_settings = [];
+      $all_settings = [];
       foreach($config_names as $config_name) {
         $config_name_parts = explode('.', $config_name);
-        $bundle_settings[$config_name_parts[2]][$config_name_parts[3]]
-          = $this->configFactory->get($config_name)->get();
+        $all_settings[$config_name_parts[2]][$config_name_parts[3]] = $this->supplementDefaultSettings(
+          'entity',
+          $this->configFactory->get($config_name)->get()
+        );
+      }
+      return $all_settings;
+    }
+  }
+
+  /**
+   * @param string $type
+   * @param array $settings
+   * @return array
+   */
+  protected function supplementDefaultSettings($type, $settings) {
+    foreach(self::$allowed_link_settings[$type] as $allowed_link_setting) {
+      if (!isset($settings[$allowed_link_setting])
+        && isset($link_setting_defaults[$allowed_link_setting])) {
+        $settings[$allowed_link_setting] = self::$link_setting_defaults[$allowed_link_setting];
       }
     }
-    return $bundle_settings;
+    return $settings;
   }
 
   /**
@@ -453,7 +477,7 @@ class Simplesitemap {
       ->fetchField();
 
     if (!empty($results)) {
-      return unserialize($results);
+      return $this->supplementDefaultSettings('entity', unserialize($results));
     }
     else {
       $entity = $this->entityTypeManager->getStorage($entity_type_id)
@@ -517,6 +541,8 @@ class Simplesitemap {
    * @param array $settings
    *
    * @return $this
+   *
+   * @todo Validate $settings and throw exceptions
    */
   public function addCustomLink($path, $settings) {
     if (!$this->pathValidator->isValid($path)) {
@@ -536,34 +562,12 @@ class Simplesitemap {
       }
     }
     $link_key = isset($link_key) ? $link_key : count($custom_links);
-    $custom_links[$link_key]['path'] = $path;
-    $this->addLinkSettings('custom', $settings, $custom_links[$link_key]); //todo: dirty
+    $custom_links[$link_key] = ['path' => $path] + $settings;
     $this->configFactory->getEditable("simple_sitemap.custom")
       ->set('links', $custom_links)->save();
     return $this;
   }
 
-  /**
-   *
-   */
-  protected function addLinkSettings($type, $settings, &$target) {
-    foreach ($settings as $setting_key => $setting) {
-      if (in_array($setting_key, self::$allowed_link_settings[$type])) {
-        switch ($setting_key) {
-          case 'priority':
-            if (!FormHelper::isValidPriority($setting)) {
-              // todo: log error.
-              continue;
-            }
-            break;
-
-          // todo: add index check.
-        }
-        $target[$setting_key] = $setting;
-      }
-    }
-  }
-
   /**
    * Returns an array of custom paths and their sitemap settings.
    *
@@ -585,8 +589,8 @@ class Simplesitemap {
    */
   public function getCustomLink($path) {
     foreach ($this->getCustomLinks() as $key => $link) {
-      if ($link['path'] == $path) {
-        return $link;
+      if ($link['path'] === $path) {
+        return $this->supplementDefaultSettings('custom', $link);
       }
     }
     return FALSE;
@@ -602,7 +606,7 @@ class Simplesitemap {
   public function removeCustomLink($path) {
     $custom_links = $this->getCustomLinks();
     foreach ($custom_links as $key => $link) {
-      if ($link['path'] == $path) {
+      if ($link['path'] === $path) {
         unset($custom_links[$key]);
         $custom_links = array_values($custom_links);
         $this->configFactory->getEditable("simple_sitemap.custom")

src/SitemapGenerator.php

@@ -149,6 +149,7 @@ class SitemapGenerator {
     foreach ($this->generator->getCustomLinks() as $i => $custom_path) {
       $paths[$i]['path'] = $custom_path['path'];
       $paths[$i]['priority'] = isset($custom_path['priority']) ? $custom_path['priority'] : NULL;
+      $paths[$i]['changefreq'] = isset($custom_path['changefreq']) ? $custom_path['changefreq'] : NULL;
       // todo: implement lastmod.
       $paths[$i]['lastmod'] = NULL;
     }
@@ -292,7 +293,10 @@ class SitemapGenerator {
         $writer->writeElement('lastmod', $link['lastmod']);
       }
 
-      //todo: Implement changefreq here.
+      // Add changefreq if any.
+      if (isset($link['changefreq'])) {
+        $writer->writeElement('changefreq', $link['changefreq']);
+      }
 
       // Add priority if any.
       if (isset($link['priority'])) {