Limit slug length

app/assets/javascripts/admin/commons/slug_input.js

@@ -34,17 +34,27 @@ window.osuny.slugInput = {
 
     onSourceChange: function (sourceInputs, slugInput) {
         'use strict';
-        var values = [],
+        var maxLength = slugInput.maxLength,
+            generatedSlug,
+            values = [],
             value,
             i;
+
         for (i = 0; i < sourceInputs.length; i += 1) {
             value = sourceInputs[i].value.trim();
             if (value !== '') {
                 values.push(value);
             }
         }
 
-        slugInput.value = window.slug(values.join(' '));
+        generatedSlug = window.slug(values.join(' '));
+        if (generatedSlug.length > maxLength) {
+            generatedSlug = generatedSlug.slice(0, maxLength);
+        }
+        if (generatedSlug[generatedSlug.length - 1] === '-') {
+            generatedSlug = generatedSlug.slice(0, -1);
+        }
+        slugInput.value = generatedSlug;
     },
 
     invoke: function () {

app/controllers/admin/education/diplomas_controller.rb

@@ -64,6 +64,6 @@ def breadcrumb
 
   def diploma_params
     params.require(:education_diploma)
-          .permit(:name, :short_name, :summary, :level, :ects, :duration)
+          .permit(:name, :slug, :short_name, :summary, :level, :ects, :duration)
   end
 end

app/controllers/admin/research/publications_controller.rb

@@ -61,6 +61,6 @@ def breadcrumb
 
   def publication_params
     params.require(:research_publication)
-          .permit(:title, :publication_date, :abstract, :authors_list, :doi, :ref, :journal_title, :url, :open_access, :citation_full, researcher_ids: [])
+          .permit(:title, :slug, :publication_date, :abstract, :authors_list, :doi, :ref, :journal_title, :url, :open_access, :citation_full, researcher_ids: [])
   end
 end

app/models/communication/website/agenda/category.rb

@@ -39,15 +39,16 @@ class Communication::Website::Agenda::Category < ApplicationRecord
   include AsDirectObject
   include Contentful
   include Sanitizable
-  include Sluggable
+  include Sluggable # We override slug_unavailable? method
+  include Pathable # Included after Sluggable to make sure slug is correct before anything
   include WithBlobs
   include WithFeaturedImage
   include WithMenuItemTarget
   include WithPermalink
   include WithPosition
   include WithTranslations
   include WithUniversity
-  
+
   belongs_to              :parent,
                           class_name: 'Communication::Website::Agenda::Category',
                           optional: true

app/models/communication/website/menu.rb

@@ -28,6 +28,8 @@
 #  fk_rails_dcc7198fc5  (communication_website_id => communication_websites.id)
 #
 class Communication::Website::Menu < ApplicationRecord
+  IDENTIFIER_MAX_LENGTH = 100
+
   include AsDirectObject
   include Sanitizable
   include WithAutomatism
@@ -37,7 +39,8 @@ class Communication::Website::Menu < ApplicationRecord
   has_many :items, class_name: 'Communication::Website::Menu::Item', dependent: :destroy
 
   validates :title, :identifier, presence: true
-  validates :identifier, uniqueness: { scope: [:communication_website_id, :language_id] }
+  validates :identifier,  length: { maximum: IDENTIFIER_MAX_LENGTH },
+                          uniqueness: { scope: [:communication_website_id, :language_id] }
 
   scope :ordered, -> { order(created_at: :asc) }
 

app/models/communication/website/page.rb

@@ -49,10 +49,12 @@
 #
 
 class Communication::Website::Page < ApplicationRecord
+  # FIXME: Remove legacy column from db
   self.ignored_columns = %w(path)
 
   include AsDirectObject
   include Contentful
+  include Sluggable # We override slug_unavailable? method (and set_slug and skip_slug_validation? in Page::Home)
   include Sanitizable
   include WithAccessibility
   include WithAutomaticMenus
@@ -66,7 +68,7 @@ class Communication::Website::Page < ApplicationRecord
   include WithPermalink
   include WithTranslations
   include WithTree
-  include WithPath # WithPath overwrites the git_path method defined in WithWebsites
+  include WithPath # Must be included after Sluggable. WithPath overwrites the git_path method defined in WithWebsites
   include WithUniversity
 
   has_summernote :text # TODO: Remove text attribute
@@ -155,6 +157,13 @@ def self.direct_connection_permitted_about_type
 
   protected
 
+  def slug_unavailable?(slug)
+    self.class.unscoped
+              .where(communication_website_id: self.communication_website_id, language_id: language_id, slug: slug)
+              .where.not(id: self.id)
+              .exists?
+  end
+
   def check_accessibility
     accessibility_merge_array blocks
   end

app/models/communication/website/page/home.rb

@@ -22,7 +22,7 @@ def set_slug
     self.slug = ''
   end
 
-  def validate_slug
+  def skip_slug_validation?
     true
   end
 

app/models/communication/website/page/with_path.rb

@@ -1,11 +1,6 @@
 module Communication::Website::Page::WithPath
   extend ActiveSupport::Concern
 
-  included do
-    before_validation :set_slug
-    validate :validate_slug
-  end
-
   def path
     path = ''
     if website.languages.many?
@@ -15,6 +10,7 @@ def path
     path.gsub(/\/+/, '/')
   end
 
+  # FIXME @arnaud : Should it be moved to Sluggable? To discuss
   def slug_with_ancestors
     (ancestors.map(&:slug) << slug).reject(&:blank?).join('/')
   end
@@ -48,39 +44,4 @@ def git_path_prefix
     @git_path_prefix ||= git_path_content_prefix(website)
   end
 
-  def set_slug
-    self.slug = to_s.parameterize if self.slug.blank?
-    current_slug = self.slug
-    n = 0
-    while slug_unavailable?(self.slug)
-      n += 1
-      self.slug = [current_slug, n].join('-')
-    end
-  end
-
-  def slug_unavailable?(slug)
-    self.class.unscoped
-              .where(communication_website_id: self.communication_website_id, language_id: language_id, slug: slug)
-              .where.not(id: self.id)
-              .exists?
-  end
-
-  def validate_slug
-    slug_must_be_present
-    slug_must_be_unique
-    slug_must_have_proper_format
-  end
-
-  def slug_must_be_present
-    errors.add(:slug, :absent) if slug.blank?
-  end
-
-  def slug_must_be_unique
-    errors.add(:slug, :taken) if slug_unavailable?(slug)
-  end
-
-  def slug_must_have_proper_format
-    errors.add(:slug, I18n.t('slug_error')) unless /\A[a-z0-9\-]+\z/.match?(slug)
-  end
-
 end

app/models/communication/website/post/category.rb

@@ -45,6 +45,7 @@ class Communication::Website::Post::Category < ApplicationRecord
   include Contentful
   include Sanitizable
   include Sluggable # We override slug_unavailable? method
+  include Pathable # Included after Sluggable to make sure slug is correct before anything
   include WithBlobs
   include WithFeaturedImage
   include WithMenuItemTarget
@@ -72,8 +73,6 @@ class Communication::Website::Post::Category < ApplicationRecord
 
   validates :name, presence: true
 
-  after_save :update_children_paths, if: :saved_change_to_path?
-
   def to_s
     "#{name}"
   end
@@ -101,13 +100,6 @@ def references
     abouts_with_post_block
   end
 
-  def update_children_paths
-    children.each do |child|
-      child.update_column :path, child.generated_path
-      child.update_children_paths
-    end
-  end
-
   def siblings
     self.class.unscoped.where(parent: parent, university: university, website: website).where.not(id: id)
   end

app/models/concerns/pathable.rb

@@ -0,0 +1,27 @@
+module Pathable
+  extend ActiveSupport::Concern
+
+  included do
+    before_validation :make_path
+    after_save :update_children_paths, if: :saved_change_to_path?
+
+    def generated_path
+      "#{parent.nil? ? '/' : parent.path}#{slug}/".gsub(/\/+/, '/')
+    end
+
+    def update_children_paths
+      children.each do |child|
+        child.update_column :path, child.generated_path
+        child.update_children_paths
+      end
+    end
+
+    protected
+
+    def make_path
+      return unless respond_to?(:path) && respond_to?(:parent)
+      self.path = generated_path
+    end
+
+  end
+end

app/models/concerns/sluggable.rb

@@ -1,47 +1,54 @@
 module Sluggable
   extend ActiveSupport::Concern
 
+  # Filenames uses 255 bytes so should have 255 characters max. To be safe, we limit to 100.
+  # Source: https://askubuntu.com/questions/166764/how-long-can-file-names-be/166767#166767
+  SLUG_MAX_LENGTH = 100
+
   included do
-    validates :slug, presence: true
-    validate :slug_must_be_unique
-    validates :slug, format: { with: /\A[a-z0-9\-]+\z/, message: I18n.t('slug_error') }
-
-    before_validation :check_slug, :make_path
-
-    def check_slug
-      self.slug = to_s.parameterize if self.slug.blank?
-      current_slug = self.slug
-      n = 0
-      while slug_unavailable?(self.slug)
-        n += 1
-        self.slug = [current_slug, n].join('-')
-      end
-    end
+    validates :slug, presence: true, unless: :skip_slug_validation?
+    validate :slug_must_be_unique, unless: :skip_slug_validation?
+    validates :slug, format: { with: /\A[a-z0-9\-]+\z/, message: I18n.t('slug_error') }, unless: :skip_slug_validation?
+
+    before_validation :set_slug
+  end
 
-    def generated_path
-      "#{parent.nil? ? '/' : parent.path}#{slug}/".gsub(/\/+/, '/')
+  def set_slug
+    self.slug = to_s.parameterize if self.slug.blank?
+    adjust_slug_length
+    current_slug = self.slug
+    n = 0
+    while slug_unavailable?(self.slug)
+      n += 1
+      self.slug = [current_slug, n].join('-')
     end
+  end
+
+  protected
 
-    protected
+  def adjust_slug_length
+    return unless self.slug.length > SLUG_MAX_LENGTH
+    # "my-very-long-and-detailed-first-blog-post" => "my-very-long-and-detailed-first-"
+    self.slug = self.slug[0, SLUG_MAX_LENGTH]
+    # "my-very-long-and-detailed-first-" => "my-very-long-and-detailed-first"
+    self.slug.chop! if self.slug.ends_with?('-')
+  end
 
-    def slug_unavailable?(slug)
-      existence_params = { university_id: self.university_id, slug: slug }
-      existence_params[:language_id] = self.language_id if respond_to?(:language_id)
+  def slug_unavailable?(slug)
+    existence_params = { university_id: self.university_id, slug: slug }
+    existence_params[:language_id] = self.language_id if respond_to?(:language_id)
 
-      self.class.unscoped
-                .where(**existence_params)
-                .where.not(id: self.id)
-                .exists?
-    end
+    self.class.unscoped
+              .where(**existence_params)
+              .where.not(id: self.id)
+              .exists?
+  end
 
-    # FIXME `respond_to?(:parent)` sert à quoi ?
-    def make_path
-      return unless respond_to?(:path) && respond_to?(:parent)
-      self.path = generated_path
-    end
+  def slug_must_be_unique
+    errors.add(:slug, :taken) if slug_unavailable?(slug)
+  end
 
-    def slug_must_be_unique
-      errors.add(:slug, :taken) if slug_unavailable?(slug)
-    end
+  def skip_slug_validation?
+    false
   end
 end

app/models/education/program.rb

@@ -60,6 +60,7 @@ class Education::Program < ApplicationRecord
   include Contentful
   include Sanitizable
   include Sluggable
+  include Pathable # Included after Sluggable to make sure slug is correct before anything
   include WebsitesLinkable
   include WithAccessibility
   include WithAlumni
@@ -111,8 +112,6 @@ class Education::Program < ApplicationRecord
   validates_presence_of :name
   validates :downloadable_summary, size: { less_than: 50.megabytes }
 
-  after_save :update_children_paths, if: :saved_change_to_path?
-
   scope :published, -> { where(published: true) }
   scope :ordered_by_name, -> { order(:name) }
   scope :for_search_term, -> (term) {
@@ -168,13 +167,6 @@ def references
     references
   end
 
-  def update_children_paths
-    children.each do |child|
-      child.update_column :path, child.generated_path
-      child.update_children_paths
-    end
-  end
-
   #####################
   # WebsitesLinkable methods #
   #####################

app/views/admin/administration/locations/_form.html.erb

@@ -23,12 +23,9 @@
     </div>
     <div class="col-md-4">
       <%= osuny_panel t('metadata') do %>
-        <%= f.input :slug,
-                    as: :string,
-                    input_html: location.persisted? ? {} : {
-                      class: 'js-slug-input',
-                      data: { source: '#administration_location_name' }
-                    } %>
+        <%= render  "admin/application/slug/form",
+                    f: f,
+                    source: '#administration_location_name' %>
       <% end %>
       <%= render 'admin/application/featured_image/edit', about: location, f: f %>
     </div>
@@ -42,7 +39,7 @@
                         value_method: ->(p) { p[:id] } %>
     </div>
     <div class="col-md-4">
-      <%= f.association :schools, 
+      <%= f.association :schools,
                         as: :check_boxes,
                         collection: current_university.education_schools.ordered %>
     </div>