uk polygons with backfill of uk_geolocation

Author: starswan

.simplecov

@@ -54,8 +54,8 @@ if ENV.fetch("COVERAGE", 0).to_i.positive?
     # .25% lower (branch) and .1% lower (line) than the test run for now
     #
     # possibly the tests are stable now?
-    # SD 16/01/26 branch coverage still appears to be ~ .1% unstable
-    # 97.48% (12352 / 12673) -> 322  86.37% (2776 / 3214) -> 185 + 225 = 410
-    minimum_coverage line: 97.44, branch: 87.12
+    # SD 27/01/26 branch coverage still appears to be ~ .1% unstable
+    # 97.47% (12400 / 12722) -> 322  87.11% (2785 / 3197) -> 197 + 215 = 412
+    minimum_coverage line: 97.45, branch: 87.1
   end
 end

app/helpers/maps_helper.rb

@@ -1,26 +1,30 @@
 module MapsHelper
   def vacancy_organisations_map_markers(vacancy)
-    vacancy.organisations.map do |organisation|
-      {
-        id: vacancy.id,
-        parent_id: organisation.id,
-        geopoint: RGeo::GeoJSON.encode(organisation.geopoint)&.to_json,
-      }
+    vacancy.organisations.filter_map do |organisation|
+      if organisation.uk_geopoint?
+        {
+          id: vacancy.id,
+          parent_id: organisation.id,
+          geopoint: RGeo::GeoJSON.encode(GeoFactories.convert_sr27700_to_wgs84(organisation.uk_geopoint)).to_json,
+        }
+      end
     end
   end
 
   def organisation_map_marker(organisation)
     [
       {
         parent_id: organisation.id,
-        geopoint: RGeo::GeoJSON.encode(organisation.geopoint)&.to_json,
+        geopoint: RGeo::GeoJSON.encode(GeoFactories.convert_sr27700_to_wgs84(organisation.uk_geopoint)).to_json,
       },
     ]
   end
 
   def organisation_map_can_be_displayed?(vacancy)
     return true unless vacancy.central_office?
 
-    vacancy.organisation.geopoint.present?
+    # :nocov:
+    vacancy.organisation.uk_geopoint.present?
+    # :nocov:
   end
 end

app/models/job_preferences/location.rb

@@ -6,28 +6,27 @@ class Location < ApplicationRecord
 
     self.table_name = "job_preferences_locations"
     belongs_to :job_preferences
+    self.ignored_columns += %i[area]
 
     validates :name, presence: true
     validates :radius, presence: true
     validates :uk_area, presence: true
 
     before_validation :set_area
 
-    scope :containing, ->(point) { where("ST_Within(ST_GeomFromEWKT(?), area::geometry)", "SRID=4326;#{point.as_text}") }
+    scope :containing, ->(point) { where(arel_table[:uk_area].st_contains(point)) }
 
     private
 
     def set_area
       if LocationPolygon.contain?(name)
-        polygon = LocationPolygon.buffered(radius).with_name(name)
-        self.area = polygon.area
-        self.uk_area = polygon.uk_area
+        # :nocov:
+        self.uk_area = LocationPolygon.buffered(radius).with_name(name).uk_area
+        # :nocov:
       else
         lat, long = Geocoding.new(name).coordinates.map(&:to_s)
         radius_meters = convert_miles_to_metres(Search::RadiusBuilder.new(name, radius).radius)
-        geopoint = GeoFactories::FACTORY_4326.point(long, lat)
-        self.area = geopoint.buffer(radius_meters)
-        self.uk_area = GeoFactories.convert_wgs84_to_sr27700(geopoint).buffer(radius_meters)
+        self.uk_area = GeoFactories.convert_wgs84_to_sr27700(GeoFactories::FACTORY_4326.point(long, lat).buffer(radius_meters))
       end
     end
   end

app/models/location_polygon.rb

@@ -1,19 +1,13 @@
 class LocationPolygon < ApplicationRecord
   extend DistanceHelper
 
