Skip to content

Commit

Permalink
Fix specs / Fix lint
Browse files Browse the repository at this point in the history
  • Loading branch information
JoonasAapro committed Nov 26, 2024
1 parent 975cb67 commit 6aa7808
Show file tree
Hide file tree
Showing 18 changed files with 107 additions and 126 deletions.
7 changes: 6 additions & 1 deletion Gemfile
Original file line number Diff line number Diff line change
Expand Up @@ -23,8 +23,13 @@ gem "uglifier", "~> 4.1"
group :development, :test do
gem "byebug", "~> 11.0", platform: :mri
gem "decidim-dev", DECIDIM_VERSION

# rubocop & rubocop-rspec are set to the following versions because of a change where FactoryBot/CreateList
# must be a boolean instead of contextual. These version locks can be removed when this problem is handled
# through decidim-dev.
gem "rubocop", "~>1.28"
gem "rubocop-faker"
gem "rubocop-performance", "~> 1.15.0" # ?
gem "rubocop-rspec", "2.20"
end

group :development do
Expand Down
16 changes: 4 additions & 12 deletions Gemfile.lock
Original file line number Diff line number Diff line change
Expand Up @@ -665,26 +665,17 @@ GEM
parser (>= 3.3.1.0)
rubocop-capybara (2.21.0)
rubocop (~> 1.41)
rubocop-factory_bot (2.26.0)
rubocop (~> 1.41)
rubocop-faker (1.2.0)
faker (>= 2.12.0)
rubocop (>= 1.13.0)
rubocop-performance (1.15.2)
rubocop (>= 1.7.0, < 2.0)
rubocop-ast (>= 0.4.0)
rubocop-rails (2.25.1)
activesupport (>= 4.2.0)
rack (>= 1.1)
rubocop (>= 1.33.0, < 2.0)
rubocop-ast (>= 1.31.1, < 2.0)
rubocop-rspec (2.31.0)
rubocop (~> 1.40)
rubocop-rspec (2.20.0)
rubocop (~> 1.33)
rubocop-capybara (~> 2.17)
rubocop-factory_bot (~> 2.22)
rubocop-rspec_rails (~> 2.28)
rubocop-rspec_rails (2.29.0)
rubocop (~> 1.40)
ruby-progressbar (1.13.0)
ruby-vips (2.2.2)
ffi (~> 1.12)
Expand Down Expand Up @@ -804,8 +795,9 @@ DEPENDENCIES
listen (~> 3.1)
puma (>= 6.4.2)
rgeo-geojson (~> 2.1, >= 2.1.1)
rubocop (~> 1.28)
rubocop-faker
rubocop-performance (~> 1.15.0)
rubocop-rspec (= 2.20)
spring (~> 2.0)
spring-watcher-listen (~> 2.0)
uglifier (~> 4.1)
Expand Down
2 changes: 1 addition & 1 deletion app/cells/decidim/locations/map_cell.rb
Original file line number Diff line number Diff line change
Expand Up @@ -100,7 +100,7 @@ def markers_data_for_map
{
id: data[0],
title: data[1],
body: body,
body:,
address: data[4],
latitude: data[5],
longitude: data[6],
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,7 @@ def update_locations(locatable, form)
Decidim::Locations::Location.new(attributes)
end
end.compact
locatable.update!(locations: locations)
locatable.update!(locations:)
end
end
end
Expand Down
2 changes: 1 addition & 1 deletion app/forms/concerns/decidim/locations/locatable_form.rb
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,7 @@ def validate_locations
return
end

record = self.class.locations_record_class.find_by(id: id)
record = self.class.locations_record_class.find_by(id:)
return unless record

unexisting_ids = provided_ids.map(&:to_i) - record.locations.pluck(:id)
Expand Down
2 changes: 1 addition & 1 deletion app/forms/decidim/locations/location_form.rb
Original file line number Diff line number Diff line change
Expand Up @@ -49,7 +49,7 @@ def json_validation
# check if GeoJSON is valid
begin
geo_factory = RGeo::Geographic.spherical_factory
RGeo::GeoJSON.decode(geojson, geo_factory: geo_factory)
RGeo::GeoJSON.decode(geojson, geo_factory:)

coord_validation
rescue JSON::ParserError
Expand Down
15 changes: 15 additions & 0 deletions app/packs/src/decidim/map/provider/default.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@
import "leaflet"
import "src/decidim/geoman/leaflet-geoman.min.js";

/**
* NOTE:
* This has to load before decidim/map in order for it to apply correctly when
* the map is initialized. The document.ready handler set by this script has to
* be registered before decidim/map registers its own.
*/
$(() => {
$("[data-decidim-map]").on("configure.decidim", (_ev, map, mapConfig) => {
const tilesConfig = mapConfig.tileLayer;
L.tileLayer(tilesConfig.url, tilesConfig.options).addTo(map);
});
});
1 change: 0 additions & 1 deletion config/assets.rb
Original file line number Diff line number Diff line change
Expand Up @@ -13,4 +13,3 @@
Decidim::Webpacker.register_stylesheet_import("src/decidim/geoman/leaflet-geoman")
Decidim::Webpacker.register_stylesheet_import("stylesheets/decidim/locations/locations", group: :admin)
Decidim::Webpacker.register_stylesheet_import("src/decidim/geoman/leaflet-geoman", group: :admin)

4 changes: 2 additions & 2 deletions spec/cells/decidim/locations/form_cell_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -7,10 +7,10 @@

let(:my_cell) { cell("decidim/locations/form", form_builder) }
let!(:organization) { create(:organization) }
let(:user) { create(:user, :confirmed, organization: organization) }
let(:user) { create(:user, :confirmed, organization:) }
let(:template_class) do
Class.new(ActionView::Base) do
include ::Cell::RailsExtensions::ActionView
include Cell::RailsExtensions::ActionView

delegate :snippets, to: :controller

Expand Down
14 changes: 7 additions & 7 deletions spec/cells/decidim/locations/locations_cell_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -5,14 +5,14 @@
describe Decidim::Locations::LocationsCell, type: :cell do
subject { my_cell.call }

let(:my_cell) { cell("decidim/locations/locations", dummy, form: form, map_configuration: "single", coords: [1, 2]) }
let(:my_cell) { cell("decidim/locations/locations", dummy, form:, map_configuration: "single", coords: [1, 2]) }
let(:dummy_form) { Decidim::Dev::DummyResourceForm.from_model(dummy) }
let(:form) { Decidim::FormBuilder.new("dummy", dummy_form, template, {}) }
let!(:organization) { create(:organization) }
let(:user) { create(:user, :confirmed, organization: organization) }
let(:user) { create(:user, :confirmed, organization:) }
let(:template_class) do
Class.new(ActionView::Base) do
include ::Cell::RailsExtensions::ActionView
include Cell::RailsExtensions::ActionView

delegate :snippets, :current_organization, to: :controller
end
Expand All @@ -30,17 +30,17 @@

context "when cell called" do
before do
utility = Decidim::Map.autocomplete(organization: organization)
allow(Decidim::Map).to receive(:autocomplete).with(organization: organization).and_return(utility)
utility = Decidim::Map.autocomplete(organization:)
allow(Decidim::Map).to receive(:autocomplete).with(organization:).and_return(utility)
allow(utility).to receive(:builder_options).and_return(
api_key: "key1234"
)
allow(my_cell).to receive(:random_id).and_return("example")
end

it "renders the view" do
expect(subject).to have_css("input[name=\"dummy[address]\"]")
expect(subject).to have_selector("#example")
expect(subject).to have_field("dummy[address]")
expect(subject).to have_css("#example")
expect(subject).to have_content(
<<~TXT.squish
The following element is a map which presents the items on this page as map points.
Expand Down
6 changes: 3 additions & 3 deletions spec/cells/decidim/locations/map_cell_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -7,10 +7,10 @@

let(:my_cell) { cell("decidim/locations/map", dummy) }
let!(:organization) { create(:organization) }
let(:user) { create(:user, :confirmed, organization: organization) }
let(:user) { create(:user, :confirmed, organization:) }
let(:template_class) do
Class.new(ActionView::Base) do
include ::Cell::RailsExtensions::ActionView
include Cell::RailsExtensions::ActionView

delegate :snippets, to: :controller
end
Expand All @@ -34,7 +34,7 @@
The element can be used with a screen reader but it may be hard to understand.
TXT
)
expect(subject).to have_css(".map__help")
expect(subject).to have_css(".map__skip")
expect(subject).to have_css("[id$='_bottom']")
end
end
Expand Down
4 changes: 2 additions & 2 deletions spec/commands/decidim/locations/locations_command_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -6,14 +6,14 @@ module Decidim
module Locations
describe LocationsCommand do
let!(:organization) { create(:organization) }
let(:user) { create(:user, :admin, :confirmed, organization: organization) }
let(:user) { create(:user, :admin, :confirmed, organization:) }
let(:dummy) { create(:dummy_resource) }
let!(:form_klass) { Decidim::Dev::DummyResourceForm }
let(:form_params) do
{
title: "This title has to be at least 15 chars",
body: "This body has to be at least 15 chars",
locations: locations
locations:
}
end

Expand Down
24 changes: 12 additions & 12 deletions spec/helpers/decidim/locations/locations_helper_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -6,10 +6,10 @@ module Decidim
module Locations
describe LocationsHelper do
let!(:organization) { create(:organization) }
let!(:dummy1) { create(:dummy_resource, body: "A reasonable body") }
let!(:dummy2) { create(:dummy_resource, body: "Another reasonable body") }
let!(:loc1) { create(:location, locatable: dummy1, address: "Speed street", latitude: 50.231241, longitude: 39.394056, shape: "Marker", geojson: { "lat" => 50.231241, "lng" => 39.394056 }) }
let!(:loc2) { create(:location, locatable: dummy2, address: "Brain boulevard", latitude: 14.284756, longitude: 43.182746, shape: "Marker", geojson: { "lat" => 14.284756, "lng" => 43.182746 }) }
let!(:dummy_one) { create(:dummy_resource, body: "A reasonable body") }
let!(:dummy_two) { create(:dummy_resource, body: "Another reasonable body") }
let!(:loc_one) { create(:location, locatable: dummy_one, address: "Speed street", latitude: 50.231241, longitude: 39.394056, shape: "Marker", geojson: { "lat" => 50.231241, "lng" => 39.394056 }) }
let!(:loc_two) { create(:location, locatable: dummy_two, address: "Brain boulevard", latitude: 14.284756, longitude: 43.182746, shape: "Marker", geojson: { "lat" => 14.284756, "lng" => 43.182746 }) }
let(:helper) do
Class.new(ActionView::Base) do
include TranslatableAttributes
Expand All @@ -18,39 +18,39 @@ module Locations
end

describe "#format_map_locations - single" do
subject { helper.format_map_locations(dummy1) }
subject { helper.format_map_locations(dummy_one) }

it "returns an array of a single model locations" do
expect(subject).to eq([[dummy1.id, dummy1.title["en"], nil, dummy1.body, loc1.address, loc1.latitude, loc1.longitude, "Marker", { "lat" => 50.231241, "lng" => 39.394056 }]])
expect(subject).to eq([[dummy_one.id, dummy_one.title["en"], nil, dummy_one.body, loc_one.address, loc_one.latitude, loc_one.longitude, "Marker", { "lat" => 50.231241, "lng" => 39.394056 }]])
end
end

describe "#format_map_locations - multiple" do
let(:dummies) { Decidim::Dev::DummyResource.where(id: [dummy1.id, dummy2.id]) }
let(:dummies) { Decidim::Dev::DummyResource.where(id: [dummy_one.id, dummy_two.id]) }

subject { helper.format_map_locations(dummies) }

it "returns an array of multiple model locations" do
expect(subject).to include(
[dummy1.id, dummy1.title["en"], nil, dummy1.body, loc1.address, loc1.latitude, loc1.longitude, "Marker", { "lat" => 50.231241, "lng" => 39.394056 }],
[dummy2.id, dummy2.title["en"], nil, dummy2.body, loc2.address, loc2.latitude, loc2.longitude, "Marker", { "lat" => 14.284756, "lng" => 43.182746 }]
[dummy_one.id, dummy_one.title["en"], nil, dummy_one.body, loc_one.address, loc_one.latitude, loc_one.longitude, "Marker", { "lat" => 50.231241, "lng" => 39.394056 }],
[dummy_two.id, dummy_two.title["en"], nil, dummy_two.body, loc_two.address, loc_two.latitude, loc_two.longitude, "Marker", { "lat" => 14.284756, "lng" => 43.182746 }]
)
end
end

describe "#model_has_address?" do
context "when model has address" do
subject { model_has_address?(dummy1) }
subject { model_has_address?(dummy_one) }

it "returns true" do
expect(subject).to be(true)
end
end

context "when model has no address" do
let(:dummy3) { create(:dummy_resource) }
let(:dummy_three) { create(:dummy_resource) }

subject { model_has_address?(dummy3) }
subject { model_has_address?(dummy_three) }

it "returns false" do
expect(subject).to be(false)
Expand Down
2 changes: 1 addition & 1 deletion spec/shared/map_cell_render_examples.rb
Original file line number Diff line number Diff line change
Expand Up @@ -72,7 +72,7 @@

it "renders the map with 3 shapes" do
expect(page).to have_css(".leaflet-marker-icon", count: 1)
expect(page).to have_selector('img[title="Title of the marker"]')
expect(page).to have_css('img[title="Title of the marker"]')
expect(page).to have_css("path.leaflet-interactive", count: 2)
end

Expand Down
24 changes: 12 additions & 12 deletions spec/shared/reverse_geocoding_examples.rb
Original file line number Diff line number Diff line change
Expand Up @@ -170,8 +170,8 @@
add_marker
expect(page).to have_css(".leaflet-marker-icon", visible: :all)
find(".leaflet-marker-icon").click
click_button "Delete shape"
expect(page).not_to have_css(".leaflet-marker-icon")
click_on "Delete shape"
expect(page).to have_no_css(".leaflet-marker-icon")
end
end

Expand All @@ -181,8 +181,8 @@
add_line
expect(page).to have_css(".leaflet-interactive")
find(".leaflet-interactive").click
click_button "Delete shape"
expect(page).not_to have_css(".leaflet-interactive")
click_on "Delete shape"
expect(page).to have_no_css(".leaflet-interactive")
end
end

Expand All @@ -192,8 +192,8 @@
add_polygon
expect(page).to have_css(".leaflet-interactive")
find(".leaflet-interactive").click
click_button "Delete shape"
expect(page).not_to have_css(".leaflet-interactive")
click_on "Delete shape"
expect(page).to have_no_css(".leaflet-interactive")
end
end
end
Expand All @@ -206,7 +206,7 @@
expect(page).to have_css(".leaflet-marker-icon", visible: :all)
find('div[title="Remove Layers"] a').click
find(".leaflet-marker-icon").click
expect(page).not_to have_css(".leaflet-marker-icon")
expect(page).to have_no_css(".leaflet-marker-icon")
end
end

Expand All @@ -217,7 +217,7 @@
expect(page).to have_css(".leaflet-interactive")
find('div[title="Remove Layers"] a').click
find(".leaflet-interactive").click
expect(page).not_to have_css(".leaflet-interactive")
expect(page).to have_no_css(".leaflet-interactive")
end
end

Expand All @@ -228,7 +228,7 @@
expect(page).to have_css(".leaflet-interactive")
find('div[title="Remove Layers"] a').click
find(".leaflet-interactive").click
expect(page).not_to have_css(".leaflet-interactive")
expect(page).to have_no_css(".leaflet-interactive")
end
end
end
Expand All @@ -241,7 +241,7 @@
add_marker(latitude: 11.523, longitude: 5.523)
expect(page).to have_css(".leaflet-marker-icon", count: 2)
find(".leaflet-marker-icon", match: :first).click
click_button "Delete shape"
click_on "Delete shape"
expect(page).to have_css(".leaflet-marker-icon", count: 1)
end
end
Expand All @@ -253,7 +253,7 @@
add_line(latitude: [60.25240524264372, 60.25116980538645], longitude: [25.10409421539333, 25.104342109680122])
expect(page).to have_css(".leaflet-interactive", count: 2)
find(".leaflet-interactive", match: :first).click
click_button "Delete shape"
click_on "Delete shape"
expect(page).to have_css(".leaflet-interactive", count: 1)
end
end
Expand All @@ -265,7 +265,7 @@
add_polygon(latitude: [85.05109772344434, 85.05101347805167, 85.05106165693364], longitude: [24.741211170645243, 24.7438833903077, 24.74355157804477])
expect(page).to have_css(".leaflet-interactive", count: 2)
find(".leaflet-interactive", match: :first).click
click_button "Delete shape"
click_on "Delete shape"
expect(page).to have_css(".leaflet-interactive", count: 1)
end
end
Expand Down
Loading

0 comments on commit 6aa7808

Please sign in to comment.