From 097a15d6244b1ec740d94d3f3767e8a6d1a07ccb Mon Sep 17 00:00:00 2001 From: Paul Chavard Date: Thu, 29 Nov 2018 23:10:13 +0100 Subject: [PATCH 1/3] Test and fix all possible cases of champ carte serialization --- app/models/champs/carte_champ.rb | 2 +- app/serializers/champ_serializer.rb | 8 +- spec/serializers/champ_serializer_spec.rb | 140 ++++++++++++++++------ 3 files changed, 108 insertions(+), 42 deletions(-) diff --git a/app/models/champs/carte_champ.rb b/app/models/champs/carte_champ.rb index 7eff98481..f2857767a 100644 --- a/app/models/champs/carte_champ.rb +++ b/app/models/champs/carte_champ.rb @@ -51,7 +51,7 @@ class Champs::CarteChamp < Champ # We used to store in the value column a json array with coordinates. if geo_json.is_a?(Array) # If it is a coordinates array, format it as a GEO-JSON - GeojsonService.to_json_polygon_for_selection_utilisateur(geo_json) + JSON.parse(GeojsonService.to_json_polygon_for_selection_utilisateur(geo_json)) else # It is already a GEO-JSON geo_json diff --git a/app/serializers/champ_serializer.rb b/app/serializers/champ_serializer.rb index c6c30a3f9..2b76d8b70 100644 --- a/app/serializers/champ_serializer.rb +++ b/app/serializers/champ_serializer.rb @@ -12,11 +12,9 @@ class ChampSerializer < ActiveModel::Serializer def value case object when GeoArea - object.geometry + object.geometry.to_json when Champs::CarteChamp - if object.value.present? - JSON.parse(object.value) - end + object.user_geometry.to_json when Champs::DecimalNumberChamp if object.value.present? object.value.to_f @@ -71,7 +69,7 @@ class ChampSerializer < ActiveModel::Serializer libelle: legacy_carto_libelle, type_champ: legacy_carto_type_champ, order_place: -1, - descripton: '' + description: '' } end diff --git a/spec/serializers/champ_serializer_spec.rb b/spec/serializers/champ_serializer_spec.rb index 1c82e296a..c2a92f3f0 100644 --- a/spec/serializers/champ_serializer_spec.rb +++ b/spec/serializers/champ_serializer_spec.rb @@ -1,6 +1,7 @@ describe ChampSerializer do describe '#attributes' do - subject { ChampSerializer.new(champ).serializable_hash } + subject { ChampSerializer.new(serializable_object).serializable_hash } + let(:serializable_object) { champ } context 'when type champ is piece justificative' do include Rails.application.routes.url_helpers @@ -20,51 +21,118 @@ describe ChampSerializer do end context 'when type champ is carte' do - let(:geo_area) { create(:geo_area) } - let(:coordinates) { [[{ "lat": 48.87442541960633, "lng": 2.3859214782714844 }, { "lat": 48.87273183590832, "lng": 2.3850631713867183 }, { "lat": 48.87081237174292, "lng": 2.3809432983398438 }, { "lat": 48.8712640169951, "lng": 2.377510070800781 }, { "lat": 48.87510283703279, "lng": 2.3778533935546875 }, { "lat": 48.87544154230615, "lng": 2.382831573486328 }, { "lat": 48.87442541960633, "lng": 2.3859214782714844 }]] } + let(:champ) { create(:champ_carte, value: value, geo_areas: [geo_area].compact) } + let(:value) { nil } + let(:geo_area) { create(:geo_area, geometry: parsed_geo_json) } + let(:parsed_geo_json) { JSON.parse(geo_json) } + let(:geo_json) { GeojsonService.to_json_polygon_for_selection_utilisateur(coordinates) } + let(:coordinates) { [[{ "lat" => 48.87442541960633, "lng" => 2.3859214782714844 }, { "lat" => 48.87273183590832, "lng" => 2.3850631713867183 }, { "lat" => 48.87081237174292, "lng" => 2.3809432983398438 }, { "lat" => 48.8712640169951, "lng" => 2.377510070800781 }, { "lat" => 48.87510283703279, "lng" => 2.3778533935546875 }, { "lat" => 48.87544154230615, "lng" => 2.382831573486328 }, { "lat" => 48.87442541960633, "lng" => 2.3859214782714844 }]] } - let(:champ_carte) { create(:champ_carte, value: coordinates.to_json, geo_areas: [geo_area]) } - let(:champ) { champ_carte } + let(:serialized_champ) { + { + type_de_champ: { + description: serialized_description, + id: serialized_id, + libelle: serialized_libelle, + order_place: serialized_order_place, + type_champ: serialized_type_champ + }, + geo_areas: serialized_geo_areas, + value: geo_json + }.compact + } + let(:serialized_id) { -1 } + let(:serialized_description) { "" } + let(:serialized_order_place) { -1 } + let(:serialized_geo_areas) { nil } - context 'legacy champ user_geometry' do - let(:champ) { champ_carte.user_geo_area } + context 'and geo_area is selection_utilisateur' do + context 'old_api' do + let(:serialized_libelle) { "user geometry" } + let(:serialized_type_champ) { "user_geometry" } - it { - expect(subject).to include( - type_de_champ: { - descripton: "", - id: -1, - libelle: "user geometry", - order_place: -1, - type_champ: "user_geometry" - }, - value: champ_carte.user_geometry - ) - } + let(:serializable_object) { champ.user_geo_area } + + context 'when value is coordinates' do + let(:value) { coordinates.to_json } + + it { expect(subject).to eq(serialized_champ) } + end + + context 'when value is geojson' do + let(:value) { geo_json } + + it { expect(subject).to eq(serialized_champ) } + end + end + + context 'new_api' do + let(:geo_area) { nil } + let(:serialized_geo_areas) { [] } + let(:serialized_id) { champ.type_de_champ.stable_id } + let(:serialized_description) { champ.description } + let(:serialized_order_place) { champ.order_place } + let(:serialized_libelle) { champ.libelle } + let(:serialized_type_champ) { champ.type_champ } + + context 'when value is coordinates' do + let(:value) { coordinates.to_json } + + it { expect(subject).to eq(serialized_champ) } + end + + context 'when value is geojson' do + let(:value) { geo_json } + + it { expect(subject).to eq(serialized_champ) } + end + end end context 'and geo_area is cadastre' do - it { - expect(subject[:geo_areas].first).to include( - source: GeoArea.sources.fetch(:cadastre), - numero: '42', - feuille: 'A11' - ) - expect(subject[:geo_areas].first.key?(:nom)).to be_falsey - } + context 'new_api' do + it { + expect(subject[:geo_areas].first).to include( + source: GeoArea.sources.fetch(:cadastre), + geometry: parsed_geo_json, + numero: '42', + feuille: 'A11' + ) + expect(subject[:geo_areas].first.key?(:nom)).to be_falsey + } + end + + context 'old_api' do + let(:serializable_object) { champ.geo_areas.first } + let(:serialized_libelle) { "cadastre" } + let(:serialized_type_champ) { "cadastre" } + + it { expect(subject).to eq(serialized_champ) } + end end context 'and geo_area is quartier_prioritaire' do - let(:geo_area) { create(:geo_area, :quartier_prioritaire) } + let(:geo_area) { create(:geo_area, :quartier_prioritaire, geometry: parsed_geo_json) } - it { - expect(subject[:geo_areas].first).to include( - source: GeoArea.sources.fetch(:quartier_prioritaire), - nom: 'XYZ', - commune: 'Paris' - ) - expect(subject[:geo_areas].first.key?(:numero)).to be_falsey - } + context 'new_api' do + it { + expect(subject[:geo_areas].first).to include( + source: GeoArea.sources.fetch(:quartier_prioritaire), + geometry: parsed_geo_json, + nom: 'XYZ', + commune: 'Paris' + ) + expect(subject[:geo_areas].first.key?(:numero)).to be_falsey + } + end + + context 'old_api' do + let(:serializable_object) { champ.geo_areas.first } + let(:serialized_libelle) { "quartier prioritaire" } + let(:serialized_type_champ) { "quartier_prioritaire" } + + it { expect(subject).to eq(serialized_champ) } + end end end From d77a5c9f15b5184558f931173bc9eb238a692089 Mon Sep 17 00:00:00 2001 From: Paul Chavard Date: Fri, 30 Nov 2018 13:19:19 +0100 Subject: [PATCH 2/3] More tests and fixes on carte champ --- app/helpers/champ_helper.rb | 8 +- app/javascript/shared/carte.js | 30 +++++--- app/models/champs/carte_champ.rb | 36 ++++++--- app/serializers/champ_serializer.rb | 4 +- .../champs/carte_controller_spec.rb | 2 +- spec/models/champs/carte_champ_spec.rb | 63 ++++++++++++++++ spec/serializers/champ_serializer_spec.rb | 73 ++++++++++++++++--- 7 files changed, 172 insertions(+), 44 deletions(-) create mode 100644 spec/models/champs/carte_champ_spec.rb diff --git a/app/helpers/champ_helper.rb b/app/helpers/champ_helper.rb index 7f3df63d2..573a8defe 100644 --- a/app/helpers/champ_helper.rb +++ b/app/helpers/champ_helper.rb @@ -6,13 +6,7 @@ module ChampHelper def geo_data(champ) # rubocop:disable Rails/OutputSafety - raw({ - position: champ.position, - selection: champ.value.present? ? JSON.parse(champ.value) : [], - quartiersPrioritaires: champ.quartiers_prioritaires? ? champ.quartiers_prioritaires.as_json(except: :properties) : [], - cadastres: champ.cadastres? ? champ.cadastres.as_json(except: :properties) : [], - parcellesAgricoles: champ.parcelles_agricoles? ? champ.parcelles_agricoles.as_json(except: :properties) : [] - }.to_json) + raw(champ.to_render_data.to_json) # rubocop:enable Rails/OutputSafety end end diff --git a/app/javascript/shared/carte.js b/app/javascript/shared/carte.js index a9360e829..b026a9fff 100644 --- a/app/javascript/shared/carte.js +++ b/app/javascript/shared/carte.js @@ -71,23 +71,23 @@ export function drawParcellesAgricoles( } export function drawUserSelection(map, { selection }, editable = false) { - let hasSelection = selection && selection.length > 0; + if (selection) { + const coordinates = toLatLngs(selection); - if (editable) { - if (hasSelection) { - selection.forEach(polygon => map.freeDraw.create(polygon)); - let polygon = map.freeDraw.all()[0]; + if (editable) { + coordinates.forEach(polygon => map.freeDraw.create(polygon)); + const polygon = map.freeDraw.all()[0]; if (polygon) { map.fitBounds(polygon.getBounds()); } - } - } else if (hasSelection) { - const polygon = L.polygon(selection, { - color: 'red', - zIndex: 3 - }).addTo(map); + } else { + const polygon = L.polygon(coordinates, { + color: 'red', + zIndex: 3 + }).addTo(map); - map.fitBounds(polygon.getBounds()); + map.fitBounds(polygon.getBounds()); + } } } @@ -125,6 +125,12 @@ export function addFreeDrawEvents(map, selector) { }); } +function toLatLngs({ coordinates }) { + return coordinates.map(polygon => + polygon[0].map(point => ({ lng: point[0], lat: point[1] })) + ); +} + function findInput(selector) { return typeof selector === 'string' ? document.querySelector(selector) diff --git a/app/models/champs/carte_champ.rb b/app/models/champs/carte_champ.rb index f2857767a..8e5c6bb3d 100644 --- a/app/models/champs/carte_champ.rb +++ b/app/models/champs/carte_champ.rb @@ -44,24 +44,38 @@ class Champs::CarteChamp < Champ end def geo_json - @geo_json ||= value.blank? ? nil : JSON.parse(value) + @geo_json ||= begin + parsed_value = value.blank? ? nil : JSON.parse(value) + # We used to store in the value column a json array with coordinates. + if parsed_value.is_a?(Array) + # Empty array is sent instead of blank to distinguish between empty and error + if parsed_value.empty? + nil + else + # If it is a coordinates array, format it as a GEO-JSON + JSON.parse(GeojsonService.to_json_polygon_for_selection_utilisateur(parsed_value)) + end + else + # It is already a GEO-JSON + parsed_value + end + end end - def user_geometry - # We used to store in the value column a json array with coordinates. - if geo_json.is_a?(Array) - # If it is a coordinates array, format it as a GEO-JSON - JSON.parse(GeojsonService.to_json_polygon_for_selection_utilisateur(geo_json)) - else - # It is already a GEO-JSON - geo_json - end + def to_render_data + { + position: position, + selection: geo_json, + quartiersPrioritaires: quartiers_prioritaires? ? quartiers_prioritaires.as_json(except: :properties) : [], + cadastres: cadastres? ? cadastres.as_json(except: :properties) : [], + parcellesAgricoles: parcelles_agricoles? ? parcelles_agricoles.as_json(except: :properties) : [] + } end def user_geo_area if geo_json.present? GeoArea.new( - geometry: user_geometry, + geometry: geo_json, source: GeoArea.sources.fetch(:selection_utilisateur) ) end diff --git a/app/serializers/champ_serializer.rb b/app/serializers/champ_serializer.rb index 2b76d8b70..c880cf2bb 100644 --- a/app/serializers/champ_serializer.rb +++ b/app/serializers/champ_serializer.rb @@ -14,7 +14,9 @@ class ChampSerializer < ActiveModel::Serializer when GeoArea object.geometry.to_json when Champs::CarteChamp - object.user_geometry.to_json + if object.geo_json.present? + object.geo_json.to_json + end when Champs::DecimalNumberChamp if object.value.present? object.value.to_f diff --git a/spec/controllers/champs/carte_controller_spec.rb b/spec/controllers/champs/carte_controller_spec.rb index 6fbbd88e6..a530593eb 100644 --- a/spec/controllers/champs/carte_controller_spec.rb +++ b/spec/controllers/champs/carte_controller_spec.rb @@ -41,7 +41,7 @@ describe Champs::CarteController, type: :controller do expect(assigns(:error)).to eq(nil) expect(champ.reload.value).to eq(nil) expect(champ.reload.geo_areas).to eq([]) - expect(response.body).to include("DS.drawMapData(\".carte-1\", {\"position\":{\"lon\":\"2.428462\",\"lat\":\"46.538192\",\"zoom\":\"13\"},\"selection\":[],\"quartiersPrioritaires\":[],\"cadastres\":[],\"parcellesAgricoles\":[]});") + expect(response.body).to include("DS.drawMapData(\".carte-1\", {\"position\":{\"lon\":\"2.428462\",\"lat\":\"46.538192\",\"zoom\":\"13\"},\"selection\":null,\"quartiersPrioritaires\":[],\"cadastres\":[],\"parcellesAgricoles\":[]});") } end diff --git a/spec/models/champs/carte_champ_spec.rb b/spec/models/champs/carte_champ_spec.rb new file mode 100644 index 000000000..7c8c6f0cf --- /dev/null +++ b/spec/models/champs/carte_champ_spec.rb @@ -0,0 +1,63 @@ +require 'spec_helper' + +describe Champs::CarteChamp do + let(:champ) { Champs::CarteChamp.new(value: value) } + let(:value) { '' } + let(:geo_json) { GeojsonService.to_json_polygon_for_selection_utilisateur(coordinates) } + let(:coordinates) { [[{ "lat" => 48.87442541960633, "lng" => 2.3859214782714844 }, { "lat" => 48.87273183590832, "lng" => 2.3850631713867183 }, { "lat" => 48.87081237174292, "lng" => 2.3809432983398438 }, { "lat" => 48.8712640169951, "lng" => 2.377510070800781 }, { "lat" => 48.87510283703279, "lng" => 2.3778533935546875 }, { "lat" => 48.87544154230615, "lng" => 2.382831573486328 }, { "lat" => 48.87442541960633, "lng" => 2.3859214782714844 }]] } + let(:parsed_geo_json) { JSON.parse(geo_json) } + + describe '#to_render_data' do + subject { champ.to_render_data } + + let(:render_data) { + { + position: champ.position, + selection: selection, + cadastres: [], + parcellesAgricoles: [], + quartiersPrioritaires: [] + } + } + + context 'when the value is nil' do + let(:value) { nil } + + let(:selection) { nil } + + it { is_expected.to eq(render_data) } + end + + context 'when the value is blank' do + let(:value) { '' } + + let(:selection) { nil } + + it { is_expected.to eq(render_data) } + end + + context 'when the value is empty array' do + let(:value) { '[]' } + + let(:selection) { nil } + + it { is_expected.to eq(render_data) } + end + + context 'when the value is coordinates' do + let(:value) { coordinates.to_json } + + let(:selection) { parsed_geo_json } + + it { is_expected.to eq(render_data) } + end + + context 'when the value is geojson' do + let(:value) { geo_json } + + let(:selection) { parsed_geo_json } + + it { is_expected.to eq(render_data) } + end + end +end diff --git a/spec/serializers/champ_serializer_spec.rb b/spec/serializers/champ_serializer_spec.rb index c2a92f3f0..a72cc4fa3 100644 --- a/spec/serializers/champ_serializer_spec.rb +++ b/spec/serializers/champ_serializer_spec.rb @@ -30,23 +30,45 @@ describe ChampSerializer do let(:serialized_champ) { { - type_de_champ: { - description: serialized_description, - id: serialized_id, - libelle: serialized_libelle, - order_place: serialized_order_place, - type_champ: serialized_type_champ - }, - geo_areas: serialized_geo_areas, - value: geo_json - }.compact + type_de_champ: serialized_type_de_champ, + value: serialized_value + } + } + let(:serialized_type_de_champ) { + { + description: serialized_description, + id: serialized_id, + libelle: serialized_libelle, + order_place: serialized_order_place, + type_champ: serialized_type_champ + } } let(:serialized_id) { -1 } let(:serialized_description) { "" } let(:serialized_order_place) { -1 } - let(:serialized_geo_areas) { nil } + let(:serialized_value) { geo_json } context 'and geo_area is selection_utilisateur' do + context 'value is empty' do + context 'when value is nil' do + let(:value) { nil } + + it { expect(champ.user_geo_area).to be_nil } + end + + context 'when value is empty array' do + let(:value) { '[]' } + + it { expect(champ.user_geo_area).to be_nil } + end + + context 'when value is blank' do + let(:value) { '' } + + it { expect(champ.user_geo_area).to be_nil } + end + end + context 'old_api' do let(:serialized_libelle) { "user geometry" } let(:serialized_type_champ) { "user_geometry" } @@ -68,7 +90,13 @@ describe ChampSerializer do context 'new_api' do let(:geo_area) { nil } - let(:serialized_geo_areas) { [] } + let(:serialized_champ) { + { + type_de_champ: serialized_type_de_champ, + geo_areas: [], + value: serialized_value + } + } let(:serialized_id) { champ.type_de_champ.stable_id } let(:serialized_description) { champ.description } let(:serialized_order_place) { champ.order_place } @@ -86,6 +114,27 @@ describe ChampSerializer do it { expect(subject).to eq(serialized_champ) } end + + context 'when value is nil' do + let(:value) { nil } + let(:serialized_value) { nil } + + it { expect(subject).to eq(serialized_champ) } + end + + context 'when value is empty array' do + let(:value) { '[]' } + let(:serialized_value) { nil } + + it { expect(subject).to eq(serialized_champ) } + end + + context 'when value is blank' do + let(:value) { '' } + let(:serialized_value) { nil } + + it { expect(subject).to eq(serialized_champ) } + end end end From 6043e59937a86fa136e46e85b5aac3e687bb29d8 Mon Sep 17 00:00:00 2001 From: Paul Chavard Date: Sat, 1 Dec 2018 10:30:35 +0100 Subject: [PATCH 3/3] Disable after party --- config/deploy.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/config/deploy.rb b/config/deploy.rb index ad139f136..09d6c0ff0 100644 --- a/config/deploy.rb +++ b/config/deploy.rb @@ -103,7 +103,7 @@ task :deploy do invoke :'bundle:install' invoke :'yarn:install' invoke :'rails:db_migrate' - invoke :'after_party:run' + # invoke :'after_party:run' invoke :'rails:assets_precompile' on :launch do