-  # British National Grid SRID (EPSG:27700) is a projected coordinate system used for mapping in Great Britain.
-  # It provides coordinates in meters, which is useful for distance calculations, which we need
-  # for radius-based searches.
-  # It is significantly more accurate for distance calculations in Great Britain that EPSG:3857 (Web Mercator).
-  # EPSG:3857 distort distances and areas, especially as you move away from the equator. What would cause a multiplier
-  # between 1.5x and 1.7x for radius/buffer distances in our case to get the matches we would expect.
-  BRITISH_NATIONAL_GRID_SRID = 27700 # rubocop:disable Style/NumericLiterals
-
   validates :name, presence: true
 
   # Scope that expands any polygons returned by subsequent scopes by the provided radius
   # by overriding the `area` attribute with a buffered version of itself
-  scope :buffered, ->(radius_in_miles) { select("*, ST_Buffer(area, #{convert_miles_to_metres(radius_in_miles || 0)}) AS area") }
+  scope :buffered, ->(radius_in_miles) { select("*, ST_Buffer(uk_area, #{convert_miles_to_metres(radius_in_miles || 0)}) AS uk_area") }
+
+  self.ignored_columns += %i[area centroid]
 
   def self.with_name(location)
     find_by(name: mapped_name(location))
@@ -29,48 +23,21 @@ def self.mapped_name(location)
 
   def self.find_valid_for_location(location)
     polygon = with_name(location)
-    if polygon.present? && polygon.area.invalid_reason.nil?
+    if polygon.present? && polygon.uk_area.invalid_reason.nil?
       polygon
     end
   rescue RGeo::Error::InvalidGeometry
     nil
   end
 
   # Buffers the polygon's area by the given radius in metres and returns the resulting expanded area in geometry.
-  # Transformations are needed since the original area is a geographic type.
-  #
   # The "where & pick" approach is more efficient to retrieve a single computed value from the DB than loading the whole object.
-  #
-  # Why buffering in British National Grid SRID (27700) to then store it as SRID 4326?
-  # Buffering is best done in a projected coordinate system as it buffers in metres instead of degrees).
-  # After buffering, we transform to SRID: 4326 (lat/lon data). As we're doing spatial queries on this data, 4326 is more appropriate.
   def buffered_geometry_area(radius_in_metres)
-    wkb = self.class.where(id: id).pick(
-      Arel.sql(
-        "ST_AsBinary(
-            ST_Transform(
-              ST_Buffer(
-                ST_Transform(area::geometry, #{BRITISH_NATIONAL_GRID_SRID}),
-                #{radius_in_metres}
-              ),
-            4326)
-          )",
-      ),
-    )
-    return nil unless wkb
-
-    # Has to use this or the returned geometry gets a SRID: 0 even when set in the DB as 4326
-    RGeo::Cartesian.factory(srid: 4326).parse_wkb(wkb)
-  rescue RGeo::Error::InvalidGeometry
-    nil
-  end
-
-  # Buffers the polygon's area by the given radius in metres and returns the resulting expanded area in geometry.
-  # The "where & pick" approach is more efficient to retrieve a single computed value from the DB than loading the whole object.
-  def buffered_geometry_uk_area(radius_in_metres)
     wkb = self.class.where(id: id)
-              .pick(Arel.sql("ST_AsBinary(ST_Buffer(uk_area::geometry,#{radius_in_metres}))"))
+        .pick(Arel.sql("ST_AsBinary(ST_Buffer(uk_area::geometry,#{radius_in_metres}))"))
     # Has to use this or the returned geometry gets a SRID: 0 even when set in the DB as 27700
     GeoFactories::FACTORY_27700.parse_wkb(wkb) if wkb
+  rescue RGeo::Error::InvalidGeometry
+    nil
   end
 end

app/models/organisation.rb

