Merge pull request #309 from Trim/fix-link-validation

link and bookmark model validate URL

Author: Oumph

app/assets/javascripts/edition_in_place.coffee

@@ -42,6 +42,20 @@ class EditionInPlace
     false
 
   error: =>
+    try
+      error = @el.find("ul.error")
+      response = $.parseJSON(@xhr.responseText)
+      messages = []
+      for attribute, errors of response.errors
+        for message in errors
+          messages.push(message)
+      if messages.length == 1
+        error.text("Erreur : " + messages[0])
+      else
+        error.text("Erreurs :")
+        for message in messages
+          error.append($("<li>").append(message))
+      error.show()
     @el.trigger "in_place:error", @xhr
     @xhr = null
 

app/assets/stylesheets/parts/content.scss

@@ -13,7 +13,9 @@ body#news-revision #contents,
 body#boards-show #contents,
 body#stylesheets-edit #contents,
 body#wiki_pages-changes #contents,
-body#admin-index #container > ul {
+body#admin-index #container > ul,
+#redaction #new_link,
+#redaction .edit_link {
   display: block;
   background: white;
   padding: 10px $PX_PAD_NODE 10px $PX_PAD_NODE;

app/assets/stylesheets/parts/redaction.scss

@@ -503,6 +503,10 @@ body#redaction-index {
       }
     }
   }
+  #new_link,
+  .edit_link {
+    padding: 5px;
+  }
   input#link_title {
     width: 100%; // this rule force the input to not enlarge the #edition flex box
   }

app/controllers/redaction/links_controller.rb

@@ -12,22 +12,29 @@ def new
   def create
     @link.attributes = link_params
     @link.user = current_user
-    @link.save
-    render partial: 'button'
+    if @link.save
+      render partial: 'button'
+    else
+      render status: :unprocessable_entity, json: { errors: @link.errors.as_json }
+    end
   end
 
   def edit
     if @link.lock_by(current_user)
       render partial: 'form'
     else
-      render status: :forbidden, text: "Désolé, #{@link.locker} est déjà en train de modifier ce lien !"
+      render status: :forbidden, plain: "Désolé, #{@link.locker} est déjà en train de modifier ce lien !"
     end
   end
 
   def update
     @link.attributes = link_params
     @link.update_by(current_user)
-    render @link
+    if @link.destroyed? || @link.save
+      render @link
+    else
+      render status: :unprocessable_entity, json: { errors: @link.errors.as_json }
+    end
   end
 
   def unlock

app/controllers/redaction/news_controller.rb

@@ -59,14 +59,18 @@ def reorganize
       @news.put_paragraphs_together
       render :reorganize, layout: "chat_n_edit"
     else
-      render status: :forbidden, text: "Désolé, un verrou a déjà été posé sur cette dépêche !"
+      render status: :forbidden, plain: "Désolé, un verrou a déjà été posé sur cette dépêche !"
     end
   end
 
   def reorganized
     @news.editor = current_account
-    @news.reorganize news_params
-    redirect_to [@news.draft? ? :redaction : :moderation, @news]
+    if @news.reorganize news_params
+      redirect_to [@news.draft? ? :redaction : :moderation, @news]
+    else
+      load_board
+      render :reorganize, layout: "chat_n_edit"
+    end
   end
 
   def followup

app/controllers/redaction/paragraphs_controller.rb

@@ -25,7 +25,7 @@ def edit
     if @paragraph.lock_by(current_user)
       render partial: 'form'
     else
-      render status: :forbidden, text: "Désolé, #{@paragraph.locker} est déjà en train de modifier ce paragraphe !"
+      render status: :forbidden, plain: "Désolé, #{@paragraph.locker} est déjà en train de modifier ce paragraphe !"
     end
   end
 

app/models/bookmark.rb

@@ -26,6 +26,7 @@ class Bookmark < Content
   validates :title,     presence: { message: "Le titre est obligatoire" },
                         length: { maximum: 100, message: "Le titre est trop long" }
   validates :link, presence: { message: "Vous ne pouvez pas poster un lien vide" },
+                   http_url: { message: "Le lien n'est pas valide" },
                    length: { maximum: 255, message: "Le lien est trop long" }
 
   def create_node(attrs={})

app/models/link.rb

@@ -26,9 +26,9 @@ class Link < ActiveRecord::Base
 
   validates :title, presence: { message: "Un lien doit obligatoirement avoir un titre" },
                     length: { maximum: 100, message: "Le titre est trop long" }
-  validates :url, presence: { message: "Un lien doit obligatoirement avoir une adresse" },
+  validates :url, http_url: { protocols: PROTOCOLS, message: "L'adresse n'est pas valide" },
+                  presence: { message: "Un lien doit obligatoirement avoir une adresse" },
                   length: { maximum: 255, message: "L’adresse est trop longue" }
-  validate  :authorized_protocol
 
   def url=(raw)
     raw.strip!
@@ -39,17 +39,9 @@ def url=(raw)
       uri = URI.parse(raw)
     end
     write_attribute :url, uri.to_s
-  end
-
-  def authorized_protocol
-    if url.blank?
-      errors.add(:url, "L’adresse est obligatoire")
-    else
-      uri = URI.parse(url)
-      return true if PROTOCOLS.include?(uri.scheme)
-      return true if uri.scheme.nil? && uri.host == MY_DOMAIN
-      errors.add(:url, "L’adresse d’un lien n’est pas valide")
-    end
+  # Let raw value if error when parsed, HttpUrlValidator will manage it
+  rescue URI::InvalidURIError
+    write_attribute :url, raw
   end
 
 ### Behaviour ###

app/models/news.rb

@@ -260,11 +260,20 @@ def nb_editors
   end
 
   def reorganize(params)
-    Paragraph.where(news_id: self.id).delete_all
-    self.attributes = params
-    create_parts
-    save
-    unlock
+    reorganized = false
+    self.transaction do
+      Paragraph.where(news_id: self.id).delete_all
+      self.attributes = params
+      create_parts
+      # Let commit transaction only if save is successful so version will be well created
+      if save
+        reorganized = true
+      else
+        raise ActiveRecord::Rollback
+      end
+    end
+    unlock if reorganized
+    reorganized
   end
 
 ### Associated node ###

app/validators/http_url_validator.rb

@@ -0,0 +1,24 @@
+# Validate if a value is a HTTP url
+class HttpUrlValidator < ActiveModel::EachValidator
+  def validate_each(record, attribute, value)
+    if value.present? && not(valid?(value, options))
+      record.errors.add attribute, (options[:message] || "n'est pas une URL HTTP valide")
+    end
+  end
+
+  private
+
+  def valid?(value, options)
+    uri = URI.parse(value)
+    if uri.scheme.blank? && uri.host.blank?
+      value = "http://#{value}"
+      uri = URI.parse(value)
+    end
+    if options.has_key?(:protocols)
+      return true if options[:protocols].include?(uri.scheme)
+    end
+    uri.scheme.nil? && uri.host == MY_DOMAIN
+  rescue URI::InvalidURIError
+    false
+  end 
+end