issue filtering by bounding box and distance #51

jkraemer · jkraemer · commit c19c1b733cf8 · 2017-12-06T14:09:44.000+08:00
- to filter by bbounding box, use the bbox URL param with a value like
  '123.193645|9.256139|123.331833|9.364216' (lng1,lat1,lng2,lat2)
- to filter by distance, use the distance URL param with a value like
  '&lt;=1000|123.2696|9.3050' where 1000 is the distance in meters and the
  following two values are lng,lat.
- when filtering by distance, you can also use sort=distance or
  sort=distance:desc to use it for ordering

- at this time this works only through the API, for the Web UI some Javascript
  is needed to transfer the current map center / extend into the filter form
  fields.
diff --git a/app/views/issues/index.api.rsb b/app/views/issues/index.api.rsb
@@ -26,6 +26,10 @@ api.array :issues, api_meta(:total_count => @issue_count, :offset => @offset, :l
         api.geojson ""
       end
 
+      if issue.distance
+        api.distance issue.distance
+      end
+
       render_api_custom_values issue.visible_custom_field_values, api
 
       api.created_on issue.created_on
diff --git a/config/locales/en.yml b/config/locales/en.yml
@@ -26,7 +26,8 @@ en:
   gtt_text_settings_geometry_example: "2747491302261fbc47967ba62621af22"
 
   label_gtt: "GTT"
-  label_gtt_bounding_box_filter: Location
+  label_gtt_bbox_filter: Location
+  label_gtt_distance: Distance
   label_gtt_tile_source: Tile Source
   label_gtt_tile_source_new: New Tile Source
   label_gtt_tile_source_plural: Tile Sources
diff --git a/lib/redmine_gtt/patches/issue_patch.rb b/lib/redmine_gtt/patches/issue_patch.rb
@@ -8,6 +8,7 @@ def self.apply
           Issue.prepend self
           Issue.prepend GeojsonAttribute
           Issue.class_eval do
+            attr_reader :distance
             safe_attributes "geojson",
               if: ->(issue, user){
                 perm = issue.new_record? ? :add_issues : :edit_issues
diff --git a/lib/redmine_gtt/patches/issue_query_patch.rb b/lib/redmine_gtt/patches/issue_query_patch.rb
@@ -5,36 +5,147 @@ module IssueQueryPatch
       def self.apply
         unless IssueQuery < self
           IssueQuery.prepend self
-          IssueQuery.add_available_column QueryColumn.new(:geom,
-                                                          caption: :field_geom)
         end
       end
 
       def self.prepended(base)
       end
 
+      def issues(*_)
+        super.tap do |issues|
+          if center = find_center_point
+            load_distances(issues, center)
+          end
+        end
+      rescue ::ActiveRecord::StatementInvalid => e
+        raise ::Query::StatementInvalid.new(e.message)
+      end
+
+      def available_columns
+        return @available_columns if @available_columns
+
+        super.tap do |columns|
+
+          if project.nil? or project.module_enabled?('gtt')
+            columns << QueryColumn.new(:geom,
+              caption: :field_geom
+            )
+            columns << QueryColumn.new(:distance,
+              caption: :label_gtt_distance,
+              sortable: lambda{
+                lng, lat = find_center_point
+                distance_query lng, lat
+              }
+            )
+          end
+
+        end
+      end
+
+
       # weiter: use 'On map' option tag to hold current map extent
       # - wenn filter hinzugefügt befüllen (dom hook oder so?)
       # - on map move / zoom updaten (app.map.js)
       def initialize_available_filters()
         super
         if project and project.module_enabled?('gtt')
           add_available_filter(
-            'location_filter',
-            name: l(:label_gtt_location_filter),
+            'bbox',
+            name: l(:label_gtt_bbox_filter),
             type: :list,
             values: ['On map']
           )
+          add_available_filter(
+            'distance',
+            name: l(:label_gtt_distance),
+            type: :float,
+          )
         end
       end
 
-      def available_columns
-        super.tap do |columns|
-          if project and !project.module_enabled?('gtt')
-            columns.reject!{|c| c.name == :geom}
+
+      def sql_for_distance_field(field, operator, value)
+        case operator
+        when '*'
+          "#{Issue.table_name}.geom IS NOT NULL"
+        when '!*'
+          "#{Issue.table_name}.geom IS NULL"
+        else
+          # value has to be ['meters_min', 'lng', 'lat']
+          # or ['meters_min', 'meters_max', 'lng', 'lat'] if op == '><'
+          lng, lat = value.last(2).map(&:to_f)
+          distance = value.first.to_i
+          sql = "ST_Distance_Sphere(#{Issue.table_name}.geom, ST_GeomFromText('POINT(#{lng} #{lat})',4326))"
+          if operator == '><'
+            distance_max = value[1].to_i
+            sql << " BETWEEN #{distance} AND #{distance_max}"
+          else
+            sql << " #{operator} #{distance}"
           end
         end
       end
+
+
+      def sql_for_bbox_field(field, operator, value)
+        not_in = "not " if operator == '!'
+
+        # value should be ['lng1|lat1|lng2|lat2'] or 'lng1|lat1|lng2|lat2' or
+        # ['lng1','lat1','lng2','lat2']
+        if value.is_a?(Array) && value.size == 1
+          value = value.first
+        end
+
+        if value.is_a?(String)
+          value = value.split('|')
+        end
+
+        # sanitize the coordinate values:
+        lng1,lat1,lng2,lat2 = value.map(&:to_f)
+
+        # TODO
+        # First I tried this, but it continued to complain about mixing
+        # different SRIDs:
+        # coordinates = [
+        #  [lng1,lat1], [lng2,lat1], [lng2,lat2], [lng1,lat2]
+        # ].map{|a| a.join ' '}.join(',')
+        # box = "ST_Polygon(ST_GeomFromText('LINESTRING(#{coordinates})'), 4326)"
+        # "#{not_in}ST_Contains(#{box}, ST_SetSRID(#{db_table}.geom, 4326))"
+
+        # So instead, I came up with this:
+        "#{not_in}ST_MakeEnvelope(#{lng1},#{lat1},#{lng2},#{lat2}, 4326) ~ #{Issue.table_name}.geom"
+        # I am not sure about the implications of using ~ instead of one of
+        # the ST_ functions. Appears that it is a  PostgreSQL geometric
+        # operator, leading to casting the PostGIS values to a PostgreSQL
+        # geometric type. Fact is, this statement does not seem to care
+        # about the SRID (can even leave it out of the MakeEnvelope call).
+        # Doesn't feel right but works for my simple test case and is
+        # probably good enough for this simple case (bbox is a rectangle and
+        # other geometry is a point). We should check if indizes are actually
+        # used though.
+      end
+
+      private
+
+      def find_center_point
+        if v = values_for('distance') and v.size > 2
+          v.last(2).map(&:to_f)
+        end
+      end
+
+      def distance_query(lng, lat)
+        "ST_Distance_Sphere(#{Issue.table_name}.geom, ST_GeomFromText('POINT(#{lng} #{lat})',4326))"
+      end
+
+      def load_distances(issues, center_point)
+        lng, lat = center_point
+        distances = Hash[
+          Issue.
+            where(id: issues.map(&:id)).
+            pluck(:id, "#{distance_query(lng, lat)}")
+        ]
+        issues.each{|i| i.instance_variable_set :@distance, distances[i.id]}
+      end
+
     end
   end
 end
diff --git a/test/integration/issue_filter_api_test.rb b/test/integration/issue_filter_api_test.rb
@@ -0,0 +1,207 @@
+require_relative '../test_helper'
+
+class IssueFilterApiTest < Redmine::ApiTest::Base
+  fixtures :projects,
+    :users,
+    :roles,
+    :members,
+    :member_roles,
+    :issues,
+    :issue_statuses,
+    :issue_relations,
+    :versions,
+    :trackers,
+    :projects_trackers,
+    :issue_categories,
+    :enabled_modules,
+    :enumerations,
+    :attachments,
+    :workflows,
+    :custom_fields,
+    :custom_values,
+    :custom_fields_projects,
+    :custom_fields_trackers,
+    :time_entries,
+    :journals,
+    :journal_details,
+    :queries,
+    :attachments
+
+  setup do
+    @project = Project.find 'ecookbook'
+    @project.enabled_modules.create name: 'gtt'
+  end
+
+  POINT_OUT = {
+    'type' => 'Feature',
+    'geometry' => {
+      'type' => 'Point',
+      'coordinates' => [123.324966,9.425016]
+    }
+  }
+
+  POINT_IN = {
+    'type' => 'Feature',
+    'geometry' => {
+      'type' => 'Point',
+      'coordinates' => [123.269691,9.305099]
+    }
+  }
+
+  # x1,y1,x2,y2 (Lng1,Lat1,...)
+  BBOX = '123.193645|9.256139|123.331833|9.364216'
+
+
+  test 'should filter by distance' do
+
+    get '/projects/ecookbook/issues.json', params: {
+      status_id: 'o',
+    }
+
+    assert_response :success
+    assert data = JSON.parse(response.body)
+    assert_equal 6, data['issues'].size
+
+    issue_in = @project.issues.find 1
+    issue_in.update_attribute :geojson, POINT_IN.to_json
+
+    issue_out = @project.issues.find 2
+    issue_out.update_attribute :geojson, POINT_OUT.to_json
+
+    # find everyting inside 1km radius
+    # using the 'short' parameter format
+
+    get '/projects/ecookbook/issues.json', params: {
+      status_id: 'o',
+      distance: '<=1000|123.2696|9.3050',
+    }
+    assert_response :success
+    assert data = JSON.parse(response.body)
+    assert_equal 1, data['issues'].size
+    assert_equal issue_in.id, data['issues'][0]['id']
+    assert dist = data['issues'][0]['distance'].to_i
+    assert dist > 0
+    assert dist < 1000
+
+
+    # filter and sort by distance
+    #
+    get '/projects/ecookbook/issues.json', params: {
+      status_id: 'o',
+      distance: '<=100000|123.2696|9.3050',
+      sort: 'distance'
+    }
+    assert_response :success
+    assert data = JSON.parse(response.body)
+    assert_equal 2, data['issues'].size
+    assert_equal issue_in.id, data['issues'][0]['id']
+    assert_equal issue_out.id, data['issues'][1]['id']
+
+    get '/projects/ecookbook/issues.json', params: {
+      status_id: 'o',
+      distance: '<=100000|123.2696|9.3050',
+      sort: 'distance:desc'
+    }
+    assert_response :success
+    assert data = JSON.parse(response.body)
+    assert_equal 2, data['issues'].size
+    assert_equal issue_out.id, data['issues'][0]['id']
+    assert_equal issue_in.id, data['issues'][1]['id']
+
+
+    # find everyting outside 1km radius
+    # this is using the url params as they come from the web ui just ot make
+    # sure this works as well
+
+    get '/projects/ecookbook/issues.json', params: {
+      set_filter: 1,
+      f: %w(status_id distance),
+      op: { status_id: 'o', distance: '>=' },
+      v:  { distance: ['1000', '123.2696', '9.3050'] }
+    }
+    assert_response :success
+    assert data = JSON.parse(response.body)
+    assert_equal 1, data['issues'].size
+    assert_equal issue_out.id, data['issues'][0]['id']
+
+
+    # find everyting on the 1km radius
+    # more of a theoretical use case...
+    get '/projects/ecookbook/issues.json', params: {
+      status_id: 'o',
+      distance: '=1000|123.2696|9.3050'
+    }
+    assert_response :success
+    assert data = JSON.parse(response.body)
+    assert_equal 0, data['issues'].size
+
+    # find everyting with a distance between 10 and 1000m
+    get '/projects/ecookbook/issues.json', params: {
+      status_id: 'o',
+      distance: '><10|1000|123.2696|9.3050'
+    }
+    assert_response :success
+    assert data = JSON.parse(response.body)
+    assert_equal 1, data['issues'].size
+    assert_equal issue_in.id, data['issues'][0]['id']
+
+    # find everyting having any distance (finds anything with a geometry set)
+    get '/projects/ecookbook/issues.json', params: {
+      status_id: 'o', distance: '*'
+    }
+    assert_response :success
+    assert data = JSON.parse(response.body)
+    assert_equal 2, data['issues'].size
+
+    # find everyting having no distance (finds anything with no geometry set)
+    get '/projects/ecookbook/issues.json', params: {
+      status_id: 'o', distance: '!*'
+    }
+    assert_response :success
+    assert data = JSON.parse(response.body)
+    assert_equal 4, data['issues'].size
+  end
+
+
+  test 'should filter by bounding box' do
+
+    get '/projects/ecookbook/issues.json', params: { status_id: 'o' }
+
+    assert_response :success
+    assert data = JSON.parse(response.body)
+    assert_equal 6, data['issues'].size
+
+    issue_in = @project.issues.find 1
+    issue_in.update_attribute :geojson, POINT_IN.to_json
+
+    issue_out = @project.issues.find 2
+    issue_out.update_attribute :geojson, POINT_OUT.to_json
+
+    # find everyting inside the given box
+    # using the shorter API parameter format
+
+    get '/projects/ecookbook/issues.json', params: {
+      status_id: 'o', bbox: "=#{BBOX}"
+    }
+    assert_response :success
+    assert data = JSON.parse(response.body)
+    assert_equal 1, data['issues'].size
+    assert_equal issue_in.id, data['issues'][0]['id']
+
+
+    # find everyting *outside* the given box
+    # using the parameter format used by the web UI
+
+    get '/projects/ecookbook/issues.json', params: {
+      set_filter: 1,
+      f: %w(status_id bbox),
+      op: { status_id: 'o', bbox: '!' },
+      v:  { bbox: [BBOX] }
+    }
+    assert_response :success
+    assert data = JSON.parse(response.body)
+    assert_equal 1, data['issues'].size
+    assert_equal issue_out.id, data['issues'][0]['id']
+  end
+
+end
