From a0603985ef438fbd1041333bf0c78b07aa9d58f0 Mon Sep 17 00:00:00 2001 From: taitus Date: Wed, 13 Sep 2017 18:07:18 +0200 Subject: [PATCH] Refactor. --- app/models/budget/investment.rb | 3 +- app/models/concerns/mappable.rb | 9 + app/models/proposal.rb | 5 +- spec/features/admin/settings_spec.rb | 11 +- spec/features/budgets/investments_spec.rb | 2 +- spec/features/proposals_spec.rb | 33 +--- spec/shared/features/mapeable.rb | 220 ---------------------- spec/shared/features/mappable.rb | 220 ++++++++++++++++++++++ 8 files changed, 243 insertions(+), 260 deletions(-) create mode 100644 app/models/concerns/mappable.rb delete mode 100644 spec/shared/features/mapeable.rb create mode 100644 spec/shared/features/mappable.rb diff --git a/app/models/budget/investment.rb b/app/models/budget/investment.rb index e48bc2c57..ddf6e819c 100644 --- a/app/models/budget/investment.rb +++ b/app/models/budget/investment.rb @@ -8,6 +8,7 @@ class Budget include Followable include Communitable include Imageable + include Mappable include Documentable documentable max_documents_allowed: 3, max_file_size: 3.megabytes, @@ -27,8 +28,6 @@ class Budget has_many :valuators, through: :valuator_assignments has_many :comments, as: :commentable has_many :milestones - has_one :map_location, dependent: :destroy - accepts_nested_attributes_for :map_location, allow_destroy: true validates :title, presence: true validates :author, presence: true diff --git a/app/models/concerns/mappable.rb b/app/models/concerns/mappable.rb new file mode 100644 index 000000000..c108bdca7 --- /dev/null +++ b/app/models/concerns/mappable.rb @@ -0,0 +1,9 @@ +module Mappable + extend ActiveSupport::Concern + + included do + has_one :map_location, dependent: :destroy + accepts_nested_attributes_for :map_location, allow_destroy: true + end + +end diff --git a/app/models/proposal.rb b/app/models/proposal.rb index b8d919e97..da0fc109f 100644 --- a/app/models/proposal.rb +++ b/app/models/proposal.rb @@ -11,10 +11,11 @@ class Proposal < ActiveRecord::Base include Followable include Communitable include Imageable + include Mappable include Documentable documentable max_documents_allowed: 3, max_file_size: 3.megabytes, - accepted_content_types: [ "application/pdf" ] + accepted_content_types: [ "application/pdf" ] include EmbedVideosHelper acts_as_votable @@ -25,8 +26,6 @@ class Proposal < ActiveRecord::Base belongs_to :author, -> { with_hidden }, class_name: 'User', foreign_key: 'author_id' belongs_to :geozone - has_one :map_location, dependent: :destroy - accepts_nested_attributes_for :map_location, allow_destroy: true has_many :comments, as: :commentable has_many :proposal_notifications diff --git a/spec/features/admin/settings_spec.rb b/spec/features/admin/settings_spec.rb index e3c968eca..73d190cb9 100644 --- a/spec/features/admin/settings_spec.rb +++ b/spec/features/admin/settings_spec.rb @@ -61,17 +61,24 @@ feature 'Admin settings' do expect(page).to have_content "Map configuration updated succesfully" end - scenario "Should update marker", :js do + scenario "Should display marker by default", :js do Setting['feature.map'] = true admin = create(:administrator).user login_as(admin) + visit admin_settings_path expect(find("#latitude", visible: false).value).to eq "51.48" expect(find("#longitude", visible: false).value).to eq "0.0" + end + scenario "Should update marker", :js do + Setting['feature.map'] = true + admin = create(:administrator).user + login_as(admin) + + visit admin_settings_path find("#admin-map").click - within "#map-form" do click_on "Update" end diff --git a/spec/features/budgets/investments_spec.rb b/spec/features/budgets/investments_spec.rb index 6d487772e..d17bd9c91 100644 --- a/spec/features/budgets/investments_spec.rb +++ b/spec/features/budgets/investments_spec.rb @@ -485,7 +485,7 @@ feature 'Budget Investments' do "Create Investment", "Budget Investment created successfully." - it_behaves_like "mapeable", + it_behaves_like "mappable", "budget_investment", "investment", "new_budget_investment_path", diff --git a/spec/features/proposals_spec.rb b/spec/features/proposals_spec.rb index 53af5d6ac..f0e50c7eb 100644 --- a/spec/features/proposals_spec.rb +++ b/spec/features/proposals_spec.rb @@ -1328,7 +1328,7 @@ feature 'Proposals' do "Save changes", "Proposal updated successfully" - it_behaves_like "mapeable", + it_behaves_like "mappable", "proposal", "proposal", "new_proposal_path", @@ -1541,37 +1541,6 @@ feature 'Proposals' do end - scenario 'Add Map on proposal' do - author = create(:user) - login_as(author) - - visit new_proposal_path - fill_in 'proposal_title', with: 'Help refugees' - fill_in 'proposal_question', with: '¿Would you like to give assistance to war refugees?' - fill_in 'proposal_summary', with: 'In summary, what we want is...' - check 'proposal_terms_of_service' - - click_button 'Create proposal' - - expect(page).to have_content 'Proposal created successfully.' - expect(page).to have_content 'Help refugees' - expect(page).not_to have_content 'You can also see more information about improving your campaign' - - click_link 'Not now, go to my proposal' - - expect(page).to have_content 'Help refugees' - expect(page).to have_content '¿Would you like to give assistance to war refugees?' - 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 author.name - expect(page).to have_content 'Refugees' - expect(page).to have_content 'Solidarity' - expect(page).to have_content I18n.l(Proposal.last.created_at.to_date) - end - - end feature 'Successful proposals' do diff --git a/spec/shared/features/mapeable.rb b/spec/shared/features/mapeable.rb deleted file mode 100644 index 1ed69d049..000000000 --- a/spec/shared/features/mapeable.rb +++ /dev/null @@ -1,220 +0,0 @@ -shared_examples "mapeable" do |mapeable_factory_name, mapeable_association_name, mapeable_new_path, mapeable_edit_path, mapeable_show_path, mapeable_path_arguments| - - include ActionView::Helpers - - let!(:user) { create(:user, :level_two) } - - before do - Setting['feature.map'] = true - end - - describe "At #{mapeable_new_path}" do - - let!(:arguments) { {} } - let!(:mapeable) { create("#{mapeable_factory_name}".to_sym) } - let!(:map_location) { create(:map_location, "#{mapeable_factory_name}_map_location".to_sym, "#{mapeable_association_name}": mapeable) } - - before { set_arguments(arguments, mapeable, mapeable_path_arguments) } - - scenario "Should not show marker by default on create #{mapeable_factory_name}", :js do - login_as user - visit send(mapeable_new_path, arguments) - - send("fill_in_#{mapeable_factory_name}_form") - - within ".map_location" do - expect(page).not_to have_css(".map-icon") - end - end - - scenario "Should show marker on create #{mapeable_factory_name} when click on map", :js do - login_as user - visit send(mapeable_new_path, arguments) - - send("fill_in_#{mapeable_factory_name}_form") - find("#new_map_location").click - - within ".map_location" do - expect(page).to have_css(".map-icon") - end - end - - scenario "Should create #{mapeable_factory_name} with map", :js do - login_as user - visit send(mapeable_new_path, arguments) - - send("fill_in_#{mapeable_factory_name}_form") - find("#new_map_location").click - send("submit_#{mapeable_factory_name}_form") - - expect(page).to have_css(".map_location") - end - - scenario "Can not display map on #{mapeable_factory_name} when not fill marker on map", :js do - login_as user - visit send(mapeable_new_path, arguments) - - send("fill_in_#{mapeable_factory_name}_form") - expect(page).to have_css ".map_location" - send("submit_#{mapeable_factory_name}_form") - - expect(page).not_to have_css(".map_location") - end - - scenario "Can not display map on #{mapeable_factory_name} when feature.map is disabled", :js do - Setting['feature.map'] = false - login_as user - visit send(mapeable_new_path, arguments) - - send("fill_in_#{mapeable_factory_name}_form") - expect(page).not_to have_css ".map_location" - send("submit_#{mapeable_factory_name}_form") - - expect(page).not_to have_css(".map_location") - end - - end - - describe "At #{mapeable_edit_path}" do - - let!(:mapeable) { create("#{mapeable_factory_name}".to_sym) } - let!(:map_location) { create(:map_location, "#{mapeable_factory_name}_map_location".to_sym, "#{mapeable_association_name}": mapeable) } - - before { skip } unless mapeable_edit_path.present? - - scenario "Should edit map on #{mapeable_factory_name} and contain default values", :js do - login_as mapeable.author - - visit send(mapeable_edit_path, id: mapeable.id) - - expect(page).to have_content "Navigate the map to the location and place the marker." - validate_latitude_longitude(mapeable_factory_name) - end - - scenario "Should edit default values from map on #{mapeable_factory_name} edit page", :js do - login_as mapeable.author - - visit send(mapeable_edit_path, id: mapeable.id) - find(".map_location").click - click_on("Save changes") - mapeable.reload - - expect(page).to have_css(".map_location") - expect(page).not_to have_selector(".map_location[data-marker-latitude='#{map_location.latitude}']") - expect(page).to have_selector(".map_location[data-marker-latitude='#{mapeable.map_location.latitude}']") - end - - scenario "Should edit mapeable on #{mapeable_factory_name} without change map", :js do - login_as mapeable.author - - visit send(mapeable_edit_path, id: mapeable.id) - fill_in "#{mapeable_factory_name}_title", with: "New title" - click_on("Save changes") - mapeable.reload - - expect(page).to have_css(".map_location") - expect(page).to have_selector(".map_location[data-marker-latitude='#{map_location.latitude}']") - expect(page).to have_selector(".map_location[data-marker-latitude='#{mapeable.map_location.latitude}']") - end - - scenario "Can not display map on #{mapeable_factory_name} edit when remove map marker", :js do - login_as mapeable.author - - visit send(mapeable_edit_path, id: mapeable.id) - click_link "Remove map marker" - click_on "Save changes" - - expect(page).not_to have_css(".map_location") - end - - scenario "Can not display map on #{mapeable_factory_name} edit when feature.map is disabled", :js do - Setting['feature.map'] = false - login_as mapeable.author - - visit send(mapeable_edit_path, id: mapeable.id) - fill_in "#{mapeable_factory_name}_title", with: "New title" - click_on("Save changes") - - expect(page).not_to have_css(".map_location") - end - - end - - describe "At #{mapeable_show_path}" do - - let!(:arguments) { {} } - let!(:mapeable) { create("#{mapeable_factory_name}".to_sym) } - let!(:map_location) { create(:map_location, "#{mapeable_factory_name}_map_location".to_sym, "#{mapeable_association_name}": mapeable) } - - before { set_arguments(arguments, mapeable, mapeable_path_arguments) } - - scenario "Should display map on #{mapeable_factory_name} show page", :js do - arguments.merge!("id": mapeable.id) - - visit send(mapeable_show_path, arguments) - - expect(page).to have_css(".map_location") - end - - scenario "Should not display map on #{mapeable_factory_name} show when marker is not defined", :js do - mapeable_without_map = create("#{mapeable_factory_name}".to_sym) - set_arguments(arguments, mapeable_without_map, mapeable_path_arguments) - arguments.merge!("id": mapeable_without_map.id) - - visit send(mapeable_show_path, arguments) - - expect(page).not_to have_css(".map_location") - end - - scenario "Should not display map on #{mapeable_factory_name} show page when feature.map is disable", :js do - Setting['feature.map'] = false - arguments.merge!("id": mapeable.id) - - visit send(mapeable_show_path, arguments) - - expect(page).not_to have_css(".map_location") - end - - end - -end - -def fill_in_proposal_form - fill_in 'proposal_title', with: 'Help refugees' - fill_in 'proposal_question', with: '¿Would you like to give assistance to war refugees?' - fill_in 'proposal_summary', with: 'In summary, what we want is...' -end - -def submit_proposal_form - check :proposal_terms_of_service - click_button 'Create proposal' - - click_link 'Not now, go to my proposal' -end - -def validate_latitude_longitude(mapeable_factory_name) - expect(find("##{mapeable_factory_name}_map_location_attributes_latitude", visible: false).value).to eq "51.48" - expect(find("##{mapeable_factory_name}_map_location_attributes_longitude", visible: false).value).to eq "0.0" - expect(mapeable.map_location.latitude).to eq 51.48 - expect(mapeable.map_location.longitude).to eq 0.0 -end - -def fill_in_budget_investment_form - page.select mapeable.heading.name_scoped_by_group, from: :budget_investment_heading_id - fill_in :budget_investment_title, with: "Budget investment title" - fill_in_ckeditor "budget_investment_description", with: "Budget investment description" - check :budget_investment_terms_of_service -end - -def submit_budget_investment_form - check :budget_investment_terms_of_service - click_button 'Create Investment' -end - -def set_arguments(arguments, mapeable, mapeable_path_arguments) - if mapeable_path_arguments - mapeable_path_arguments.each do |argument_name, path_to_value| - arguments.merge!("#{argument_name}": mapeable.send(path_to_value)) - end - end -end diff --git a/spec/shared/features/mappable.rb b/spec/shared/features/mappable.rb new file mode 100644 index 000000000..ccbcd5625 --- /dev/null +++ b/spec/shared/features/mappable.rb @@ -0,0 +1,220 @@ +shared_examples "mappable" do |mappable_factory_name, mappable_association_name, mappable_new_path, mappable_edit_path, mappable_show_path, mappable_path_arguments| + + include ActionView::Helpers + + let!(:user) { create(:user, :level_two) } + + before do + Setting['feature.map'] = true + end + + describe "At #{mappable_new_path}" do + + let!(:arguments) { {} } + let!(:mappable) { create("#{mappable_factory_name}".to_sym) } + let!(:map_location) { create(:map_location, "#{mappable_factory_name}_map_location".to_sym, "#{mappable_association_name}": mappable) } + + before { set_arguments(arguments, mappable, mappable_path_arguments) } + + scenario "Should not show marker by default on create #{mappable_factory_name}", :js do + login_as user + visit send(mappable_new_path, arguments) + + send("fill_in_#{mappable_factory_name}_form") + + within ".map_location" do + expect(page).not_to have_css(".map-icon") + end + end + + scenario "Should show marker on create #{mappable_factory_name} when click on map", :js do + login_as user + visit send(mappable_new_path, arguments) + + send("fill_in_#{mappable_factory_name}_form") + find("#new_map_location").click + + within ".map_location" do + expect(page).to have_css(".map-icon") + end + end + + scenario "Should create #{mappable_factory_name} with map", :js do + login_as user + visit send(mappable_new_path, arguments) + + send("fill_in_#{mappable_factory_name}_form") + find("#new_map_location").click + send("submit_#{mappable_factory_name}_form") + + expect(page).to have_css(".map_location") + end + + scenario "Can not display map on #{mappable_factory_name} when not fill marker on map", :js do + login_as user + visit send(mappable_new_path, arguments) + + send("fill_in_#{mappable_factory_name}_form") + expect(page).to have_css ".map_location" + send("submit_#{mappable_factory_name}_form") + + expect(page).not_to have_css(".map_location") + end + + scenario "Can not display map on #{mappable_factory_name} when feature.map is disabled", :js do + Setting['feature.map'] = false + login_as user + visit send(mappable_new_path, arguments) + + send("fill_in_#{mappable_factory_name}_form") + expect(page).not_to have_css ".map_location" + send("submit_#{mappable_factory_name}_form") + + expect(page).not_to have_css(".map_location") + end + + end + + describe "At #{mappable_edit_path}" do + + let!(:mappable) { create("#{mappable_factory_name}".to_sym) } + let!(:map_location) { create(:map_location, "#{mappable_factory_name}_map_location".to_sym, "#{mappable_association_name}": mappable) } + + before { skip } unless mappable_edit_path.present? + + scenario "Should edit map on #{mappable_factory_name} and contain default values", :js do + login_as mappable.author + + visit send(mappable_edit_path, id: mappable.id) + + expect(page).to have_content "Navigate the map to the location and place the marker." + validate_latitude_longitude(mappable_factory_name) + end + + scenario "Should edit default values from map on #{mappable_factory_name} edit page", :js do + login_as mappable.author + + visit send(mappable_edit_path, id: mappable.id) + find(".map_location").click + click_on("Save changes") + mappable.reload + + expect(page).to have_css(".map_location") + expect(page).not_to have_selector(".map_location[data-marker-latitude='#{map_location.latitude}']") + expect(page).to have_selector(".map_location[data-marker-latitude='#{mappable.map_location.latitude}']") + end + + scenario "Should edit mappable on #{mappable_factory_name} without change map", :js do + login_as mappable.author + + visit send(mappable_edit_path, id: mappable.id) + fill_in "#{mappable_factory_name}_title", with: "New title" + click_on("Save changes") + mappable.reload + + expect(page).to have_css(".map_location") + expect(page).to have_selector(".map_location[data-marker-latitude='#{map_location.latitude}']") + expect(page).to have_selector(".map_location[data-marker-latitude='#{mappable.map_location.latitude}']") + end + + scenario "Can not display map on #{mappable_factory_name} edit when remove map marker", :js do + login_as mappable.author + + visit send(mappable_edit_path, id: mappable.id) + click_link "Remove map marker" + click_on "Save changes" + + expect(page).not_to have_css(".map_location") + end + + scenario "Can not display map on #{mappable_factory_name} edit when feature.map is disabled", :js do + Setting['feature.map'] = false + login_as mappable.author + + visit send(mappable_edit_path, id: mappable.id) + fill_in "#{mappable_factory_name}_title", with: "New title" + click_on("Save changes") + + expect(page).not_to have_css(".map_location") + end + + end + + describe "At #{mappable_show_path}" do + + let!(:arguments) { {} } + let!(:mappable) { create("#{mappable_factory_name}".to_sym) } + let!(:map_location) { create(:map_location, "#{mappable_factory_name}_map_location".to_sym, "#{mappable_association_name}": mappable) } + + before { set_arguments(arguments, mappable, mappable_path_arguments) } + + scenario "Should display map on #{mappable_factory_name} show page", :js do + arguments.merge!("id": mappable.id) + + visit send(mappable_show_path, arguments) + + expect(page).to have_css(".map_location") + end + + scenario "Should not display map on #{mappable_factory_name} show when marker is not defined", :js do + mappable_without_map = create("#{mappable_factory_name}".to_sym) + set_arguments(arguments, mappable_without_map, mappable_path_arguments) + arguments.merge!("id": mappable_without_map.id) + + visit send(mappable_show_path, arguments) + + expect(page).not_to have_css(".map_location") + end + + scenario "Should not display map on #{mappable_factory_name} show page when feature.map is disable", :js do + Setting['feature.map'] = false + arguments.merge!("id": mappable.id) + + visit send(mappable_show_path, arguments) + + expect(page).not_to have_css(".map_location") + end + + end + +end + +def fill_in_proposal_form + fill_in 'proposal_title', with: 'Help refugees' + fill_in 'proposal_question', with: '¿Would you like to give assistance to war refugees?' + fill_in 'proposal_summary', with: 'In summary, what we want is...' +end + +def submit_proposal_form + check :proposal_terms_of_service + click_button 'Create proposal' + + click_link 'Not now, go to my proposal' +end + +def validate_latitude_longitude(mappable_factory_name) + expect(find("##{mappable_factory_name}_map_location_attributes_latitude", visible: false).value).to eq "51.48" + expect(find("##{mappable_factory_name}_map_location_attributes_longitude", visible: false).value).to eq "0.0" + expect(mappable.map_location.latitude).to eq 51.48 + expect(mappable.map_location.longitude).to eq 0.0 +end + +def fill_in_budget_investment_form + page.select mappable.heading.name_scoped_by_group, from: :budget_investment_heading_id + fill_in :budget_investment_title, with: "Budget investment title" + fill_in_ckeditor "budget_investment_description", with: "Budget investment description" + check :budget_investment_terms_of_service +end + +def submit_budget_investment_form + check :budget_investment_terms_of_service + click_button 'Create Investment' +end + +def set_arguments(arguments, mappable, mappable_path_arguments) + if mappable_path_arguments + mappable_path_arguments.each do |argument_name, path_to_value| + arguments.merge!("#{argument_name}": mappable.send(path_to_value)) + end + end +end