@@ -24,6 +24,8 @@ class Organisation < ApplicationRecord
   has_many :jobseeker_profile_exclusions, class_name: "JobseekerProfileExcludedOrganisation"
   has_many :hidden_jobseeker_profiles, through: :jobseeker_profile_exclusions, source: :jobseeker_profile
 
+  self.ignored_columns += %i[geopoint]
+
   scope :not_closed, -> { where.not(establishment_status: "Closed") }
   scope :schools, -> { where(type: "School") }
   scope :school_groups, -> { where(type: "SchoolGroup") }

app/models/subscription.rb

@@ -20,6 +20,8 @@ class Subscription < ApplicationRecord
   # support_job_roles used to be called teaching_support_job_roles and non_teaching_support_job_roles in the past, and there are still active subscriptions with this name
   JOB_ROLE_ALIASES = %i[teaching_job_roles support_job_roles teaching_support_job_roles non_teaching_support_job_roles].freeze
 
+  self.ignored_columns += %i[area geopoint]
+
   def self.encryptor(serializer: :json_allow_marshal)
     key_generator_secret = SUBSCRIPTION_KEY_GENERATOR_SECRET
     key_generator_salt = SUBSCRIPTION_KEY_GENERATOR_SALT
@@ -96,25 +98,16 @@ def set_location_data!
 
   # A subscription with location area has a polygon area seat buffered by radius, no geopoint.
   def set_location_from_polygon(polygon, radius)
-    # Cast polygon area from geography to geometry and buffer by radius before storing
-    self.area = polygon.buffered_geometry_area(self.class.convert_miles_to_metres(radius))
-    self.geopoint = nil
-
-    self.uk_area = polygon.buffered_geometry_uk_area(self.class.convert_miles_to_metres(radius))
+    self.uk_area = polygon.buffered_geometry_area(self.class.convert_miles_to_metres(radius))
     self.uk_geopoint = nil
-
     self.radius_in_metres = self.class.convert_miles_to_metres(radius)
   end
 
   # A subscription with location coordinates has a geopoint, no area.
   def set_location_from_coordinates(coordinates, radius)
-    self.geopoint = RGeo::Cartesian.factory(srid: 4326).point(coordinates.second, coordinates.first)
-    self.area = nil
-
     geopoint = GeoFactories::FACTORY_4326.point(coordinates.second, coordinates.first)
     self.uk_geopoint = GeoFactories.convert_wgs84_to_sr27700 geopoint
     self.uk_area = nil
-
     self.radius_in_metres = self.class.convert_miles_to_metres(radius)
   end
 end

app/models/vacancy.rb

@@ -157,6 +157,8 @@ class Vacancy < ApplicationRecord
 
   after_save :update_conversation_searchable_content, if: -> { saved_change_to_job_title? }
 
+  self.ignored_columns += %i[geolocation]
+
   EQUAL_OPPORTUNITIES_PUBLICATION_THRESHOLD = 5
   EXPIRY_TIME_OPTIONS = %w[8:00 9:00 12:00 15:00 23:59].freeze
 
@@ -268,11 +270,11 @@ def salary_types
   end
 
   def distance_in_miles_to(search_coordinates)
-    if geolocation.is_a? RGeo::Geographic::SphericalMultiPointImpl
+    if uk_geolocation.is_a? RGeo::Cartesian::MultiPointImpl
       # if there are multiple geolocations then return the distance to the nearest one to the given search location
-      geolocation.map { |geolocation| calculate_distance(search_coordinates, geolocation) }.min
+      uk_geolocation.map { |geolocation| self.class.calculate_distance(search_coordinates, geolocation) }.min
     else
-      calculate_distance(search_coordinates, geolocation)
+      self.class.calculate_distance(search_coordinates, uk_geolocation)
     end
   end
 
@@ -303,8 +305,12 @@ def update_conversation_searchable_content
                 .find_each(&:update_searchable_content)
   end
 
