diff --git a/app/assets/javascripts/map.js b/app/assets/javascripts/map.js index ff4ce0dd0..0684b654f 100644 --- a/app/assets/javascripts/map.js +++ b/app/assets/javascripts/map.js @@ -193,7 +193,6 @@ if (App.Map.validCoordinates(coordinates)) { marker = createMarker(coordinates.lat, coordinates.long, coordinates.title); - marker.options.id = coordinates.investment_id; marker.bindPopup(App.Map.getPopupContent(coordinates)); } }); diff --git a/app/components/admin/settings/map_form_component.html.erb b/app/components/admin/settings/map_form_component.html.erb index 7a0f00a95..e4a2e7c48 100644 --- a/app/components/admin/settings/map_form_component.html.erb +++ b/app/components/admin/settings/map_form_component.html.erb @@ -1,25 +1,10 @@
<%= help %>
- -<%= form.fields_for :map_location, map_location do |m_l_fields| %> - <%= render_map(map_location, form: m_l_fields) %> - - <%= m_l_fields.hidden_field :latitude %> - <%= m_l_fields.hidden_field :longitude %> - <%= m_l_fields.hidden_field :zoom %> -<% end %> diff --git a/config/locales/en/budgets.yml b/config/locales/en/budgets.yml index 384b55030..b6320ce64 100644 --- a/config/locales/en/budgets.yml +++ b/config/locales/en/budgets.yml @@ -80,8 +80,6 @@ en: tags_instructions: "Tag this proposal. You can choose from proposed categories or add your own" tags_label: Tags tags_placeholder: "Enter the tags you would like to use, separated by commas (',')" - map_location: "Map location" - map_location_instructions: "Navigate the map to the location and place the marker." index: title: Participatory budgeting unfeasible: Unfeasible investment projects diff --git a/config/locales/en/general.yml b/config/locales/en/general.yml index 269452fb7..b54745690 100644 --- a/config/locales/en/general.yml +++ b/config/locales/en/general.yml @@ -328,6 +328,7 @@ en: tags_placeholder: "Enter the tags you would like to use, separated by commas (',')" map_location: "Map location" map_location_instructions: "Navigate the map to the location and place the marker." + map_marker_coordinates: "Latitude: %{latitude}. Longitude: %{longitude}" map_remove_marker: "Remove map marker" index: featured_proposals: Featured diff --git a/config/locales/es/budgets.yml b/config/locales/es/budgets.yml index 53c5a4b35..489752b81 100644 --- a/config/locales/es/budgets.yml +++ b/config/locales/es/budgets.yml @@ -80,8 +80,6 @@ es: tags_instructions: "Etiqueta este proyecto. Puedes elegir entre las categorías propuestas o introducir las que desees" tags_label: Etiquetas tags_placeholder: "Escribe las etiquetas que desees separadas por coma (',')" - map_location: "Ubicación en el mapa" - map_location_instructions: "Navega por el mapa hasta la ubicación y coloca el marcador." index: title: Presupuestos participativos unfeasible: Proyectos de gasto no viables diff --git a/config/locales/es/general.yml b/config/locales/es/general.yml index 9bcb5566a..514378598 100644 --- a/config/locales/es/general.yml +++ b/config/locales/es/general.yml @@ -328,6 +328,7 @@ es: tags_placeholder: "Escribe las etiquetas que desees separadas por coma (',')" map_location: "Ubicación en el mapa" map_location_instructions: "Navega por el mapa hasta la ubicación y coloca el marcador." + map_marker_coordinates: "Latitud: %{latitude}. Longitud: %{longitude}" map_remove_marker: "Eliminar el marcador" index: featured_proposals: Destacadas diff --git a/spec/components/admin/settings/map_form_component_spec.rb b/spec/components/admin/settings/map_form_component_spec.rb new file mode 100644 index 000000000..cd274efff --- /dev/null +++ b/spec/components/admin/settings/map_form_component_spec.rb @@ -0,0 +1,10 @@ +require "rails_helper" + +describe Admin::Settings::MapFormComponent do + it "does not render a button to remove the marker" do + render_inline Admin::Settings::MapFormComponent.new + + expect(page).to have_css ".map-location" + expect(page).not_to have_button "Remove map marker" + end +end diff --git a/spec/components/budgets/investments/form_component_spec.rb b/spec/components/budgets/investments/form_component_spec.rb index 22459cf09..dc3eb0e4b 100644 --- a/spec/components/budgets/investments/form_component_spec.rb +++ b/spec/components/budgets/investments/form_component_spec.rb @@ -33,4 +33,17 @@ describe Budgets::Investments::FormComponent do expect(page).not_to have_field "I agree to the Privacy Policy and the Terms and conditions of use" end end + + describe "map" do + it "renders a button to remove the map marker" do + Setting["feature.map"] = true + + render_inline Budgets::Investments::FormComponent.new( + budget.investments.new, + url: budget_investments_path(budget) + ) + + expect(page).to have_button "Remove map marker" + end + end end diff --git a/spec/components/map_locations/form_fields_component_spec.rb b/spec/components/map_locations/form_fields_component_spec.rb new file mode 100644 index 000000000..34c17d277 --- /dev/null +++ b/spec/components/map_locations/form_fields_component_spec.rb @@ -0,0 +1,24 @@ +require "rails_helper" + +describe MapLocations::FormFieldsComponent do + let(:proposal) { Proposal.new } + let(:map_location) { MapLocation.new } + let(:form) { ConsulFormBuilder.new(:proposal, proposal, ApplicationController.new.view_context, {}) } + let(:component) { MapLocations::FormFieldsComponent.new(form, map_location: map_location) } + + it "is rendered when the map feature is enabled" do + Setting["feature.map"] = true + + render_inline component + + expect(page).to be_rendered + end + + it "is not rendered when the map feature is not enabled" do + Setting["feature.map"] = false + + render_inline component + + expect(page).not_to be_rendered + end +end diff --git a/spec/components/proposals/form_component_spec.rb b/spec/components/proposals/form_component_spec.rb new file mode 100644 index 000000000..5c3272c75 --- /dev/null +++ b/spec/components/proposals/form_component_spec.rb @@ -0,0 +1,17 @@ +require "rails_helper" + +describe Proposals::FormComponent do + include Rails.application.routes.url_helpers + + before { sign_in(create(:user)) } + + describe "map" do + it "renders a button to remove the map marker" do + Setting["feature.map"] = true + + render_inline Proposals::FormComponent.new(Proposal.new, url: proposals_path) + + expect(page).to have_button "Remove map marker" + end + end +end diff --git a/spec/components/shared/map_location_component_spec.rb b/spec/components/shared/map_location_component_spec.rb new file mode 100644 index 000000000..f16a85e31 --- /dev/null +++ b/spec/components/shared/map_location_component_spec.rb @@ -0,0 +1,62 @@ +require "rails_helper" + +describe Shared::MapLocationComponent do + describe "remove marker button" do + it "is not rendered when there's no form" do + map_location = build(:map_location, proposal: Proposal.new) + + render_inline Shared::MapLocationComponent.new(map_location) + + expect(page).not_to have_button "Remove map marker" + end + + it "is not rendered when there's no mappable" do + map_location = build(:map_location) + form = ConsulFormBuilder.new(:map_location, map_location, ApplicationController.new.view_context, {}) + + render_inline Shared::MapLocationComponent.new(map_location, form: form) + + expect(page).not_to have_button "Remove map marker" + end + + it "is rendered when there's a form and a mappable" do + map_location = build(:map_location, proposal: Proposal.new) + form = ConsulFormBuilder.new(:map_location, map_location, ApplicationController.new.view_context, {}) + + render_inline Shared::MapLocationComponent.new(map_location, form: form) + + expect(page).to have_button "Remove map marker" + end + end + + describe "marker title" do + it "uses the coordinates when there's a mappable" do + map_location = build( + :map_location, + proposal: Proposal.new(title: "Meet me here"), + latitude: "25.25", + longitude: "13.14" + ) + + render_inline Shared::MapLocationComponent.new(map_location) + + expect(page).to have_css "[data-marker-title='Latitude: 25.25. Longitude: 13.14']" + end + + it "uses the coordinates when there's no mappable" do + map_location = build(:map_location, latitude: "25.25", longitude: "13.14") + + render_inline Shared::MapLocationComponent.new(map_location) + + expect(page).to have_css "[data-marker-title='Latitude: 25.25. Longitude: 13.14']" + end + + it "is not present when the map location isn't available" do + map_location = build(:map_location, latitude: "25.25", longitude: nil) + + render_inline Shared::MapLocationComponent.new(map_location) + + expect(page).not_to have_css "[data-marker-title]" + end + end +end diff --git a/spec/shared/system/mappable.rb b/spec/shared/system/mappable.rb index d2fca3089..1b130a46f 100644 --- a/spec/shared/system/mappable.rb +++ b/spec/shared/system/mappable.rb @@ -17,36 +17,21 @@ shared_examples "mappable" do |mappable_factory_name, mappable_association_name, describe "At #{mappable_new_path}" do before { set_arguments(arguments, mappable, mappable_path_arguments) } - scenario "Should not show marker by default on create #{mappable_factory_name}" do + scenario "Should show marker and create #{mappable_factory_name} with map" do do_login_for user, management: management visit send(mappable_new_path, arguments) send("fill_in_#{mappable_factory_name}") - within ".map-location" do - expect(page).not_to have_css(".map-icon") - end - end + within_fieldset "Map location" do + expect(page).not_to have_css ".map-icon" - scenario "Should show marker on create #{mappable_factory_name} when click on map" do - do_login_for user, management: management - visit send(mappable_new_path, arguments) + find("#new_map_location").click - send("fill_in_#{mappable_factory_name}") - find("#new_map_location").click - - within ".map-location" do expect(page).to have_css ".map-icon" expect(page).not_to have_css ".map-icon[aria-label]" end - end - scenario "Should create #{mappable_factory_name} with map" do - do_login_for user, management: management - visit send(mappable_new_path, arguments) - - send("fill_in_#{mappable_factory_name}") - find("#new_map_location").click send("submit_#{mappable_factory_name}_form") within ".map-location" do @@ -96,7 +81,7 @@ shared_examples "mappable" do |mappable_factory_name, mappable_association_name, go_back - within ".map-location" do + within_fieldset "Map location" do expect(page).to have_css(".leaflet-map-pane", count: 1) end end @@ -105,15 +90,16 @@ shared_examples "mappable" do |mappable_factory_name, mappable_association_name, do_login_for user, management: management visit send(mappable_new_path, arguments) - within ".map-location" do - expect(page).not_to have_css(".map-icon") + within_fieldset "Map location" do + expect(page).not_to have_css ".map-icon" end expect(page.execute_script("return App.Map.maps[0].getZoom();")).to eq(10) map_zoom_in - find("#new_map_location").click - within ".map-location" do + within_fieldset "Map location" do + find("#new_map_location").click + expect(page).to have_css(".map-icon") end @@ -129,9 +115,9 @@ shared_examples "mappable" do |mappable_factory_name, mappable_association_name, go_back - within ".map-location" do - expect(page).to have_css(".map-icon") - expect(page.execute_script("return App.Map.maps[0].getZoom();")).to eq(11) + within_fieldset "Map location" do + expect(page).to have_css ".map-icon" + expect(page.execute_script("return App.Map.maps[0].getZoom();")).to eq 11 end end @@ -139,14 +125,15 @@ shared_examples "mappable" do |mappable_factory_name, mappable_association_name, do_login_for user, management: management visit send(mappable_new_path, arguments) - within ".map-location" do - expect(page).not_to have_css(".map-icon") + within_fieldset "Map location" do + expect(page).not_to have_css ".map-icon" end place_map_at(-68.592487, -62.391357) - find("#new_map_location").click - within ".map-location" do + within_fieldset "Map location" do + find("#new_map_location").click + expect(page).to have_css(".map-icon") end @@ -162,8 +149,8 @@ shared_examples "mappable" do |mappable_factory_name, mappable_association_name, go_back - within ".map-location" do - expect(page).to have_css(".map-icon") + within_fieldset "Map location" do + expect(page).to have_css ".map-icon" end end end @@ -180,17 +167,27 @@ shared_examples "mappable" do |mappable_factory_name, mappable_association_name, end describe "At #{mappable_edit_path}", if: mappable_edit_path.present? do - scenario "Should edit map on #{mappable_factory_name} and contain default values" do + scenario "Should edit mappable on #{mappable_factory_name} without changing the map" do mappable.map_location.update!(latitude: 51.48, longitude: 0.0) + do_login_for mappable.author, management: management visit send(mappable_edit_path, id: mappable.id) expect(page).to have_content "Navigate the map to the location and place the marker." + expect(page).to have_css ".map-icon[aria-label='Latitude: 51.48. Longitude: 0.0']" expect(page).to have_field "#{mappable_factory_name}_map_location_attributes_latitude", type: :hidden, with: "51.48" expect(page).to have_field "#{mappable_factory_name}_map_location_attributes_longitude", type: :hidden, with: "0.0" + + fill_in "#{mappable_factory_name.camelize} title", with: "New title" + click_button "Save changes" + + expect(page).not_to have_button "Save changes" + expect(page).to have_css ".map-location" + expect(page).to have_css ".map-location[data-marker-latitude='51.48']" + expect(page).to have_css ".map-location[data-marker-longitude='0.0']" end scenario "Should edit default values from map on #{mappable_factory_name} edit page" do @@ -200,7 +197,10 @@ shared_examples "mappable" do |mappable_factory_name, mappable_association_name, do_login_for mappable.author, management: management visit send(mappable_edit_path, id: mappable.id) - find(".map-location").click(x: 30, y: 30) + + within_fieldset "Map location" do + find(".map-location").click(x: 30, y: 30) + end new_latitude = find_field( "#{mappable_factory_name}_map_location_attributes_latitude", type: :hidden @@ -219,31 +219,16 @@ shared_examples "mappable" do |mappable_factory_name, mappable_association_name, expect(page).not_to have_css ".map-location[data-marker-longitude='#{original_longitude}']" end - scenario "Should edit mappable on #{mappable_factory_name} without change map" do - original_longitude = map_location.longitude - original_latitude = map_location.latitude - - do_login_for mappable.author, management: management - - visit send(mappable_edit_path, id: mappable.id) - fill_in "#{mappable_factory_name.camelize} title", with: "New title" - click_button "Save changes" - - expect(page).not_to have_button "Save changes" - expect(page).to have_css ".map-location" - expect(page).to have_css ".map-location[data-marker-latitude='#{original_latitude}']" - expect(page).to have_css ".map-location[data-marker-longitude='#{original_longitude}']" - end - scenario "Can not display map on #{mappable_factory_name} edit when remove map marker" do do_login_for mappable.author, management: management visit send(mappable_edit_path, id: mappable.id) - click_button "Remove map marker" + within_fieldset("Map location") { click_button "Remove map marker" } click_button "Save changes" expect(page).not_to have_button "Save changes" expect(page).not_to have_css ".map-location" + expect(page).not_to have_content "Map location can't be blank" end scenario "Can not display map on #{mappable_factory_name} edit when feature.map is disabled" do @@ -257,17 +242,6 @@ shared_examples "mappable" do |mappable_factory_name, mappable_association_name, expect(page).not_to have_button "Save changes" expect(page).not_to have_css ".map-location" end - - scenario "No need to skip map on update" do - do_login_for mappable.author, management: management - - visit send(mappable_edit_path, id: mappable.id) - click_button "Remove map marker" - click_button "Save changes" - - expect(page).not_to have_button "Save changes" - expect(page).not_to have_content "Map location can't be blank" - end end describe "At #{mappable_show_path}" do @@ -282,8 +256,10 @@ shared_examples "mappable" do |mappable_factory_name, mappable_association_name, do_login_for user, management: management if management visit send(mappable_show_path, arguments) + label = "Latitude: #{map_location.latitude}. Longitude: #{map_location.longitude}" + within ".map-location" do - expect(page).to have_css ".map-icon[aria-label='#{mappable.title}']" + expect(page).to have_css ".map-icon[aria-label='#{label}']" end end diff --git a/spec/system/admin/settings_spec.rb b/spec/system/admin/settings_spec.rb index 83c4369b3..5917591e6 100644 --- a/spec/system/admin/settings_spec.rb +++ b/spec/system/admin/settings_spec.rb @@ -21,22 +21,16 @@ describe "Admin settings", :admin do end describe "Map settings initialization" do - before do + scenario "Map is only initialized when the map settings tab content is shown" do Setting["feature.map"] = true - end - scenario "When `Map settings` tab content is hidden map should not be initialized" do visit admin_settings_path - expect(page).not_to have_css("#admin-map.leaflet-container", visible: :all) - end - - scenario "When `Map settings` tab content is shown map should be initialized" do - visit admin_settings_path + expect(page).not_to have_css ".map-location.leaflet-container", visible: :all click_link "Map configuration" - expect(page).to have_css("#admin-map.leaflet-container") + expect(page).to have_css ".map-location.leaflet-container" end end @@ -50,41 +44,7 @@ describe "Admin settings", :admin do expect(page).to have_content "To show the map to users you must enable " \ '"Proposals and budget investments geolocation" ' \ 'on "Features" tab.' - expect(page).not_to have_css("#admin-map") - end - - scenario "Should be able when map feature activated" do - Setting["feature.map"] = true - - visit admin_settings_path - click_link "Map configuration" - - expect(page).to have_css("#admin-map") - expect(page).not_to have_content "To show the map to users you must enable " \ - '"Proposals and budget investments geolocation" ' \ - 'on "Features" tab.' - end - - scenario "Should show successful notice" do - Setting["feature.map"] = true - - visit admin_settings_path - click_link "Map configuration" - - within "#map-form" do - click_button "Update" - end - - expect(page).to have_content "Map configuration updated successfully" - end - - scenario "Should display marker by default" do - Setting["feature.map"] = true - - visit admin_settings_path - - expect(find("#latitude", visible: :hidden).value).to eq "51.48" - expect(find("#longitude", visible: :hidden).value).to eq "0.0" + expect(page).not_to have_css ".map-location" end scenario "Should update marker" do @@ -92,13 +52,26 @@ describe "Admin settings", :admin do visit admin_settings_path click_link "Map configuration" - find("#admin-map").click + + expect(page).to have_css ".map-location" + expect(page).not_to have_content "To show the map to users you must enable " \ + '"Proposals and budget investments geolocation" ' \ + 'on "Features" tab.' + + expect(page).to have_field "Latitude", with: "51.48" + expect(page).to have_field "Longitude", with: "0.0" + expect(page).to have_css ".map-icon[aria-label='Latitude: 51.48. Longitude: 0.0']" + within "#map-form" do + find(".map-location").click click_button "Update" end - expect(find("#latitude", visible: :hidden).value).not_to eq "51.48" expect(page).to have_content "Map configuration updated successfully" + expect(page).to have_field "Latitude" + expect(page).to have_css ".map-icon" + expect(page).not_to have_field "Latitude", with: "51.48" + expect(page).not_to have_css ".map-icon[aria-label='Latitude: 51.48. Longitude: 0.0']" end end diff --git a/spec/system/budgets/budgets_spec.rb b/spec/system/budgets/budgets_spec.rb index b21d9a4aa..e272a931b 100644 --- a/spec/system/budgets/budgets_spec.rb +++ b/spec/system/budgets/budgets_spec.rb @@ -305,9 +305,8 @@ describe "Budgets" do { lat: map_location[:latitude], long: map_location[:longitude], - investment_title: investment.title, - investment_id: investment.id, - budget_id: budget.id + title: investment.title, + link: "/budgets/#{budget.id}/investments/#{investment.id}" } end @@ -316,7 +315,8 @@ describe "Budgets" do visit budgets_path within ".map-location" do - expect(page).to have_css(".map-icon", count: 1, visible: :all) + expect(page).to have_css ".map-icon", count: 1, visible: :all + expect(page).to have_css ".map-icon[aria-label='#{investment.title}']", visible: :all end end