From 4dd87f3394a7f2632a28c3bc680d11b1c1ca2105 Mon Sep 17 00:00:00 2001 From: Daniel Jackson Date: Mon, 30 Sep 2024 13:15:20 +0200 Subject: [PATCH 01/16] Add geographical area models --- .../with_parent_child_relationship.rb | 18 ++++++++++ app/models/geographical_area.rb | 33 +++++++++++++++++++ app/models/geographical_area_type.rb | 26 +++++++++++++++ ...0240930105603_create_geographical_areas.rb | 21 ++++++++++++ db/schema.rb | 26 ++++++++++++++- 5 files changed, 123 insertions(+), 1 deletion(-) create mode 100644 app/models/concerns/with_parent_child_relationship.rb create mode 100644 app/models/geographical_area.rb create mode 100644 app/models/geographical_area_type.rb create mode 100644 db/migrate/20240930105603_create_geographical_areas.rb diff --git a/app/models/concerns/with_parent_child_relationship.rb b/app/models/concerns/with_parent_child_relationship.rb new file mode 100644 index 00000000..c0b9c6fb --- /dev/null +++ b/app/models/concerns/with_parent_child_relationship.rb @@ -0,0 +1,18 @@ +# frozen_string_literal: true + +module WithParentChildRelationship + extend ActiveSupport::Concern + + belongs_to :parent, class_name: name, optional: true, inverse_of: :children + has_many :children, class_name: name, foreign_key: "parent_id", dependent: :nullify, inverse_of: :parent + + validate :no_circular_references + + private + + def no_circular_references + return unless parent.present? && parent.children.include?(self) + + errors.add(:base, "Cannot add itself as a parent") + end +end diff --git a/app/models/geographical_area.rb b/app/models/geographical_area.rb new file mode 100644 index 00000000..adc1ef19 --- /dev/null +++ b/app/models/geographical_area.rb @@ -0,0 +1,33 @@ +# frozen_string_literal: true + +class GeographicalArea < ApplicationRecord + include WithParentChildRelationship + + validates :name, presence: true + validates :geo_area, presence: true +end + +# == Schema Information +# +# Table name: geographical_areas +# +# id :bigint not null, primary key +# filterable :boolean default(TRUE) +# geo_area :geometry not null, geometry, 0 +# name :string +# order :integer +# created_at :datetime not null +# updated_at :datetime not null +# geographical_area_type_id :bigint +# parent_id :bigint +# +# Indexes +# +# index_geographical_areas_on_geographical_area_type_id (geographical_area_type_id) +# index_geographical_areas_on_parent_id (parent_id) +# +# Foreign Keys +# +# fk_rails_... (geographical_area_type_id => geographical_area_types.id) +# fk_rails_... (parent_id => geographical_areas.id) +# diff --git a/app/models/geographical_area_type.rb b/app/models/geographical_area_type.rb new file mode 100644 index 00000000..2779e661 --- /dev/null +++ b/app/models/geographical_area_type.rb @@ -0,0 +1,26 @@ +# frozen_string_literal: true + +class GeographicalAreaType < ApplicationRecord + include WithParentChildRelationship + + validates :name, presence: true +end + +# == Schema Information +# +# Table name: geographical_area_types +# +# id :bigint not null, primary key +# name :string +# created_at :datetime not null +# updated_at :datetime not null +# parent_id :bigint +# +# Indexes +# +# index_geographical_area_types_on_parent_id (parent_id) +# +# Foreign Keys +# +# fk_rails_... (parent_id => geographical_area_types.id) +# diff --git a/db/migrate/20240930105603_create_geographical_areas.rb b/db/migrate/20240930105603_create_geographical_areas.rb new file mode 100644 index 00000000..716f6b40 --- /dev/null +++ b/db/migrate/20240930105603_create_geographical_areas.rb @@ -0,0 +1,21 @@ +class CreateGeographicalAreas < ActiveRecord::Migration[7.1] + def change + create_table :geographical_area_types do |t| + t.string :name + t.references :parent, foreign_key: { to_table: :geographical_area_types } + + t.timestamps + end + + create_table :geographical_areas do |t| + t.string :name + t.boolean :filterable, default: true + t.integer :order + t.geometry :geo_area, null: false + t.references :parent, foreign_key: { to_table: :geographical_areas } + t.belongs_to :geographical_area_type, foreign_key: true + + t.timestamps + end + end +end diff --git a/db/schema.rb b/db/schema.rb index 1d2faecb..43b4bc9c 100644 --- a/db/schema.rb +++ b/db/schema.rb @@ -10,7 +10,7 @@ # # It's strongly recommended that you check this file into your version control system. -ActiveRecord::Schema[7.1].define(version: 2024_09_27_071029) do +ActiveRecord::Schema[7.1].define(version: 2024_09_30_105603) do # These are extensions that must be enabled in order to support this database enable_extension "plpgsql" enable_extension "postgis" @@ -100,6 +100,27 @@ t.index ["user_id"], name: "index_facility_reviews_on_user_id" end + create_table "geographical_area_types", force: :cascade do |t| + t.string "name" + t.bigint "parent_id" + t.datetime "created_at", null: false + t.datetime "updated_at", null: false + t.index ["parent_id"], name: "index_geographical_area_types_on_parent_id" + end + + create_table "geographical_areas", force: :cascade do |t| + t.string "name" + t.boolean "filterable", default: true + t.integer "order" + t.geometry "geo_area", limit: {:srid=>0, :type=>"geometry"}, null: false + t.bigint "parent_id" + t.bigint "geographical_area_type_id" + t.datetime "created_at", null: false + t.datetime "updated_at", null: false + t.index ["geographical_area_type_id"], name: "index_geographical_areas_on_geographical_area_type_id" + t.index ["parent_id"], name: "index_geographical_areas_on_parent_id" + end + create_table "images", force: :cascade do |t| t.string "credits" t.string "caption" @@ -287,6 +308,9 @@ add_foreign_key "facility_reviews", "facilities" add_foreign_key "facility_reviews", "spaces" add_foreign_key "facility_reviews", "users" + add_foreign_key "geographical_area_types", "geographical_area_types", column: "parent_id" + add_foreign_key "geographical_areas", "geographical_area_types" + add_foreign_key "geographical_areas", "geographical_areas", column: "parent_id" add_foreign_key "personal_data_on_space_in_lists", "personal_space_lists" add_foreign_key "personal_data_on_space_in_lists", "spaces" add_foreign_key "personal_space_lists_shared_with_mes", "personal_space_lists" From 898d355d0779c387d162738f10c8602b98f557cf Mon Sep 17 00:00:00 2001 From: Daniel Jackson Date: Mon, 30 Sep 2024 14:28:39 +0200 Subject: [PATCH 02/16] Import fylker and kommuner from geo norge --- .../with_parent_child_relationship.rb | 7 +- app/models/geographical_area.rb | 26 ++-- app/models/geographical_area_type.rb | 4 + ...external_source_info_to_geographic_area.rb | 6 + db/schema.rb | 4 +- .../import_geo_data_over_api_functions.rb | 112 ++++++++++++++++++ ...port_geographical_areas_from_geonorge.rake | 10 ++ 7 files changed, 153 insertions(+), 16 deletions(-) create mode 100644 db/migrate/20240930112551_add_external_source_info_to_geographic_area.rb create mode 100644 db/seeds/imports/import_geo_data_over_api_functions.rb create mode 100644 lib/tasks/geo/import_geographical_areas_from_geonorge.rake diff --git a/app/models/concerns/with_parent_child_relationship.rb b/app/models/concerns/with_parent_child_relationship.rb index c0b9c6fb..24875f28 100644 --- a/app/models/concerns/with_parent_child_relationship.rb +++ b/app/models/concerns/with_parent_child_relationship.rb @@ -3,15 +3,10 @@ module WithParentChildRelationship extend ActiveSupport::Concern - belongs_to :parent, class_name: name, optional: true, inverse_of: :children - has_many :children, class_name: name, foreign_key: "parent_id", dependent: :nullify, inverse_of: :parent - - validate :no_circular_references - private def no_circular_references - return unless parent.present? && parent.children.include?(self) + return unless parent_id == id errors.add(:base, "Cannot add itself as a parent") end diff --git a/app/models/geographical_area.rb b/app/models/geographical_area.rb index adc1ef19..f4165e9a 100644 --- a/app/models/geographical_area.rb +++ b/app/models/geographical_area.rb @@ -3,6 +3,12 @@ class GeographicalArea < ApplicationRecord include WithParentChildRelationship + belongs_to :parent, class_name: "GeographicalArea", optional: true, inverse_of: :children + has_many :children, class_name: "GeographicalArea", foreign_key: "parent_id", dependent: :nullify, inverse_of: :parent + validate :no_circular_references + + belongs_to :geographical_area_type + validates :name, presence: true validates :geo_area, presence: true end @@ -11,15 +17,17 @@ class GeographicalArea < ApplicationRecord # # Table name: geographical_areas # -# id :bigint not null, primary key -# filterable :boolean default(TRUE) -# geo_area :geometry not null, geometry, 0 -# name :string -# order :integer -# created_at :datetime not null -# updated_at :datetime not null -# geographical_area_type_id :bigint -# parent_id :bigint +# id :bigint not null, primary key +# external_source :string +# filterable :boolean default(TRUE) +# geo_area :geometry not null, geometry, 0 +# name :string +# order :integer +# unique_id_for_external_source :string +# created_at :datetime not null +# updated_at :datetime not null +# geographical_area_type_id :bigint +# parent_id :bigint # # Indexes # diff --git a/app/models/geographical_area_type.rb b/app/models/geographical_area_type.rb index 2779e661..22b03aa3 100644 --- a/app/models/geographical_area_type.rb +++ b/app/models/geographical_area_type.rb @@ -3,6 +3,10 @@ class GeographicalAreaType < ApplicationRecord include WithParentChildRelationship + belongs_to :parent, class_name: "GeographicalArea", optional: true, inverse_of: :children + has_many :children, class_name: "GeographicalArea", foreign_key: "parent_id", dependent: :nullify, inverse_of: :parent + validate :no_circular_references + validates :name, presence: true end diff --git a/db/migrate/20240930112551_add_external_source_info_to_geographic_area.rb b/db/migrate/20240930112551_add_external_source_info_to_geographic_area.rb new file mode 100644 index 00000000..a9140088 --- /dev/null +++ b/db/migrate/20240930112551_add_external_source_info_to_geographic_area.rb @@ -0,0 +1,6 @@ +class AddExternalSourceInfoToGeographicArea < ActiveRecord::Migration[7.1] + def change + add_column :geographical_areas, :unique_id_for_external_source, :string + add_column :geographical_areas, :external_source, :string + end +end diff --git a/db/schema.rb b/db/schema.rb index 43b4bc9c..4904a149 100644 --- a/db/schema.rb +++ b/db/schema.rb @@ -10,7 +10,7 @@ # # It's strongly recommended that you check this file into your version control system. -ActiveRecord::Schema[7.1].define(version: 2024_09_30_105603) do +ActiveRecord::Schema[7.1].define(version: 2024_09_30_112551) do # These are extensions that must be enabled in order to support this database enable_extension "plpgsql" enable_extension "postgis" @@ -117,6 +117,8 @@ t.bigint "geographical_area_type_id" t.datetime "created_at", null: false t.datetime "updated_at", null: false + t.string "unique_id_for_external_source" + t.string "external_source" t.index ["geographical_area_type_id"], name: "index_geographical_areas_on_geographical_area_type_id" t.index ["parent_id"], name: "index_geographical_areas_on_parent_id" end diff --git a/db/seeds/imports/import_geo_data_over_api_functions.rb b/db/seeds/imports/import_geo_data_over_api_functions.rb new file mode 100644 index 00000000..e78d1e32 --- /dev/null +++ b/db/seeds/imports/import_geo_data_over_api_functions.rb @@ -0,0 +1,112 @@ +# frozen_string_literal: true + +# rubocop:disable Metrics/methodLength Metrics/AbcSize + +require "net/http" +require "uri" +require "json" +require "rgeo/geo_json" + +GEONORGE_API_SOURCE_NAME = "geonorge-api" + +def load_geo_area_from_geonorge(uri_prefix, uri_id) + uri = URI.parse("#{uri_prefix}/#{uri_id}/omrade") + response = Net::HTTP.get_response(uri) + omrade = JSON.parse(response.body)["omrade"] + + RGeo::GeoJSON.decode(omrade, geo_factory: Geo.factory) +end + +FYLKE_API_END_POINT = "https://ws.geonorge.no/kommuneinfo/v1/fylker" +def load_fylker_from_geonorge + geographical_area_type = GeographicalAreaType.find_or_create_by(name: "Fylke") + + Rails.logger.debug do + "Loading fylker from API. We currently have: #{GeographicalArea.where(geographical_area_type:).count} fylker" + end + + uri = URI.parse(FYLKE_API_END_POINT) + response = Net::HTTP.get_response(uri) + fylker = JSON.parse(response.body) + + Rails.logger.debug { "API has #{fylker.size} fylker to sync with" } + + fylker.each do |fylke| + external_id = fylke["fylkesnummer"] + + fylke_in_db = GeographicalArea.find_or_create_by(unique_id_for_external_source: external_id, + geographical_area_type:) + fylke_in_db.update( + name: fylke["fylkesnavn"], + external_source: GEONORGE_API_SOURCE_NAME, + geo_area: load_geo_area_from_geonorge(FYLKE_API_END_POINT, external_id) + ) + + throw "Error loading fylke #{external_id}: #{fylke_in_db.errors}" unless fylke_in_db.persisted? + + Rails.logger.debug { "\rLoaded #{external_id} #{fylke['fylkesnavn']} " } + end + + Rails.logger.debug "\rRemoving any fylker that are not in the API \n" + GeographicalArea + .where(geographical_area_type:) + .where.not(unique_id_for_external_source: fylker.pluck("fylkesnummer")) + .destroy_all + + Rails.logger.debug do + "\rLoaded fylker from API. We now have: #{GeographicalArea.where(geographical_area_type:).count} fylker \n" + end +end + +KOMMUNE_API_END_POINT = "https://ws.geonorge.no/kommuneinfo/v1/kommuner" + +def load_kommuner_from_geonorge + geographical_area_type = GeographicalAreaType.find_or_create_by(name: "Kommune") + + Rails.logger.debug do + "Loading kommuner from API. We currently have: #{GeographicalArea.where(geographical_area_type:).count} kommuner" + end + + uri = URI.parse(KOMMUNE_API_END_POINT) + response = Net::HTTP.get_response(uri) + kommuner = JSON.parse(response.body) + + Rails.logger.debug { "API has #{kommuner.size} kommuner to sync with" } + + kommuner.each do |kommune| + kommunenummer = kommune["kommunenummer"] + fylkesnummer = kommunenummer[0..1] # First two digits are the fylkesnummer + fylke = GeographicalArea.find_by(unique_id_for_external_source: fylkesnummer, + geographical_area_type: GeographicalAreaType.find_by(name: "Fylke")) + + navn = kommune["kommunenavnNorsk"] + # Add the Saami name if it exists + navn += " (#{kommune['kommunenavn']})" if kommune["kommunenavn"] != kommune["kommunenavnNorsk"] + + kommune_in_db = GeographicalArea.find_or_create_by(unique_id_for_external_source: kommunenummer, + geographical_area_type:) + + kommune_in_db.update( + name: navn, + parent: fylke, + external_source: GEONORGE_API_SOURCE_NAME, + geo_area: load_geo_area_from_geonorge(KOMMUNE_API_END_POINT, kommunenummer) + ) + + throw "Error loading kommune #{kommunenummer}: #{kommune_in_db.errors}" unless kommune_in_db.persisted? + + Rails.logger.debug { "\rLoaded #{kommunenummer} #{navn} " } + end + + Rails.logger.debug "\rRemoving any kommuner that are not in the API \n" + GeographicalArea + .where(geographical_area_type:) + .where.not(unique_id_for_external_source: kommuner.pluck("kommunenummer")) + .destroy_all + + Rails.logger.debug do + "\rLoaded kommuner from API. We now have: #{GeographicalArea.where(geographical_area_type:).count} kommuner \n" + end +end + +# rubocop:enable Metrics/methodLength Metrics/AbcSize diff --git a/lib/tasks/geo/import_geographical_areas_from_geonorge.rake b/lib/tasks/geo/import_geographical_areas_from_geonorge.rake new file mode 100644 index 00000000..22fadd7a --- /dev/null +++ b/lib/tasks/geo/import_geographical_areas_from_geonorge.rake @@ -0,0 +1,10 @@ +# frozen_string_literal: true + +require_relative "../../../db/seeds/imports/import_geo_data_over_api_functions" + +namespace :geo do + task "import_geographical_areas_from_geonorge" => :environment do + load_fylker_from_geonorge + load_kommuner_from_geonorge + end +end From fa320b9c700cb2926bb68a4417ac73b02fd7baf1 Mon Sep 17 00:00:00 2001 From: Daniel Jackson Date: Mon, 30 Sep 2024 15:06:24 +0200 Subject: [PATCH 03/16] Add fylker and kommuner to spaces --- README.md | 2 ++ app/models/fylke.rb | 13 +++++++++ app/models/geographical_area.rb | 3 ++ app/models/kommune.rb | 13 +++++++++ app/models/space.rb | 28 +++++++++++++++++++ ...30124657_add_fylke_and_kommune_to_space.rb | 12 ++++++++ ...0125554_add_index_to_geographical_areas.rb | 5 ++++ db/schema.rb | 9 +++++- ...port_geographical_areas_from_geonorge.rake | 10 +++++++ spec/fabricators/space_fabricator.rb | 6 ++++ 10 files changed, 100 insertions(+), 1 deletion(-) create mode 100644 app/models/fylke.rb create mode 100644 app/models/kommune.rb create mode 100644 db/migrate/20240930124657_add_fylke_and_kommune_to_space.rb create mode 100644 db/migrate/20240930125554_add_index_to_geographical_areas.rb diff --git a/README.md b/README.md index 854c8341..94fc475d 100644 --- a/README.md +++ b/README.md @@ -167,6 +167,8 @@ Run `rails db:seed` to get sample data into your app. You can also set the ENV variable ["SEED_FILE"](https://github.com/lnu-norge/lokaler.lnu.no/pull/66) to load a different seed file than the current environment dictates. Useful for deploying tests on Heroku, as Heroku always wants you to run in production mode - but you might want to seed with development data. +To get geographical data for Fylker and Kommuner, run: `rails geo:import_geographical_areas_from_geonorge`. This will hit the APIs of geo norge and add, and update, any fylker and kommuner. + ## Sendgrid setup You need to set the right sendgrid environment variables, and to get sendgrid to work you will also need to set the ENV variable ["HOST"] to equal to the domain you are using, example: `ENV["HOST"] = "app.herokuapp.com` diff --git a/app/models/fylke.rb b/app/models/fylke.rb new file mode 100644 index 00000000..77db4584 --- /dev/null +++ b/app/models/fylke.rb @@ -0,0 +1,13 @@ +# frozen_string_literal: true + +class Fylke < GeographicalArea + default_scope { where(geographical_area_type: GeographicalAreaType.find_by(name: "Fylke")) } + + before_validation :set_geographical_area_type + + private + + def set_geographical_area_type + self.geographical_area_type = GeographicalAreaType.find_or_create_by(name: "Fylke") + end +end diff --git a/app/models/geographical_area.rb b/app/models/geographical_area.rb index f4165e9a..508322dc 100644 --- a/app/models/geographical_area.rb +++ b/app/models/geographical_area.rb @@ -7,6 +7,8 @@ class GeographicalArea < ApplicationRecord has_many :children, class_name: "GeographicalArea", foreign_key: "parent_id", dependent: :nullify, inverse_of: :parent validate :no_circular_references + has_many :spaces, dependent: :nullify + belongs_to :geographical_area_type validates :name, presence: true @@ -31,6 +33,7 @@ class GeographicalArea < ApplicationRecord # # Indexes # +# index_geographical_areas_on_geo_area (geo_area) USING gist # index_geographical_areas_on_geographical_area_type_id (geographical_area_type_id) # index_geographical_areas_on_parent_id (parent_id) # diff --git a/app/models/kommune.rb b/app/models/kommune.rb new file mode 100644 index 00000000..a8dd321c --- /dev/null +++ b/app/models/kommune.rb @@ -0,0 +1,13 @@ +# frozen_string_literal: true + +class Kommune < GeographicalArea + default_scope { where(geographical_area_type: GeographicalAreaType.find_by(name: "Kommune")) } + + before_validation :set_geographical_area_type + + private + + def set_geographical_area_type + self.geographical_area_type = GeographicalAreaType.find_or_create_by(name: "Kommune") + end +end diff --git a/app/models/space.rb b/app/models/space.rb index ab097af4..178a6e33 100644 --- a/app/models/space.rb +++ b/app/models/space.rb @@ -3,6 +3,9 @@ class Space < ApplicationRecord # rubocop:disable Metrics/ClassLength has_paper_trail skip: [:star_rating] + belongs_to :fylke, class_name: "Fylke", optional: true + belongs_to :kommune, class_name: "Kommune", optional: true + has_many :images, dependent: :destroy accepts_nested_attributes_for :images @@ -338,6 +341,11 @@ def to_param [id, title.parameterize].join("-") end + def set_geo_data + set_geo_point + set_geo_areas_space_belongs_to + end + private def clear_vector_tile_caches @@ -364,8 +372,22 @@ def set_geo_data_from_lng_lat return unless lat_lng_set? return if geo_point_equal_to_lng_lat? + set_geo_data + end + + def set_geo_point self.geo_point = Geo.point(lng, lat) # Postgis format end + + def set_geo_areas_space_belongs_to + # Find the Fylke that contains this point + fylke = Fylke.where("ST_Contains(geo_area, ?)", geo_point).first + self.fylke = fylke if fylke + + # Find the Kommune that contains this point + kommune = Kommune.where("ST_Contains(geo_area, ?)", geo_point).first + self.kommune = kommune if kommune + end end # == Schema Information @@ -387,14 +409,20 @@ def set_geo_data_from_lng_lat # url :string # created_at :datetime not null # updated_at :datetime not null +# fylke_id :bigint +# kommune_id :bigint # space_group_id :bigint # # Indexes # +# index_spaces_on_fylke_id (fylke_id) # index_spaces_on_geo_point (geo_point) USING gist +# index_spaces_on_kommune_id (kommune_id) # index_spaces_on_space_group_id (space_group_id) # # Foreign Keys # +# fk_rails_... (fylke_id => geographical_areas.id) +# fk_rails_... (kommune_id => geographical_areas.id) # fk_rails_... (space_group_id => space_groups.id) # diff --git a/db/migrate/20240930124657_add_fylke_and_kommune_to_space.rb b/db/migrate/20240930124657_add_fylke_and_kommune_to_space.rb new file mode 100644 index 00000000..100c463c --- /dev/null +++ b/db/migrate/20240930124657_add_fylke_and_kommune_to_space.rb @@ -0,0 +1,12 @@ +class AddFylkeAndKommuneToSpace < ActiveRecord::Migration[7.1] + def change + add_column :spaces, :fylke_id, :bigint + add_column :spaces, :kommune_id, :bigint + + add_foreign_key :spaces, :geographical_areas, column: :fylke_id + add_foreign_key :spaces, :geographical_areas, column: :kommune_id + + add_index :spaces, :fylke_id + add_index :spaces, :kommune_id + end +end diff --git a/db/migrate/20240930125554_add_index_to_geographical_areas.rb b/db/migrate/20240930125554_add_index_to_geographical_areas.rb new file mode 100644 index 00000000..efd3cd96 --- /dev/null +++ b/db/migrate/20240930125554_add_index_to_geographical_areas.rb @@ -0,0 +1,5 @@ +class AddIndexToGeographicalAreas < ActiveRecord::Migration[7.1] + def change + add_index :geographical_areas, :geo_area, using: :gist + end +end diff --git a/db/schema.rb b/db/schema.rb index 4904a149..860ef5f9 100644 --- a/db/schema.rb +++ b/db/schema.rb @@ -10,7 +10,7 @@ # # It's strongly recommended that you check this file into your version control system. -ActiveRecord::Schema[7.1].define(version: 2024_09_30_112551) do +ActiveRecord::Schema[7.1].define(version: 2024_09_30_125554) do # These are extensions that must be enabled in order to support this database enable_extension "plpgsql" enable_extension "postgis" @@ -119,6 +119,7 @@ t.datetime "updated_at", null: false t.string "unique_id_for_external_source" t.string "external_source" + t.index ["geo_area"], name: "index_geographical_areas_on_geo_area", using: :gist t.index ["geographical_area_type_id"], name: "index_geographical_areas_on_geographical_area_type_id" t.index ["parent_id"], name: "index_geographical_areas_on_parent_id" end @@ -269,7 +270,11 @@ t.string "url" t.text "location_description" t.geography "geo_point", limit: {:srid=>4326, :type=>"st_point", :geographic=>true}, null: false + t.bigint "fylke_id" + t.bigint "kommune_id" + t.index ["fylke_id"], name: "index_spaces_on_fylke_id" t.index ["geo_point"], name: "index_spaces_on_geo_point", using: :gist + t.index ["kommune_id"], name: "index_spaces_on_kommune_id" t.index ["space_group_id"], name: "index_spaces_on_space_group_id" end @@ -327,5 +332,7 @@ add_foreign_key "space_types_facilities", "space_types" add_foreign_key "space_types_relations", "space_types" add_foreign_key "space_types_relations", "spaces" + add_foreign_key "spaces", "geographical_areas", column: "fylke_id" + add_foreign_key "spaces", "geographical_areas", column: "kommune_id" add_foreign_key "spaces", "space_groups" end diff --git a/lib/tasks/geo/import_geographical_areas_from_geonorge.rake b/lib/tasks/geo/import_geographical_areas_from_geonorge.rake index 22fadd7a..b65d6f0e 100644 --- a/lib/tasks/geo/import_geographical_areas_from_geonorge.rake +++ b/lib/tasks/geo/import_geographical_areas_from_geonorge.rake @@ -6,5 +6,15 @@ namespace :geo do task "import_geographical_areas_from_geonorge" => :environment do load_fylker_from_geonorge load_kommuner_from_geonorge + Rake::Task["geo:set_geographical_areas_spaces_belong_to"].invoke + end + + task "set_geographical_areas_spaces_belong_to" => :environment do + Space.find_each do |space| + Rails.logger.debug { "Setting geo data for space #{space.id}" } + space.set_geo_data + space.save! + end + Rails.logger.debug { "Finished setting geo data" } end end diff --git a/spec/fabricators/space_fabricator.rb b/spec/fabricators/space_fabricator.rb index 73aa1d99..29c666dd 100644 --- a/spec/fabricators/space_fabricator.rb +++ b/spec/fabricators/space_fabricator.rb @@ -60,14 +60,20 @@ def add_some_space_facilities(space) # url :string # created_at :datetime not null # updated_at :datetime not null +# fylke_id :bigint +# kommune_id :bigint # space_group_id :bigint # # Indexes # +# index_spaces_on_fylke_id (fylke_id) # index_spaces_on_geo_point (geo_point) USING gist +# index_spaces_on_kommune_id (kommune_id) # index_spaces_on_space_group_id (space_group_id) # # Foreign Keys # +# fk_rails_... (fylke_id => geographical_areas.id) +# fk_rails_... (kommune_id => geographical_areas.id) # fk_rails_... (space_group_id => space_groups.id) # From f260b4340ea11837d755908d632e6a95cd840234 Mon Sep 17 00:00:00 2001 From: Daniel Jackson Date: Mon, 30 Sep 2024 15:34:58 +0200 Subject: [PATCH 04/16] Add inflection for fylke and kommune --- config/initializers/inflections.rb | 11 +++++++---- 1 file changed, 7 insertions(+), 4 deletions(-) diff --git a/config/initializers/inflections.rb b/config/initializers/inflections.rb index aa7435fb..e490a54b 100644 --- a/config/initializers/inflections.rb +++ b/config/initializers/inflections.rb @@ -1,4 +1,5 @@ # frozen_string_literal: true + # Be sure to restart your server when you modify this file. # Add new inflection rules using the following format. Inflections @@ -11,7 +12,9 @@ # inflect.uncountable %w( fish sheep ) # end -# These inflection rules are supported but not enabled by default: -# ActiveSupport::Inflector.inflections(:en) do |inflect| -# inflect.acronym 'RESTful' -# end +ActiveSupport::Inflector.inflections(:en) do |inflect| + # Norwegian mixed in with english, + # just so I don't have to remember how to type municipalities in english + inflect.irregular "fylke", "fylker" + inflect.irregular "kommune", "kommuner" +end From 9f499ed841153355f1b3f88da5c18c37653374d1 Mon Sep 17 00:00:00 2001 From: Daniel Jackson Date: Mon, 30 Sep 2024 15:36:45 +0200 Subject: [PATCH 05/16] Add kommuner and fylker relation, and start using it in filter_on_location --- app/models/fylke.rb | 12 +++++++++++- app/models/kommune.rb | 6 +++++- config/locales/space_filter.nb.yml | 1 + 3 files changed, 17 insertions(+), 2 deletions(-) diff --git a/app/models/fylke.rb b/app/models/fylke.rb index 77db4584..d6313108 100644 --- a/app/models/fylke.rb +++ b/app/models/fylke.rb @@ -1,7 +1,17 @@ # frozen_string_literal: true class Fylke < GeographicalArea - default_scope { where(geographical_area_type: GeographicalAreaType.find_by(name: "Fylke")) } + default_scope do + where( + geographical_area_type: GeographicalAreaType.find_by(name: "Fylke") + ) + .order(:unique_id_for_external_source) + end + has_many :kommuner, + class_name: "Kommune", + foreign_key: :parent_id, + inverse_of: :fylke, + dependent: :destroy before_validation :set_geographical_area_type diff --git a/app/models/kommune.rb b/app/models/kommune.rb index a8dd321c..7ca02eb3 100644 --- a/app/models/kommune.rb +++ b/app/models/kommune.rb @@ -1,7 +1,11 @@ # frozen_string_literal: true class Kommune < GeographicalArea - default_scope { where(geographical_area_type: GeographicalAreaType.find_by(name: "Kommune")) } + default_scope do + where(geographical_area_type: GeographicalAreaType.find_by(name: "Kommune")) + .order(:name) + end + belongs_to :fylke, class_name: "Fylke", foreign_key: :parent_id, inverse_of: :kommuner before_validation :set_geographical_area_type diff --git a/config/locales/space_filter.nb.yml b/config/locales/space_filter.nb.yml index 3ca86a3d..bc33e29d 100644 --- a/config/locales/space_filter.nb.yml +++ b/config/locales/space_filter.nb.yml @@ -15,6 +15,7 @@ nb: edit_filter: 'Rediger filtre' location: 'Område' choose_location: 'Søk her, eller flytt på kart' + set_location_by_map: 'Søk i kartet' title: 'Navn på lokalet' more_filters: 'Flere filtre' map_reload: 'Søk i området' From a69f7fb54a1ee5e5b119c99aef525293c869743f Mon Sep 17 00:00:00 2001 From: Daniel Jackson Date: Mon, 30 Sep 2024 15:37:40 +0200 Subject: [PATCH 06/16] WIP check boxes for filtering on kommuner and fylker - doesn't auto open when clicked, and do not filter yet --- .../_filter_on_location.html.erb | 44 ++++++++++++++++--- 1 file changed, 39 insertions(+), 5 deletions(-) diff --git a/app/views/spaces/index/search_form_fields/_filter_on_location.html.erb b/app/views/spaces/index/search_form_fields/_filter_on_location.html.erb index 6eaf07b5..6dbc490f 100644 --- a/app/views/spaces/index/search_form_fields/_filter_on_location.html.erb +++ b/app/views/spaces/index/search_form_fields/_filter_on_location.html.erb @@ -2,11 +2,45 @@ <%= form.label t("space_filter.location"), for: "locationInput", class: "text-xl font-bold my-4" %>
- + <% Fylke.all.each do |fylke| %> + <% fylke_checked = params[:fylke]&.include?(fylke.id.to_s) %> + <%= form.label "geo_area_#{fylke.id}", class: "flex gap-1 my-1 items-top" do %> + <%= form.check_box :fylke, + { + id: "geo_area_#{fylke.id}", + multiple: true, + class: "h-5 w-5 relative top-0.5 text-lnu-pink focus:ring-lnu-pink border-gray-300 rounded", + onchange: "this.form.requestSubmit()", + checked: fylke_checked + }, + fylke.id.to_s, + nil + %> + <%= fylke.name %> + <% end %> + + <% next unless fylke_checked and fylke.kommuner.any? or fylke.kommuner.find_by(id: params[:kommune]).present? %> + +
+ <% fylke.kommuner.each do |kommune| %> + <%= form.label "geo_area_#{kommune.id}", class: "flex gap-1 my-1 items-top" do %> + <%= form.check_box :kommune, + { + id: "geo_area_#{kommune.id}", + multiple: true, + class: "h-5 w-5 relative top-0.5 text-lnu-pink focus:ring-lnu-pink border-gray-300 rounded", + onchange: "this.form.requestSubmit()", + checked: params[:kommune]&.include?(kommune.id.to_s) + }, + kommune.id.to_s, + nil + %> + <%= kommune.name %> + <% end %> + <% end %> +
+ <% end %> + <%= form.hidden_field :north_west_lat, value: params[:north_west_lat], class: "text_field", data: { mapbox_target: "northWestLatInput" } %> <%= form.hidden_field :north_west_lng, value: params[:north_west_lng], class: "text_field", data: { mapbox_target: "northWestLngInput" } %> <%= form.hidden_field :south_east_lat, value: params[:south_east_lat], class: "text_field", data: { mapbox_target: "southEastLatInput" } %> From b1f035c4dbf49fbfec959b42f3414e297d5c1f1d Mon Sep 17 00:00:00 2001 From: Daniel Jackson Date: Tue, 1 Oct 2024 11:42:04 +0200 Subject: [PATCH 07/16] Filter by fylker or kommuner, move map to show the new selection, and highlight it on the map. --- app/controllers/concerns/filterable_spaces.rb | 103 +++++++++++----- .../map_selected_geo_area_controller.rb | 31 +++++ .../controllers/mapbox_controller.js | 114 ++++++++++++++++-- app/models/space.rb | 5 +- .../_filter_on_location.html.erb | 17 ++- config/routes.rb | 1 + 6 files changed, 222 insertions(+), 49 deletions(-) create mode 100644 app/controllers/spaces/map_selected_geo_area_controller.rb diff --git a/app/controllers/concerns/filterable_spaces.rb b/app/controllers/concerns/filterable_spaces.rb index c9eb5c00..8e64caee 100644 --- a/app/controllers/concerns/filterable_spaces.rb +++ b/app/controllers/concerns/filterable_spaces.rb @@ -14,23 +14,45 @@ def set_filterable_space_types end def set_filtered_facilities - facility_ids = sanitize_facility_list(params[:facilities]) + facility_ids = sanitize_id_list(params[:facilities]) @filtered_facilities = Facility.includes(:facility_categories).find(facility_ids) end - def filter_spaces - set_filters_from_session_or_params + def set_filtered_geo_area_ids + unless params[:fylker].present? || params[:kommuner].present? + return @filtered_fylke_ids, @filtered_kommune_ids, @filtered_geo_area_ids = [] + end - @filtered_spaces = Space.all + kommune_ids = sanitize_id_list(params[:kommuner]) + fylke_ids = fylke_ids_excluding_those_with_a_selected_kommune(sanitize_id_list(params[:fylker]), kommune_ids) + + @filtered_fylke_ids = fylke_ids + @filtered_kommune_ids = kommune_ids + @filtered_geo_area_ids = [*fylke_ids, *kommune_ids] + end + def fylke_ids_excluding_those_with_a_selected_kommune(unfiltered_fylke_ids, kommune_ids) + return unfiltered_fylke_ids if kommune_ids.blank? + + fylker_for_selected_kommuner = Kommune.where(id: kommune_ids).pluck(:parent_id).uniq + + unfiltered_fylke_ids.reject do |fylke_id| + fylker_for_selected_kommuner.include?(fylke_id) + end + end + + def filter_spaces + set_filters_from_session_or_params set_filterable_facility_categories set_filterable_space_types set_filtered_facilities - filter_by_location - filter_by_title - filter_by_space_types - filter_and_order_by_facilities + @filtered_spaces = Space.all + @filtered_spaces = filter_by_fylker_and_kommuner + @filtered_spaces = filter_by_map_bounds + @filtered_spaces = filter_by_title + @filtered_spaces = filter_by_space_types + @filtered_spaces = filter_and_order_by_facilities store_filters_in_session end @@ -39,36 +61,52 @@ def filter_spaces_for_vector_tiles set_filters_from_session_or_params @filtered_spaces = Space.all - - filter_by_title - filter_by_space_types - filter_and_order_by_facilities + @filtered_spaces = filter_by_fylker_and_kommuner + @filtered_spaces = filter_by_title + @filtered_spaces = filter_by_space_types + @filtered_spaces = filter_and_order_by_facilities end def filter_by_title - return if params[:search_for_title].blank? + return @filtered_spaces if params[:search_for_title].blank? @search_by_title = params[:search_for_title] - @filtered_spaces = @filtered_spaces.filter_on_title(@search_by_title) + @filtered_spaces.filter_on_title(@search_by_title) end def filter_by_space_types - return if params[:space_types].blank? + return @filtered_spaces if params[:space_types].blank? space_types = params[:space_types]&.map(&:to_i) @filtered_space_types = SpaceType.find(space_types) - @filtered_spaces = @filtered_spaces.filter_on_space_types(space_types) + @filtered_spaces.filter_on_space_types(space_types) end - def location_params_present? + def fylke_or_kommune_params_present? + params[:fylker].present? || params[:kommuner].present? + end + + def map_bounding_box_params_present? params[:north_west_lat].present? && params[:north_west_lng].present? && params[:south_east_lat].present? && params[:south_east_lng].present? end - def filter_by_location - return unless location_params_present? + def filter_by_fylker_and_kommuner + return @filtered_spaces unless fylke_or_kommune_params_present? + + set_filtered_geo_area_ids - @filtered_spaces = @filtered_spaces.filter_on_location( + @filtered_spaces + .filter_on_fylker_or_kommuner( + fylke_ids: @filtered_fylke_ids, + kommune_ids: @filtered_kommune_ids + ) + end + + def filter_by_map_bounds + return @filtered_spaces unless map_bounding_box_params_present? + + @filtered_spaces.filter_on_map_bounds( params[:north_west_lat].to_f, params[:north_west_lng].to_f, params[:south_east_lat].to_f, @@ -76,17 +114,17 @@ def filter_by_location ) end - def sanitize_facility_list(array_of_facility_ids) - return [] if array_of_facility_ids.blank? + def sanitize_id_list(array_of_ids) + return [] if array_of_ids.blank? - array_of_facility_ids.uniq.select { |id| id.to_i.to_s == id }.map(&:to_i) + array_of_ids.uniq.select { |id| id.to_i.to_s == id }.map(&:to_i) end def filter_and_order_by_facilities - sanitized_facility_list = sanitize_facility_list(params[:facilities]) - return @filtered_spaces = @filtered_spaces.order_by_star_rating if sanitized_facility_list.blank? + sanitized_facility_list = sanitize_id_list(params[:facilities]) + return @filtered_spaces.order_by_star_rating if sanitized_facility_list.blank? - @filtered_spaces = @filtered_spaces.filter_and_order_by_facilities(sanitized_facility_list) + @filtered_spaces.filter_and_order_by_facilities(sanitized_facility_list) end def any_filters_set? @@ -104,8 +142,6 @@ def store_filters_in_session def filter_keys %w[ - facilities - space_types search_for_title north_west_lat north_west_lng @@ -114,6 +150,15 @@ def filter_keys ] end + def filter_array_keys + { + facilities: [], + space_types: [], + fylker: [], + kommuner: [] + } + end + def set_filters_from_session_or_params return params_from_search if any_filters_set? return params_from_session if any_filters_stored_in_session? @@ -134,7 +179,7 @@ def params_from_search def set_permitted_params remove_duplicate_params - params.permit(*filter_keys, facilities: [], space_types: []) + params.permit(*filter_keys, *filter_array_keys) end def remove_duplicate_params diff --git a/app/controllers/spaces/map_selected_geo_area_controller.rb b/app/controllers/spaces/map_selected_geo_area_controller.rb new file mode 100644 index 00000000..d3490d45 --- /dev/null +++ b/app/controllers/spaces/map_selected_geo_area_controller.rb @@ -0,0 +1,31 @@ +# frozen_string_literal: true + +module Spaces + class MapSelectedGeoAreaController < ApplicationController + include FilterableSpaces + def show + set_filters_from_session_or_params + set_filtered_geo_area_ids + + geo_areas = GeographicalArea.where(id: @filtered_geo_area_ids) + + geojson_features = geo_areas.map do |geo_area| + { + type: "Feature", + geometry: RGeo::GeoJSON.encode(geo_area.geo_area), + properties: { + id: geo_area.id, + name: geo_area.name + } + } + end + + geojson = { + type: "FeatureCollection", + features: geojson_features + } + + render json: geojson + end + end +end diff --git a/app/javascript/controllers/mapbox_controller.js b/app/javascript/controllers/mapbox_controller.js index 99e8e5d2..9c422856 100644 --- a/app/javascript/controllers/mapbox_controller.js +++ b/app/javascript/controllers/mapbox_controller.js @@ -5,7 +5,6 @@ import Rails from "@rails/ujs"; export default class extends Controller { static targets = [ "form", - "location", "northWestLatInput", "northWestLngInput", "southEastLatInput", @@ -81,8 +80,12 @@ export default class extends Controller { this.markersFromSearchResults = {}; this.loadMarkersFromSearchResults(); + this.add_selected_geo_area_layer() + this.add_spaces_vector_layer() this.add_hover_effect_on_spaces_vector_layer() + + this.reload_layers_when_filter_changes() }) } @@ -176,10 +179,6 @@ export default class extends Controller { this.reloadPosition(); } }); - - this.locationTarget.onchange = (event) => { - this.getSearchCoordinatesFromGeoNorge(event) - }; } loadPositionOn(event) { this.map.on(event, () => { @@ -272,6 +271,15 @@ export default class extends Controller { return this.storeBoundsAndMoveMapToFit(this.geoNorgeAvgrensingsBoksToBoundingBox(avgrensningsboks)); } + mapboxglBoundsToBoundingBox(mapboxBounds) { + return { + northWestLat: mapboxBounds.getNorthWest().lat, + northWestLng: mapboxBounds.getNorthWest().lng, + southEastLat: mapboxBounds.getSouthEast().lat, + southEastLng: mapboxBounds.getSouthEast().lng + } + } + geoNorgeAvgrensingsBoksToBoundingBox = (avgrensningsboks) => { const [northWestLng, northWestLat] = avgrensningsboks[1]; const [southEastLng, southEastLat] = avgrensningsboks[3]; @@ -362,23 +370,109 @@ export default class extends Controller { } } - add_spaces_vector_layer() { - const formData = new FormData(this.formTarget); - const filter_params = new URLSearchParams(formData).toString(); + reload_layers_when_filter_changes() { this.formTarget.onchange = (e) => { if (e.target.name === 'north_west_lat' || e.target.name === 'north_west_lng' || e.target.name === 'south_east_lat' || e.target.name === 'south_east_lng') { return; } - this.debounce("reloadVectorTiles", () => { + this.debounce("reload_layers", () => { + this.map.removeLayer('selected_geo_area_layer'); + this.map.removeSource('selected_geo_area_source'); + this.add_selected_geo_area_layer(); + this.map.removeLayer('spaces_vector_layer'); this.map.removeSource('spaces_vector_source'); this.add_spaces_vector_layer(); }, 100); } + } + + async add_selected_geo_area_layer() { + const formData = new FormData(this.formTarget); + const filter_params = new URLSearchParams(formData).toString(); + this.selected_geo_areas_string = JSON.stringify(formData.getAll('fylker[]').concat(formData.getAll('kommuner[]'))); + + this.selected_geo_areas_geo_json = await fetch(`${window.location.origin}/lokaler/map_selected_geo_area?${filter_params}`).then(response => response.json()); + + this.map.addSource('selected_geo_area_source', { + type: 'geojson', + data: this.selected_geo_areas_geo_json + }); + + this.map.addLayer({ + id: 'selected_geo_area_layer', + type: 'fill', + source: 'selected_geo_area_source', + paint: { + 'fill-color': '#C40066', + 'fill-opacity': 0.1 + } + }); - console.log("Adding filtered spaces to map") + this.map.on('sourcedata', this.fitMapToSelectedGeoArea.bind(this)); + } + + fitMapToSelectedGeoArea(e) { + if (e.sourceId !== 'selected_geo_area_source' || !e.isSourceLoaded) { + return; + } + + if (!!this.selected_geo_areas_string && !!this.previously_selected_geo_areas_string && this.selected_geo_areas_string === this.previously_selected_geo_areas_string) { + return; + } + + const features = this.selected_geo_areas_geo_json.features; + if (!features || features.length === 0) { + return; + } + + this.previously_selected_geo_areas_string = `${this.selected_geo_areas_string}`; + + + // Get the bounds of the source data + let bounds = new mapboxgl.LngLatBounds(); + + features.forEach(function(feature) { + switch (feature.geometry.type) { + case 'Point': + bounds.extend(feature.geometry.coordinates); + break; + case 'LineString': + feature.geometry.coordinates.forEach(function(coord) { + bounds.extend(coord); + }); + break; + case 'Polygon': + feature.geometry.coordinates[0].forEach(function(coord) { + bounds.extend(coord); + }); + break; + case 'MultiPolygon': + feature.geometry.coordinates.forEach(function(polygon) { + polygon[0].forEach(function(coord) { + bounds.extend(coord); + }); + }); + break; + default: + console.warn('Unsupported geometry type:', feature.geometry.type); + } + }); + + const formattedBounds = this.mapboxglBoundsToBoundingBox(bounds); + + // Fit the map to the bounds + this.storeBoundsAndMoveMapToFit(formattedBounds); + + } + + + + add_spaces_vector_layer() { + const formData = new FormData(this.formTarget); + const filter_params = new URLSearchParams(formData).toString(); this.map.addSource('spaces_vector_source', { type: 'vector', diff --git a/app/models/space.rb b/app/models/space.rb index 178a6e33..88e76e56 100644 --- a/app/models/space.rb +++ b/app/models/space.rb @@ -35,7 +35,10 @@ class Space < ApplicationRecord # rubocop:disable Metrics/ClassLength scope :filter_on_space_types, lambda { |space_type_ids| joins(:space_types).where(space_types: { id: space_type_ids }) } - scope :filter_on_location, lambda { |north_west_lat, north_west_lng, south_east_lat, south_east_lng| + scope :filter_on_fylker_or_kommuner, lambda { |fylke_ids:, kommune_ids:| + where(fylke_id: fylke_ids).or(where(kommune_id: kommune_ids)) + } + scope :filter_on_map_bounds, lambda { |north_west_lat, north_west_lng, south_east_lat, south_east_lng| where(":north_west_lat >= lat AND :north_west_lng <= lng AND :south_east_lat <= lat AND :south_east_lng >= lng", north_west_lat:, north_west_lng:, diff --git a/app/views/spaces/index/search_form_fields/_filter_on_location.html.erb b/app/views/spaces/index/search_form_fields/_filter_on_location.html.erb index 6dbc490f..fd6ca1a5 100644 --- a/app/views/spaces/index/search_form_fields/_filter_on_location.html.erb +++ b/app/views/spaces/index/search_form_fields/_filter_on_location.html.erb @@ -1,11 +1,10 @@ <%# locals: (form:) %> -<%= form.label t("space_filter.location"), for: "locationInput", class: "text-xl font-bold my-4" %> +

<%= t("space_filter.location") %>

-
- <% Fylke.all.each do |fylke| %> - <% fylke_checked = params[:fylke]&.include?(fylke.id.to_s) %> + <% Fylke.includes(:kommuner).all.each do |fylke| %> + <% fylke_checked = params[:fylker]&.include?(fylke.id.to_s) %> <%= form.label "geo_area_#{fylke.id}", class: "flex gap-1 my-1 items-top" do %> - <%= form.check_box :fylke, + <%= form.check_box :fylker, { id: "geo_area_#{fylke.id}", multiple: true, @@ -19,18 +18,18 @@ <%= fylke.name %> <% end %> - <% next unless fylke_checked and fylke.kommuner.any? or fylke.kommuner.find_by(id: params[:kommune]).present? %> + <% next unless fylke_checked and fylke.kommuner.any? or fylke.kommuner.find_by(id: params[:kommuner]).present? %>
<% fylke.kommuner.each do |kommune| %> <%= form.label "geo_area_#{kommune.id}", class: "flex gap-1 my-1 items-top" do %> - <%= form.check_box :kommune, + <%= form.check_box :kommuner, { id: "geo_area_#{kommune.id}", multiple: true, class: "h-5 w-5 relative top-0.5 text-lnu-pink focus:ring-lnu-pink border-gray-300 rounded", onchange: "this.form.requestSubmit()", - checked: params[:kommune]&.include?(kommune.id.to_s) + checked: params[:kommuner]&.include?(kommune.id.to_s) }, kommune.id.to_s, nil @@ -41,9 +40,9 @@
<% end %> + <%= form.hidden_field :north_west_lat, value: params[:north_west_lat], class: "text_field", data: { mapbox_target: "northWestLatInput" } %> <%= form.hidden_field :north_west_lng, value: params[:north_west_lng], class: "text_field", data: { mapbox_target: "northWestLngInput" } %> <%= form.hidden_field :south_east_lat, value: params[:south_east_lat], class: "text_field", data: { mapbox_target: "southEastLatInput" } %> <%= form.hidden_field :south_east_lng, value: params[:south_east_lng], class: "text_field", data: { mapbox_target: "southEastLngInput" } %> -
diff --git a/config/routes.rb b/config/routes.rb index b5ccbba4..042e3b26 100644 --- a/config/routes.rb +++ b/config/routes.rb @@ -37,6 +37,7 @@ post "/lokaler/:space_id/facility_review/:facility_id", to: "facility_reviews#create", as: "create_facility_review" get "/lokaler/mapbox_vector_tiles/:z/:x/:y", to: "spaces/map_vector#show", as: "space_map_vector" get "/lokaler/:space_id/map_marker", to: "spaces/map_marker#show", as: "map_marker" + get "/lokaler/map_selected_geo_area", to: "spaces/map_selected_geo_area#show", as: "map_selected_geo_area" # Review routes resources "reviews", except: "new" From 1ceb2e55e75c924c31dc6c41fa719bb3570a794d Mon Sep 17 00:00:00 2001 From: Daniel Jackson Date: Tue, 1 Oct 2024 12:15:37 +0200 Subject: [PATCH 08/16] Add toggle for filter while map is moving --- app/controllers/concerns/filterable_spaces.rb | 7 ++++- .../controllers/mapbox_controller.js | 30 +++++++++++++++---- .../_filter_on_location.html.erb | 13 ++++++++ config/locales/space_filter.nb.yml | 2 +- 4 files changed, 44 insertions(+), 8 deletions(-) diff --git a/app/controllers/concerns/filterable_spaces.rb b/app/controllers/concerns/filterable_spaces.rb index 8e64caee..c626884b 100644 --- a/app/controllers/concerns/filterable_spaces.rb +++ b/app/controllers/concerns/filterable_spaces.rb @@ -49,7 +49,7 @@ def filter_spaces @filtered_spaces = Space.all @filtered_spaces = filter_by_fylker_and_kommuner - @filtered_spaces = filter_by_map_bounds + @filtered_spaces = filter_by_map_bounds unless filter_by_map_bounds_turned_off? @filtered_spaces = filter_by_title @filtered_spaces = filter_by_space_types @filtered_spaces = filter_and_order_by_facilities @@ -127,6 +127,10 @@ def filter_and_order_by_facilities @filtered_spaces.filter_and_order_by_facilities(sanitized_facility_list) end + def filter_by_map_bounds_turned_off? + params[:filter_by_map_bounds] == "off" + end + def any_filters_set? filter_keys.detect { |key| params[key] } end @@ -142,6 +146,7 @@ def store_filters_in_session def filter_keys %w[ + filter_by_map_bounds search_for_title north_west_lat north_west_lng diff --git a/app/javascript/controllers/mapbox_controller.js b/app/javascript/controllers/mapbox_controller.js index 9c422856..d90310e9 100644 --- a/app/javascript/controllers/mapbox_controller.js +++ b/app/javascript/controllers/mapbox_controller.js @@ -378,12 +378,7 @@ export default class extends Controller { } this.debounce("reload_layers", () => { - this.map.removeLayer('selected_geo_area_layer'); - this.map.removeSource('selected_geo_area_source'); this.add_selected_geo_area_layer(); - - this.map.removeLayer('spaces_vector_layer'); - this.map.removeSource('spaces_vector_source'); this.add_spaces_vector_layer(); }, 100); } @@ -392,10 +387,22 @@ export default class extends Controller { async add_selected_geo_area_layer() { const formData = new FormData(this.formTarget); const filter_params = new URLSearchParams(formData).toString(); + const selected_geo_areas = formData.getAll('fylker[]').concat(formData.getAll('kommuner[]')); this.selected_geo_areas_string = JSON.stringify(formData.getAll('fylker[]').concat(formData.getAll('kommuner[]'))); + if (selected_geo_areas.length === 0 && this.selectedGeoAreaHasChanged()) { + return this.moveMapToFitNorway(); + } + this.selected_geo_areas_geo_json = await fetch(`${window.location.origin}/lokaler/map_selected_geo_area?${filter_params}`).then(response => response.json()); + if (this.map.getLayer('selected_geo_area_layer')) { + this.map.removeLayer('selected_geo_area_layer'); + } + if (this.map.getSource('selected_geo_area_source')) { + this.map.removeSource('selected_geo_area_source'); + } + this.map.addSource('selected_geo_area_source', { type: 'geojson', data: this.selected_geo_areas_geo_json @@ -414,12 +421,16 @@ export default class extends Controller { this.map.on('sourcedata', this.fitMapToSelectedGeoArea.bind(this)); } + selectedGeoAreaHasChanged() { + return !!this.selected_geo_areas_string && !!this.previously_selected_geo_areas_string && this.selected_geo_areas_string === this.previously_selected_geo_areas_string; + } + fitMapToSelectedGeoArea(e) { if (e.sourceId !== 'selected_geo_area_source' || !e.isSourceLoaded) { return; } - if (!!this.selected_geo_areas_string && !!this.previously_selected_geo_areas_string && this.selected_geo_areas_string === this.previously_selected_geo_areas_string) { + if (this.selectedGeoAreaHasChanged()) { return; } @@ -474,6 +485,13 @@ export default class extends Controller { const formData = new FormData(this.formTarget); const filter_params = new URLSearchParams(formData).toString(); + if (this.map.getLayer('spaces_vector_layer')) { + this.map.removeLayer('spaces_vector_layer'); + } + if (this.map.getSource('spaces_vector_source')) { + this.map.removeSource('spaces_vector_source'); + } + this.map.addSource('spaces_vector_source', { type: 'vector', tiles: [window.location.origin + '/lokaler/mapbox_vector_tiles/{z}/{x}/{y}.mvt?' + filter_params], diff --git a/app/views/spaces/index/search_form_fields/_filter_on_location.html.erb b/app/views/spaces/index/search_form_fields/_filter_on_location.html.erb index fd6ca1a5..1dc3009f 100644 --- a/app/views/spaces/index/search_form_fields/_filter_on_location.html.erb +++ b/app/views/spaces/index/search_form_fields/_filter_on_location.html.erb @@ -1,6 +1,19 @@ <%# locals: (form:) %>

<%= t("space_filter.location") %>

+ <%= form.label :filter_by_map_bounds, class: "flex gap-1 items-top" do %> + <%= form.check_box :filter_by_map_bounds, + { + class: "h-5 w-5 relative top-0.5 text-lnu-pink focus:ring-lnu-pink border-gray-300 rounded", + onchange: "this.form.requestSubmit()", + checked: params[:filter_by_map_bounds] == "off" ? nil : "checked" + }, + 'on', + 'off' + %> + <%= t("space_filter.filter_by_map_bounds") %> + <% end %> +
<% Fylke.includes(:kommuner).all.each do |fylke| %> <% fylke_checked = params[:fylker]&.include?(fylke.id.to_s) %> <%= form.label "geo_area_#{fylke.id}", class: "flex gap-1 my-1 items-top" do %> diff --git a/config/locales/space_filter.nb.yml b/config/locales/space_filter.nb.yml index bc33e29d..f6df8342 100644 --- a/config/locales/space_filter.nb.yml +++ b/config/locales/space_filter.nb.yml @@ -15,7 +15,7 @@ nb: edit_filter: 'Rediger filtre' location: 'Område' choose_location: 'Søk her, eller flytt på kart' - set_location_by_map: 'Søk i kartet' + filter_by_map_bounds: 'Filtrer når kartet flyttes' title: 'Navn på lokalet' more_filters: 'Flere filtre' map_reload: 'Søk i området' From d8ad2e89605d2d11661a0aa74919ba181e990d14 Mon Sep 17 00:00:00 2001 From: Daniel Jackson Date: Tue, 1 Oct 2024 12:30:10 +0200 Subject: [PATCH 09/16] Open and close filters when loading them --- app/views/spaces/index.html.erb | 4 ++++ app/views/spaces/index/_search_form.html.erb | 23 +++++++++---------- .../_filter_on_location.html.erb | 3 +-- 3 files changed, 16 insertions(+), 14 deletions(-) diff --git a/app/views/spaces/index.html.erb b/app/views/spaces/index.html.erb index 924a21ce..51d77fd8 100644 --- a/app/views/spaces/index.html.erb +++ b/app/views/spaces/index.html.erb @@ -18,6 +18,10 @@ flex flex-col-reverse md:flex-row md:flex-1" do %> + <%= turbo_stream.replace "search_form" do %> + <%= render partial: "spaces/index/search_form" %> + <% end %> +
- +<%= turbo_frame_tag 'search_form' do %> + <%= form_with url: spaces_path, method: :get, data: { + turbo_frame: 'search_results', + controller: 'sync-fields-with-same-id', + sync_fields_with_same_id_target: 'form', + mapbox_target: 'form', + search_target: 'form' + } do |form| %> + <%= render 'spaces/index/search_form_fields', form: %> + <%= form.submit "Søk", class: "button submit fixed bottom-4 sr-only" %> + <% end %> <% end %> diff --git a/app/views/spaces/index/search_form_fields/_filter_on_location.html.erb b/app/views/spaces/index/search_form_fields/_filter_on_location.html.erb index 1dc3009f..691b79f0 100644 --- a/app/views/spaces/index/search_form_fields/_filter_on_location.html.erb +++ b/app/views/spaces/index/search_form_fields/_filter_on_location.html.erb @@ -52,10 +52,9 @@ <% end %>
<% end %> - - <%= form.hidden_field :north_west_lat, value: params[:north_west_lat], class: "text_field", data: { mapbox_target: "northWestLatInput" } %> <%= form.hidden_field :north_west_lng, value: params[:north_west_lng], class: "text_field", data: { mapbox_target: "northWestLngInput" } %> <%= form.hidden_field :south_east_lat, value: params[:south_east_lat], class: "text_field", data: { mapbox_target: "southEastLatInput" } %> <%= form.hidden_field :south_east_lng, value: params[:south_east_lng], class: "text_field", data: { mapbox_target: "southEastLngInput" } %> +
From 471ba4e337e45e2ac29578bb00737783a26e131e Mon Sep 17 00:00:00 2001 From: Daniel Jackson Date: Tue, 1 Oct 2024 12:37:37 +0200 Subject: [PATCH 10/16] testing someting for heroku --- db/seeds/imports/import_geo_data_over_api_functions.rb | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/db/seeds/imports/import_geo_data_over_api_functions.rb b/db/seeds/imports/import_geo_data_over_api_functions.rb index e78d1e32..b245a010 100644 --- a/db/seeds/imports/import_geo_data_over_api_functions.rb +++ b/db/seeds/imports/import_geo_data_over_api_functions.rb @@ -42,7 +42,7 @@ def load_fylker_from_geonorge geo_area: load_geo_area_from_geonorge(FYLKE_API_END_POINT, external_id) ) - throw "Error loading fylke #{external_id}: #{fylke_in_db.errors}" unless fylke_in_db.persisted? + # throw "Error loading fylke #{external_id}: #{fylke_in_db.errors}" unless fylke_in_db.persisted? Rails.logger.debug { "\rLoaded #{external_id} #{fylke['fylkesnavn']} " } end @@ -93,7 +93,7 @@ def load_kommuner_from_geonorge geo_area: load_geo_area_from_geonorge(KOMMUNE_API_END_POINT, kommunenummer) ) - throw "Error loading kommune #{kommunenummer}: #{kommune_in_db.errors}" unless kommune_in_db.persisted? + # throw "Error loading kommune #{kommunenummer}: #{kommune_in_db.errors}" unless kommune_in_db.persisted? Rails.logger.debug { "\rLoaded #{kommunenummer} #{navn} " } end From d2e31ef69227f35454ac82cacbae39c5ab8982d3 Mon Sep 17 00:00:00 2001 From: Daniel Jackson Date: Tue, 1 Oct 2024 13:04:03 +0200 Subject: [PATCH 11/16] Fixing for Heroku --- .../with_parent_child_relationship.rb | 2 + app/models/fylke.rb | 2 + app/models/geographical_area.rb | 2 - app/models/kommune.rb | 2 + .../import_geo_data_over_api_functions.rb | 41 ++++++++++--------- 5 files changed, 27 insertions(+), 22 deletions(-) diff --git a/app/models/concerns/with_parent_child_relationship.rb b/app/models/concerns/with_parent_child_relationship.rb index 24875f28..80c58aa8 100644 --- a/app/models/concerns/with_parent_child_relationship.rb +++ b/app/models/concerns/with_parent_child_relationship.rb @@ -6,6 +6,8 @@ module WithParentChildRelationship private def no_circular_references + return if parent_id.blank? + return if id.blank? return unless parent_id == id errors.add(:base, "Cannot add itself as a parent") diff --git a/app/models/fylke.rb b/app/models/fylke.rb index d6313108..984fb0d5 100644 --- a/app/models/fylke.rb +++ b/app/models/fylke.rb @@ -13,6 +13,8 @@ class Fylke < GeographicalArea inverse_of: :fylke, dependent: :destroy + has_many :spaces, dependent: :nullify, inverse_of: :fylke + before_validation :set_geographical_area_type private diff --git a/app/models/geographical_area.rb b/app/models/geographical_area.rb index 508322dc..7fb0daf0 100644 --- a/app/models/geographical_area.rb +++ b/app/models/geographical_area.rb @@ -7,8 +7,6 @@ class GeographicalArea < ApplicationRecord has_many :children, class_name: "GeographicalArea", foreign_key: "parent_id", dependent: :nullify, inverse_of: :parent validate :no_circular_references - has_many :spaces, dependent: :nullify - belongs_to :geographical_area_type validates :name, presence: true diff --git a/app/models/kommune.rb b/app/models/kommune.rb index 7ca02eb3..10a67890 100644 --- a/app/models/kommune.rb +++ b/app/models/kommune.rb @@ -9,6 +9,8 @@ class Kommune < GeographicalArea before_validation :set_geographical_area_type + has_many :spaces, dependent: :nullify, inverse_of: :kommune + private def set_geographical_area_type diff --git a/db/seeds/imports/import_geo_data_over_api_functions.rb b/db/seeds/imports/import_geo_data_over_api_functions.rb index b245a010..0d2d57c9 100644 --- a/db/seeds/imports/import_geo_data_over_api_functions.rb +++ b/db/seeds/imports/import_geo_data_over_api_functions.rb @@ -19,17 +19,16 @@ def load_geo_area_from_geonorge(uri_prefix, uri_id) FYLKE_API_END_POINT = "https://ws.geonorge.no/kommuneinfo/v1/fylker" def load_fylker_from_geonorge + # rubocop:disable Rails/Output geographical_area_type = GeographicalAreaType.find_or_create_by(name: "Fylke") - Rails.logger.debug do - "Loading fylker from API. We currently have: #{GeographicalArea.where(geographical_area_type:).count} fylker" - end + p "Loading fylker from API. We currently have: #{GeographicalArea.where(geographical_area_type:).count} fylker" uri = URI.parse(FYLKE_API_END_POINT) response = Net::HTTP.get_response(uri) fylker = JSON.parse(response.body) - Rails.logger.debug { "API has #{fylker.size} fylker to sync with" } + p "API has #{fylker.size} fylker to sync with" fylker.each do |fylke| external_id = fylke["fylkesnummer"] @@ -42,36 +41,37 @@ def load_fylker_from_geonorge geo_area: load_geo_area_from_geonorge(FYLKE_API_END_POINT, external_id) ) - # throw "Error loading fylke #{external_id}: #{fylke_in_db.errors}" unless fylke_in_db.persisted? + p fylke_in_db.errors.full_messages if fylke_in_db.errors.any? + throw "Error loading fylke #{external_id}: #{fylke_in_db.errors.full_messages}" unless fylke_in_db.persisted? - Rails.logger.debug { "\rLoaded #{external_id} #{fylke['fylkesnavn']} " } + print "\rLoaded #{external_id} #{fylke['fylkesnavn']} " end - Rails.logger.debug "\rRemoving any fylker that are not in the API \n" + print "\rRemoving any fylker that are not in the API \n" GeographicalArea .where(geographical_area_type:) .where.not(unique_id_for_external_source: fylker.pluck("fylkesnummer")) .destroy_all - Rails.logger.debug do - "\rLoaded fylker from API. We now have: #{GeographicalArea.where(geographical_area_type:).count} fylker \n" - end + print "\rLoaded fylker from API. We now have: #{GeographicalArea.where(geographical_area_type:).count} fylker \n" + + # rubocop:enable Rails/Output end KOMMUNE_API_END_POINT = "https://ws.geonorge.no/kommuneinfo/v1/kommuner" def load_kommuner_from_geonorge + # rubocop:disable Rails/Output + geographical_area_type = GeographicalAreaType.find_or_create_by(name: "Kommune") - Rails.logger.debug do - "Loading kommuner from API. We currently have: #{GeographicalArea.where(geographical_area_type:).count} kommuner" - end + p "Loading kommuner from API. We currently have: #{GeographicalArea.where(geographical_area_type:).count} kommuner" uri = URI.parse(KOMMUNE_API_END_POINT) response = Net::HTTP.get_response(uri) kommuner = JSON.parse(response.body) - Rails.logger.debug { "API has #{kommuner.size} kommuner to sync with" } + p "API has #{kommuner.size} kommuner to sync with" kommuner.each do |kommune| kommunenummer = kommune["kommunenummer"] @@ -93,20 +93,21 @@ def load_kommuner_from_geonorge geo_area: load_geo_area_from_geonorge(KOMMUNE_API_END_POINT, kommunenummer) ) - # throw "Error loading kommune #{kommunenummer}: #{kommune_in_db.errors}" unless kommune_in_db.persisted? + throw "Error loading kommune #{kommunenummer}: #{kommune_in_db.errors}" unless kommune_in_db.persisted? - Rails.logger.debug { "\rLoaded #{kommunenummer} #{navn} " } + print "\rLoaded #{kommunenummer} #{navn} " end - Rails.logger.debug "\rRemoving any kommuner that are not in the API \n" + print "\rRemoving any kommuner that are not in the API \n" GeographicalArea .where(geographical_area_type:) .where.not(unique_id_for_external_source: kommuner.pluck("kommunenummer")) .destroy_all - Rails.logger.debug do - "\rLoaded kommuner from API. We now have: #{GeographicalArea.where(geographical_area_type:).count} kommuner \n" - end + count = GeographicalArea.where(geographical_area_type:).count + print "\rLoaded kommuner from API. We now have: #{count} kommuner \n" + + # rubocop:enable Rails/Output end # rubocop:enable Metrics/methodLength Metrics/AbcSize From 9ffce4588d91abd4f993c0e690dd7b34f97dbf47 Mon Sep 17 00:00:00 2001 From: Daniel Jackson Date: Tue, 1 Oct 2024 13:27:46 +0200 Subject: [PATCH 12/16] Nested checkboxes for areas --- .../nested_checkbox_filter_controller.js | 25 +++++-- app/views/spaces/index.html.erb | 4 -- .../_filter_on_facilities.html.erb | 5 +- .../_filter_on_location.html.erb | 67 +++++++++++-------- 4 files changed, 63 insertions(+), 38 deletions(-) diff --git a/app/javascript/controllers/nested_checkbox_filter_controller.js b/app/javascript/controllers/nested_checkbox_filter_controller.js index 2a48b5c1..270afa58 100644 --- a/app/javascript/controllers/nested_checkbox_filter_controller.js +++ b/app/javascript/controllers/nested_checkbox_filter_controller.js @@ -7,6 +7,8 @@ export default class extends Controller { initialize() { this.toggle = this.toggle.bind(this) this.refresh = this.refresh.bind(this) + this.checkAllIfParentIsChecked = this.element.dataset.checkAllIfParentIsChecked === "true" + this.hideUnlessParentIsChecked = this.element.dataset.hideUnlessParentIsChecked === "true" } parentTargetConnected(parentCheckbox) { @@ -36,10 +38,16 @@ export default class extends Controller { toggle(e) { e.preventDefault() - this.childTargets.forEach((checkbox) => { - checkbox.checked = e.target.checked - this.triggerInputEvent(checkbox) - }) + if (this.checkAllIfParentIsChecked || this.parentTarget.checked === false) { + this.childTargets.forEach((checkbox) => { + checkbox.checked = e.target.checked + this.triggerInputEvent(checkbox) + }) + } + + if (this.hideUnlessParentIsChecked) { + this.childrenContainerTarget.classList.toggle('hidden', !this.parentTarget.checked) + } this.parentTarget.form.requestSubmit() } @@ -48,8 +56,15 @@ export default class extends Controller { const checkboxesCount = this.childTargets.length const checkboxesCheckedCount = this.checked.length - this.parentTarget.checked = checkboxesCheckedCount > 0 + if (this.checkAllIfParentIsChecked) { + this.parentTarget.checked = checkboxesCheckedCount > 0 + } + this.parentTarget.indeterminate = checkboxesCheckedCount > 0 && checkboxesCheckedCount < checkboxesCount + + if (this.hideUnlessParentIsChecked) { + this.childrenContainerTarget.classList.toggle('hidden', !this.parentTarget.checked) + } } triggerInputEvent(checkbox) { diff --git a/app/views/spaces/index.html.erb b/app/views/spaces/index.html.erb index 51d77fd8..924a21ce 100644 --- a/app/views/spaces/index.html.erb +++ b/app/views/spaces/index.html.erb @@ -18,10 +18,6 @@ flex flex-col-reverse md:flex-row md:flex-1" do %> - <%= turbo_stream.replace "search_form" do %> - <%= render partial: "spaces/index/search_form" %> - <% end %> -
+
+ <%= form.hidden_field :north_west_lat, value: params[:north_west_lat], class: "text_field", data: { mapbox_target: "northWestLatInput" } %> <%= form.hidden_field :north_west_lng, value: params[:north_west_lng], class: "text_field", data: { mapbox_target: "northWestLngInput" } %> <%= form.hidden_field :south_east_lat, value: params[:south_east_lat], class: "text_field", data: { mapbox_target: "southEastLatInput" } %> <%= form.hidden_field :south_east_lng, value: params[:south_east_lng], class: "text_field", data: { mapbox_target: "southEastLngInput" } %> From f8f67644471174079ec215d70c69ac3c5a04d2e4 Mon Sep 17 00:00:00 2001 From: Daniel Jackson Date: Tue, 1 Oct 2024 14:11:46 +0200 Subject: [PATCH 13/16] Disable caching of vector tiles until I understand how they work --- app/controllers/spaces/map_vector_controller.rb | 11 ++++++----- 1 file changed, 6 insertions(+), 5 deletions(-) diff --git a/app/controllers/spaces/map_vector_controller.rb b/app/controllers/spaces/map_vector_controller.rb index dd9af6ad..a310742c 100644 --- a/app/controllers/spaces/map_vector_controller.rb +++ b/app/controllers/spaces/map_vector_controller.rb @@ -10,11 +10,12 @@ class MapVectorController < ApplicationController VECTOR_TILE_CACHE_KEY_PREFIX = "spaces_vector_tile/" - def mvt_data_to_use - cached_mvt_data_for_tile(cache_key_prefix: VECTOR_TILE_CACHE_KEY_PREFIX) - # This cache never expires. To clear it, do - # Rails.cache.delete_matched("venstre_score_vector/*") - end + # TODO: Re-enable caching once I understand how it works on Heroku + # def mvt_data_to_use + # cached_mvt_data_for_tile(cache_key_prefix: VECTOR_TILE_CACHE_KEY_PREFIX) + # # This cache never expires. To clear it, do + # # Rails.cache.delete_matched("spaces_vector_tile/*") + # end def pre_filter_spaces_subquery filter_spaces_for_vector_tiles From a05a264a49fd8d9b4e65a76c94e6855472d1549a Mon Sep 17 00:00:00 2001 From: Daniel Jackson Date: Tue, 1 Oct 2024 20:41:47 +0200 Subject: [PATCH 14/16] Make sure filters are stored in session again --- app/controllers/concerns/filterable_spaces.rb | 36 ++++++++++++++----- .../spaces/map_vector_controller.rb | 2 +- 2 files changed, 28 insertions(+), 10 deletions(-) diff --git a/app/controllers/concerns/filterable_spaces.rb b/app/controllers/concerns/filterable_spaces.rb index c626884b..115ed1ee 100644 --- a/app/controllers/concerns/filterable_spaces.rb +++ b/app/controllers/concerns/filterable_spaces.rb @@ -43,6 +43,8 @@ def fylke_ids_excluding_those_with_a_selected_kommune(unfiltered_fylke_ids, komm def filter_spaces set_filters_from_session_or_params + store_filters_in_session + set_filterable_facility_categories set_filterable_space_types set_filtered_facilities @@ -53,8 +55,6 @@ def filter_spaces @filtered_spaces = filter_by_title @filtered_spaces = filter_by_space_types @filtered_spaces = filter_and_order_by_facilities - - store_filters_in_session end def filter_spaces_for_vector_tiles @@ -132,19 +132,22 @@ def filter_by_map_bounds_turned_off? end def any_filters_set? - filter_keys.detect { |key| params[key] } + all_filter_keys.detect { |key| params[key] } end def any_filters_stored_in_session? session[:last_filter_params].present? && - filter_keys.any? { |key| session[:last_filter_params][key].present? } + all_filter_keys.any? { |key| session[:last_filter_params][key].present? } end def store_filters_in_session - session[:last_filter_params] = params_from_search + session[:last_filter_params] = {} + all_filter_keys.each do |key| + session[:last_filter_params][key] = params[key] + end end - def filter_keys + def singular_filters %w[ filter_by_map_bounds search_for_title @@ -155,7 +158,7 @@ def filter_keys ] end - def filter_array_keys + def filter_arrays { facilities: [], space_types: [], @@ -164,6 +167,20 @@ def filter_array_keys } end + def all_filters + [ + *singular_filters, + **filter_arrays + ] + end + + def all_filter_keys + [ + *singular_filters, + *filter_arrays.keys.map(&:to_s) + ] + end + def set_filters_from_session_or_params return params_from_search if any_filters_set? return params_from_session if any_filters_stored_in_session? @@ -172,9 +189,10 @@ def set_filters_from_session_or_params end def params_from_session - filter_keys.each do |key| + all_filter_keys.each do |key| params[key] = session[:last_filter_params][key] end + set_permitted_params end @@ -184,7 +202,7 @@ def params_from_search def set_permitted_params remove_duplicate_params - params.permit(*filter_keys, *filter_array_keys) + params.permit(all_filters) end def remove_duplicate_params diff --git a/app/controllers/spaces/map_vector_controller.rb b/app/controllers/spaces/map_vector_controller.rb index a310742c..4b8fe5bb 100644 --- a/app/controllers/spaces/map_vector_controller.rb +++ b/app/controllers/spaces/map_vector_controller.rb @@ -25,7 +25,7 @@ def pre_filter_spaces_subquery def set_permitted_params remove_duplicate_params - params.permit([*filter_keys, :z, :x, :y, { facilities: [], space_types: [] }]) + params.permit([:z, :x, :y, *all_filters]) end def vector_tile_query # rubocop:disable Metrics/MethodLength From dd49a21e84d4b2f5aa696bba99ca0ce6312c6199 Mon Sep 17 00:00:00 2001 From: Daniel Jackson Date: Tue, 1 Oct 2024 20:42:41 +0200 Subject: [PATCH 15/16] Make sure markers are cleared before new ones are added --- app/javascript/controllers/mapbox_controller.js | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/app/javascript/controllers/mapbox_controller.js b/app/javascript/controllers/mapbox_controller.js index d90310e9..07d7238c 100644 --- a/app/javascript/controllers/mapbox_controller.js +++ b/app/javascript/controllers/mapbox_controller.js @@ -343,9 +343,8 @@ export default class extends Controller { async loadMarkersFromSearchResults() { const new_markers = JSON.parse(this.markerContainerTarget.dataset.markers) || []; - // Remove markers that are no longer relevant + // Remove old markers Object.keys(this.markersFromSearchResults).forEach((key) => { - if (new_markers.find((space) => space.id === key)) return; this.removeSearchResultMarker(key); }); From fd96e0986c91557bd2e7f408f9f7851a40714d2b Mon Sep 17 00:00:00 2001 From: Daniel Jackson Date: Tue, 1 Oct 2024 21:01:06 +0200 Subject: [PATCH 16/16] Make sure we default to remembering logins, and remember them both for google oauth and email+password --- app/controllers/users/omniauth_callbacks_controller.rb | 3 +++ app/views/devise/sessions/new.html.erb | 2 +- config/initializers/devise.rb | 2 +- config/routes.rb | 6 +----- 4 files changed, 6 insertions(+), 7 deletions(-) diff --git a/app/controllers/users/omniauth_callbacks_controller.rb b/app/controllers/users/omniauth_callbacks_controller.rb index ab0c9412..dfb45dc4 100644 --- a/app/controllers/users/omniauth_callbacks_controller.rb +++ b/app/controllers/users/omniauth_callbacks_controller.rb @@ -2,10 +2,13 @@ module Users class OmniauthCallbacksController < Devise::OmniauthCallbacksController + include Devise::Controllers::Rememberable + def google_oauth2 # rubocop:disable Metrics/AbcSize user = User.from_google(email: auth.info.email, first_name: auth.info.first_name, last_name: auth.info.last_name) if user.present? + remember_me(user) sign_out_all_scopes sign_in_and_redirect user else diff --git a/app/views/devise/sessions/new.html.erb b/app/views/devise/sessions/new.html.erb index a1fa25a6..0a7a2dfa 100644 --- a/app/views/devise/sessions/new.html.erb +++ b/app/views/devise/sessions/new.html.erb @@ -47,7 +47,7 @@ <%= t('simple_form.labels.user.session.login_with_email') %> <% end %> <% if devise_mapping.rememberable? %> - <%= f.input :remember_me, as: :boolean, wrapper_html: { class: "flex items-center" } %> + <%= f.input :remember_me, as: :boolean, input_html: {checked: 'checked'}, wrapper_html: { class: "flex items-center" } %> <% end %>
<% end %> diff --git a/config/initializers/devise.rb b/config/initializers/devise.rb index fe8d3689..52a58ab6 100644 --- a/config/initializers/devise.rb +++ b/config/initializers/devise.rb @@ -98,7 +98,7 @@ # Notice that if you are skipping storage for all authentication paths, you # may want to disable generating routes to Devise's sessions controller by # passing skip: :sessions to `devise_for` in your config/routes.rb - config.skip_session_storage = [:http_auth] + # config.skip_session_storage = [:http_auth] # By default, Devise cleans up the CSRF token on authentication to # avoid CSRF token fixation attacks. This means that, when using AJAX diff --git a/config/routes.rb b/config/routes.rb index 042e3b26..96ec1d33 100644 --- a/config/routes.rb +++ b/config/routes.rb @@ -14,14 +14,10 @@ devise_scope :user do authenticated :user do root to: "spaces#index" + get "session", to: "devise/sessions#edit", as: "edit_session" end end - # Devise addons - devise_scope :user do - get "session", to: "devise/sessions#edit", as: "edit_session" - end - # Homepage for unauthenticated users root to: "high_voltage/pages#show", id: "frontpage", as: "unauthenticated_root"