-  def calculate_distance(search_coordinates, geolocation)
-    Geocoder::Calculations.distance_between(search_coordinates, [geolocation.latitude, geolocation.longitude])
+  class << self
+    def calculate_distance(search_coordinates, geolocation)
+      search_location = GeoFactories::FACTORY_4326.point(search_coordinates.second, search_coordinates.first)
+      search_point = GeoFactories.convert_wgs84_to_sr27700 search_location
+      search_point.distance(geolocation) / DistanceHelper::METRES_PER_MILE
+    end
   end
 
   def slug_candidates
@@ -317,18 +323,12 @@ def slug_candidates
   # In the former case, it gets an argument, which we don't need and thus ignore
   #
   def refresh_geolocation(_school_added_or_removed = nil)
-    self.geolocation = if organisations.one?
-                         organisation.geopoint
-                       else
-                         points = organisations.filter_map(&:geopoint)
-                         points.presence && points.first.factory.multi_point(points)
-                       end
-    # self.uk_geolocation = if organisations.one?
-    #                         organisation.uk_geopoint
-    #                       else
-    #                         uk_points = organisations.filter_map(&:uk_geopoint)
-    #                         uk_points.presence && uk_points.first.factory.multi_point(uk_points)
-    #                       end
+    self.uk_geolocation = if organisations.one?
+                            organisation.uk_geopoint
+                          else
+                            uk_points = organisations.filter_map(&:uk_geopoint)
+                            uk_points.presence && uk_points.first.factory.multi_point(uk_points)
+                          end
   end
 end
 # rubocop:enable Metrics/ClassLength

app/queries/location_query.rb

