From 2181a65bc2c6e0aaf1e9cb51e9a7addef8873051 Mon Sep 17 00:00:00 2001 From: Angel Perez Date: Tue, 12 Sep 2017 11:42:51 -0400 Subject: [PATCH] Allow only YouTube/Vimeo URLs on 'video_url' attribute Fixes #1841 On branch aperez-valid-video-url Changes to be committed: modified: app/models/proposal.rb modified: spec/factories.rb modified: spec/models/proposal_spec.rb Allow only YouTube/Vimeo URLs on 'video_url' attribute Fixes #1841 On branch aperez-valid-video-url Changes to be committed: modified: app/helpers/embed_videos_helper.rb modified: app/models/proposal.rb modified: spec/features/management/proposals_spec.rb modified: spec/features/proposals_spec.rb modified: spec/features/tags/proposals_spec.rb modified: spec/models/proposal_spec.rb --- app/helpers/embed_videos_helper.rb | 16 +++++++++++++--- app/models/proposal.rb | 3 +++ spec/factories.rb | 2 +- spec/features/management/proposals_spec.rb | 4 ++-- spec/features/proposals_spec.rb | 10 +++++----- spec/features/tags/proposals_spec.rb | 2 +- spec/models/proposal_spec.rb | 12 ++++++++++++ 7 files changed, 37 insertions(+), 12 deletions(-) diff --git a/app/helpers/embed_videos_helper.rb b/app/helpers/embed_videos_helper.rb index 1034a7a59..f1750bf8b 100644 --- a/app/helpers/embed_videos_helper.rb +++ b/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 \ No newline at end of file + 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 diff --git a/app/models/proposal.rb b/app/models/proposal.rb index 4ee4f851e..f9eeecc6f 100644 --- a/app/models/proposal.rb +++ b/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 diff --git a/spec/factories.rb b/spec/factories.rb index 9cebfddea..9a038816a 100644 --- a/spec/factories.rb +++ b/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 diff --git a/spec/features/management/proposals_spec.rb b/spec/features/management/proposals_spec.rb index 8f63146e9..349a4f904 100644 --- a/spec/features/management/proposals_spec.rb +++ b/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) diff --git a/spec/features/proposals_spec.rb b/spec/features/proposals_spec.rb index 0c69eee3e..f9bef400c 100644 --- a/spec/features/proposals_spec.rb +++ b/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' diff --git a/spec/features/tags/proposals_spec.rb b/spec/features/tags/proposals_spec.rb index cf5017d4f..d2a16b8f7 100644 --- a/spec/features/tags/proposals_spec.rb +++ b/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' diff --git a/spec/models/proposal_spec.rb b/spec/models/proposal_spec.rb index 5bf6d90e1..83c19edde 100644 --- a/spec/models/proposal_spec.rb +++ b/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