Merge pull request #1854 from wairbut-m2c/aperez-valid-video-url

Allow only YouTube/Vimeo URLs on 'video_url' attribute

app/helpers/embed_videos_helper.rb

@@ -1,5 +1,8 @@
 module EmbedVideosHelper
 
+  VIMEO_REGEX = /vimeo.*(staffpicks\/|channels\/|videos\/|video\/|\/)([^#\&\?]*).*/
+  YOUTUBE_REGEX = /youtu.*(be\/|v\/|u\/\w\/|embed\/|watch\?v=|\&v=)([^#\&\?]*).*/
+
   def embedded_video_code
     link = @proposal.video_url
     title = t('proposals.show.embed_video_title', proposal: @proposal.title)
@@ -10,10 +13,10 @@ module EmbedVideosHelper
     end
 
     if server == "Vimeo"
-      reg_exp = /vimeo.*(staffpicks\/|channels\/|videos\/|video\/|\/)([^#\&\?]*).*/
+      reg_exp = VIMEO_REGEX
       src = "https://player.vimeo.com/video/"
     elsif server == "YouTube"
-      reg_exp = /youtu.*(be\/|v\/|u\/\w\/|embed\/|watch\?v=|\&v=)([^#\&\?]*).*/
+      reg_exp = YOUTUBE_REGEX
       src = "https://www.youtube.com/embed/"
     end
 
@@ -28,4 +31,11 @@ module EmbedVideosHelper
     end
   end
 
-end
+  def valid_video_url?
+    return if video_url.blank?
+    return if video_url.match(VIMEO_REGEX)
+    return if video_url.match(YOUTUBE_REGEX)
+    errors.add(:video_url, :invalid)
+  end
+
+end

app/models/proposal.rb

@@ -15,6 +15,7 @@ class Proposal < ActiveRecord::Base
                max_file_size: 3.megabytes,
                accepted_content_types: [ "application/pdf" ]
   accepts_nested_attributes_for :documents, allow_destroy: true
+  include EmbedVideosHelper
 
   acts_as_votable
   acts_as_paranoid column: :hidden_at
@@ -41,6 +42,8 @@ class Proposal < ActiveRecord::Base
 
   validates :terms_of_service, acceptance: { allow_nil: false }, on: :create
 
+  validate :valid_video_url?
+
   before_validation :set_responsible_name
 
   before_save :calculate_hot_score, :calculate_confidence_score

spec/factories.rb

@@ -155,7 +155,7 @@ FactoryGirl.define do
     description          'Proposal description'
     question             'Proposal question'
     external_url         'http://external_documention.es'
-    video_url            'http://video_link.com'
+    video_url            'https://youtu.be/nhuNb0XtRhQ'
     responsible_name     'John Snow'
     terms_of_service     '1'
     association :author, factory: :user

spec/features/management/proposals_spec.rb

@@ -26,7 +26,7 @@ feature 'Proposals' do
       fill_in 'proposal_summary', with: 'In summary, what we want is...'
       fill_in 'proposal_description', with: 'This is very important because...'
       fill_in 'proposal_external_url', with: 'http://rescue.org/refugees'
-      fill_in 'proposal_video_url', with: 'http://youtube.com'
+      fill_in 'proposal_video_url', with: 'https://www.youtube.com/watch?v=yRYFKcMa_Ek'
       check 'proposal_terms_of_service'
 
       click_button 'Create proposal'
@@ -38,7 +38,7 @@ feature 'Proposals' do
       expect(page).to have_content 'In summary, what we want is...'
       expect(page).to have_content 'This is very important because...'
       expect(page).to have_content 'http://rescue.org/refugees'
-      expect(page).to have_content 'http://youtube.com'
+      expect(page).to have_content 'https://www.youtube.com/watch?v=yRYFKcMa_Ek'
       expect(page).to have_content user.name
       expect(page).to have_content I18n.l(Proposal.last.created_at.to_date)
 

spec/features/proposals_spec.rb

@@ -151,7 +151,7 @@ feature 'Proposals' do
     fill_in 'proposal_summary', with: 'In summary, what we want is...'
     fill_in 'proposal_description', with: 'This is very important because...'
     fill_in 'proposal_external_url', with: 'http://rescue.org/refugees'
-    fill_in 'proposal_video_url', with: 'http://youtube.com'
+    fill_in 'proposal_video_url', with: 'https://www.youtube.com/watch?v=yPQfcG-eimk'
     fill_in 'proposal_responsible_name', with: 'Isabel Garcia'
     fill_in 'proposal_tag_list', with: 'Refugees, Solidarity'
     check 'proposal_terms_of_service'
@@ -169,7 +169,7 @@ feature 'Proposals' do
     expect(page).to have_content 'In summary, what we want is...'
     expect(page).to have_content 'This is very important because...'
     expect(page).to have_content 'http://rescue.org/refugees'
-    expect(page).to have_content 'http://youtube.com'
+    expect(page).to have_content 'https://www.youtube.com/watch?v=yPQfcG-eimk'
     expect(page).to have_content author.name
     expect(page).to have_content 'Refugees'
     expect(page).to have_content 'Solidarity'
@@ -187,7 +187,7 @@ feature 'Proposals' do
     fill_in 'proposal_summary', with: 'In summary, what we want is...'
     fill_in 'proposal_description', with: 'This is very important because...'
     fill_in 'proposal_external_url', with: 'http://rescue.org/refugees'
-    fill_in 'proposal_video_url', with: 'http://youtube.com'
+    fill_in 'proposal_video_url', with: 'https://www.youtube.com/watch?v=yPQfcG-eimk'
     fill_in 'proposal_responsible_name', with: 'Isabel Garcia'
     fill_in 'proposal_tag_list', with: 'Refugees, Solidarity'
     check 'proposal_terms_of_service'
@@ -393,7 +393,7 @@ feature 'Proposals' do
       fill_in 'proposal_summary', with: 'In summary, what we want is...'
       fill_in 'proposal_description', with: 'This is very important because...'
       fill_in 'proposal_external_url', with: 'http://rescue.org/refugees'
-      fill_in 'proposal_video_url', with: 'http://youtube.com'
+      fill_in 'proposal_video_url', with: 'https://www.youtube.com/watch?v=yPQfcG-eimk'
       fill_in 'proposal_responsible_name', with: 'Isabel Garcia'
       check 'proposal_terms_of_service'
 
@@ -421,7 +421,7 @@ feature 'Proposals' do
       fill_in 'proposal_summary', with: 'In summary, what we want is...'
       fill_in 'proposal_description', with: 'This is very important because...'
       fill_in 'proposal_external_url', with: 'http://rescue.org/refugees'
-      fill_in 'proposal_video_url', with: 'http://youtube.com'
+      fill_in 'proposal_video_url', with: 'https://www.youtube.com/watch?v=yPQfcG-eimk'
       fill_in 'proposal_responsible_name', with: 'Isabel Garcia'
       check 'proposal_terms_of_service'
 

spec/features/tags/proposals_spec.rb

@@ -101,7 +101,7 @@ feature 'Tags' do
     fill_in 'proposal_summary', with: 'In summary, what we want is...'
     fill_in_ckeditor 'proposal_description', with: 'A description with enough characters'
     fill_in 'proposal_external_url', with: 'http://rescue.org/refugees'
-    fill_in 'proposal_video_url', with: 'http://youtube.com'
+    fill_in 'proposal_video_url', with: 'https://www.youtube.com/watch?v=Ae6gQmhaMn4'
     fill_in 'proposal_responsible_name', with: 'Isabel Garcia'
     check 'proposal_terms_of_service'
 

spec/models/proposal_spec.rb

@@ -67,6 +67,18 @@ describe Proposal do
     end
   end
 
+  describe "#video_url" do
+    it "should not be valid when URL is not from Youtube or Vimeo" do
+      proposal.video_url = "https://twitter.com"
+      expect(proposal).to_not be_valid
+    end
+
+    it "should be valid when URL is from Youtube or Vimeo" do
+      proposal.video_url = "https://vimeo.com/112681885"
+      expect(proposal).to be_valid
+    end
+  end
+
   describe "#responsible_name" do
     it "should be mandatory" do
       proposal.responsible_name = nil