@@ -32,7 +32,7 @@ def nationwide_location?(query)
   def handle_polygon_location(field_name, polygon, radius, sort_by_distance)
     @scope = scope.joins("
       INNER JOIN location_polygons
-      ON ST_DWithin(#{field_name}, location_polygons.area, #{radius}, false)
+      ON ST_DWithin(#{field_name}, location_polygons.uk_area, #{radius})
     ").where("location_polygons.id = ?", polygon.id)
 
     sort_by_polygon_distance(field_name) if sort_by_distance
@@ -47,23 +47,24 @@ def handle_coordinates(field_name, query, radius, sort_by_distance)
     # suitable error instead. Refactor later!
     return scope.none if coordinates == [0, 0]
 
-    point = "POINT(#{coordinates.second} #{coordinates.first})"
-    @scope = scope.where("ST_DWithin(#{field_name}, ?, ?, false)", point, radius)
+    earth_point = GeoFactories::FACTORY_4326.point(coordinates.second, coordinates.first)
+    point = GeoFactories.convert_wgs84_to_sr27700 earth_point
+    @scope = scope.where("ST_DWithin(#{field_name}, ?, ?)", "SRID=#{point.srid};#{point}", radius)
 
     sort_by_coordinates_distance(field_name, point) if sort_by_distance
 
     scope
   end
 
   def sort_by_polygon_distance(field_name)
-    @scope = scope.select("vacancies.*, ST_Distance(#{field_name}, location_polygons.centroid, false) AS distance")
+    @scope = scope.select("vacancies.*, ST_Distance(#{field_name}, location_polygons.uk_centroid) AS distance")
                   # why not using 'distance' alias? is not defined when calling this query with a 'pluck'
-                  .order(Arel.sql("ST_Distance(#{field_name}, location_polygons.centroid, false)"))
+                  .order(Arel.sql("ST_Distance(#{field_name}, location_polygons.uk_centroid)"))
   end
 
   def sort_by_coordinates_distance(field_name, point)
-    @scope = scope.select("vacancies.*, ST_Distance(#{field_name}, '#{point}', false) AS distance")
+    @scope = scope.select("vacancies.*, ST_Distance(#{field_name}, 'SRID=#{point.srid};#{point}') AS distance")
                   # why not using 'distance' alias? is not defined when calling this query with a 'pluck'
-                  .order(Arel.sql("ST_Distance(#{field_name}, '#{point}', false)"))
+                  .order(Arel.sql("ST_Distance(#{field_name}, 'SRID=#{point.srid};#{point}')"))
   end
 end

app/queries/organisation_location_query.rb

@@ -4,6 +4,6 @@ def initialize(scope = Organisation.all)
   end
 
   def call(...)
-    super("organisations.geopoint", ...)
+    super("organisations.uk_geopoint", ...)
   end
 end

app/queries/subscription_vacancies_matching_query.rb

@@ -1,14 +1,6 @@
 class SubscriptionVacanciesMatchingQuery
   attr_reader :scope
 
-  # British National Grid SRID (EPSG:27700) is a projected coordinate system used for mapping in Great Britain.
-  # It provides coordinates in meters, which is useful for distance calculations, which we need
-  # for radius-based searches.
-  # It is significantly more accurate for distance calculations in Great Britain that EPSG:3857 (Web Mercator).
-  # EPSG:3857 distort distances and areas, especially as you move away from the equator. What would cause a multiplier
-  # between 1.5x and 1.7x for radius/buffer distances in our case to get the matches we would expect.
-  BRITISH_NATIONAL_GRID_SRID = 27700 # rubocop:disable Style/NumericLiterals
-
   # Builds the query to find the IDs for the vacancies matching the subscription criteria.
   # The subquery is needed to be able to combine our requirements into a valid single SQL statement:
   # - retrieve unique DISTINCT ON vacancy ids: As the location filter may return the same vacancy multiple times (vacancy for multiple orgs).
@@ -177,9 +169,9 @@ def location_filter(scope, subscription_location, subscription)
     # 'area_before_type_cast' and 'geopoint_before_type_cast' are used to avoid casting the fields into RGeo objects.
     # This reduces memory usage and speeds up the query. As we don't need to use the actual objects in Ruby code,
     # just need to know if they are present in DB.
-    if subscription.area_before_type_cast.present?
+    if subscription.uk_area_before_type_cast.present?
       location_by_area_filter(scope, subscription)
-    elsif subscription.geopoint_before_type_cast.present? && subscription.radius_in_metres.present?
+    elsif subscription.uk_geopoint_before_type_cast.present? && subscription.radius_in_metres.present?
       location_by_geopoint_filter(scope, subscription)
     else
       scope.none # Invalid location filter (having no area or geopoint) returns no matches
@@ -197,9 +189,11 @@ def location_filter(scope, subscription_location, subscription)
   # Including 'publish_on' in the distinct: The provided scope may be ordered by publish on date. When using distinct
   # with ordering, Postgres requires all selected columns to be included in the distinct clause or will fail during execution.
   def location_by_area_filter(scope, subscription)
+    subscriptions = Subscription.arel_table
+    organisations = Organisation.arel_table
     scope.joins("INNER JOIN subscriptions ON subscriptions.id = '#{subscription.id}'")
          .joins(:organisations)
-         .where("ST_Contains(subscriptions.area, organisations.geopoint::geometry)")
+         .where(subscriptions[:uk_area].st_contains(organisations[:uk_geopoint]))
   end
 
   # Filter vacancies where their organisations' geopoints fall within the subscription's radius from the subscription's
@@ -214,8 +208,8 @@ def location_by_area_filter(scope, subscription)
   def location_by_geopoint_filter(scope, subscription)
     scope.joins("INNER JOIN subscriptions ON subscriptions.id = '#{subscription.id}'")
          .joins(:organisations)
-         .where("ST_DWithin(ST_Transform(organisations.geopoint::geometry, #{BRITISH_NATIONAL_GRID_SRID}),
-                            ST_Transform(subscriptions.geopoint, #{BRITISH_NATIONAL_GRID_SRID}),
+         .where("ST_DWithin(organisations.uk_geopoint::geometry,
+                            subscriptions.uk_geopoint,
                             subscriptions.radius_in_metres)")
   end
 end