Merge pull request #413 from gtt-project/feat/watch-radius-unit

feat: nearby-watch radius in the configured display unit

Author: dkastl

app/views/redmine_gtt/hooks/_watch_nearby_preference.html.erb

@@ -19,11 +19,17 @@
   </p>
   <p>
     <label for="pref_gtt_watch_radius"><%= l(:gtt_label_watch_nearby_radius) %></label>
-    <%= number_field_tag 'pref[gtt_watch_radius]', user.pref.gtt_watch_radius,
-          id: 'pref_gtt_watch_radius', min: 1,
-          max: RedmineGtt::Patches::UserPreferencePatch::NEARBY_WATCH_MAX_RADIUS_KM,
-          step: 1, disabled: !location_set %>
-    <%= l(:gtt_label_watch_nearby_radius_unit) %>
+    <%# entered in the configured display unit, stored in meters (#10) %>
+    <%# min = 1 meter expressed in the unit: the server treats smaller
+        values as unset, so don't offer them %>
+    <%= number_field_tag 'pref[gtt_watch_radius_in_unit]', user.pref.gtt_watch_radius_in_unit,
+          id: 'pref_gtt_watch_radius', step: 'any',
+          min: RedmineGtt::DistanceUnit.from_meters(1).round(6),
+          max: RedmineGtt::DistanceUnit.from_meters(
+            RedmineGtt::Patches::UserPreferencePatch::NEARBY_WATCH_MAX_RADIUS_M
+          ).ceil,
+          disabled: !location_set %>
+    <%= RedmineGtt::DistanceUnit.current %>
   </p>
   <% unless location_set %>
     <p><em class="info"><%= l(:gtt_text_watch_nearby_requires_location) %></em></p>

config/locales/de.yml

@@ -142,5 +142,4 @@ de:
   gtt_label_watch_nearby_legend: "Automatisch beobachten: Tickets in der Nähe"
   gtt_label_watch_nearby: "Neue Tickets in der Nähe meines Standorts beobachten"
   gtt_label_watch_nearby_radius: "Entfernung"
-  gtt_label_watch_nearby_radius_unit: "km"
   gtt_text_watch_nearby_requires_location: "Legen Sie zuerst Ihren Standort auf der Karte oben fest, um dies zu aktivieren."

config/locales/en.yml

@@ -77,7 +77,6 @@ en:
   gtt_label_watch_nearby_legend: "Auto watch: nearby issues"
   gtt_label_watch_nearby: "Watch new issues near my location"
   gtt_label_watch_nearby_radius: "Distance"
-  gtt_label_watch_nearby_radius_unit: "km"
   gtt_text_watch_nearby_requires_location: "Set your location on the map above to enable this."
 
   select_default_tracker_icon: "Select default tracker icon:"

config/locales/ja.yml

@@ -74,7 +74,6 @@ ja:
   gtt_label_watch_nearby_legend: "自動ウォッチ: 近くのチケット"
   gtt_label_watch_nearby: "登録した所在地の近くの新しいチケットをウォッチする"
   gtt_label_watch_nearby_radius: "距離"
-  gtt_label_watch_nearby_radius_unit: "km"
   gtt_text_watch_nearby_requires_location: "有効にするには上の地図で所在地を設定してください。"
 
   select_default_tracker_icon: "トラッカーアイコンを選択:"

lib/redmine_gtt/nearby_watchers.rb

@@ -19,9 +19,9 @@ def self.subscribe(issue)
 
     def self.nearby_users(issue)
       candidates(issue).select do |user|
-        radius_km = user.pref.gtt_watch_nearby? && user.pref.gtt_watch_radius_km
-        radius_km &&
-          user['gtt_distance_m'].to_f <= radius_km * 1000 &&
+        radius_m = user.pref.gtt_watch_nearby? && user.pref.gtt_watch_radius_m
+        radius_m &&
+          user['gtt_distance_m'].to_f <= radius_m &&
           issue.visible?(user)
       end
     end
@@ -30,7 +30,7 @@ def self.nearby_users(issue)
     # The issue geometry is bound as an EWKB hex parameter; the distance is
     # measured on the geography type (meters, any geometry type).
     def self.candidates(issue)
-      max_meters = Patches::UserPreferencePatch::NEARBY_WATCH_MAX_RADIUS_KM * 1000
+      max_meters = Patches::UserPreferencePatch::NEARBY_WATCH_MAX_RADIUS_M
       distance = User.send(:sanitize_sql_array, [
         "ST_Distance(#{User.table_name}.geom::geography, ?::geometry::geography)",
         ewkb_hex(issue.geom)

lib/redmine_gtt/patches/user_preference_patch.rb

@@ -4,21 +4,23 @@ module Patches
     # Stores the "auto watch nearby issues" opt-in (#14) in the serialized
     # UserPreference#others hash, so no schema change is needed. The user's
     # stored location (users.geom) is the center of the watch area; the
-    # radius is kept in kilometers as entered on the My account page.
+    # radius is stored in meters (the plugin's internal unit, #10) and
+    # entered/displayed in the configured display unit on My account.
     #
     # This patch only adds the preference storage and validated readers.
     # The watcher assignment itself hooks into issue creation separately.
     module UserPreferencePatch
 
-      # Upper bound for the watch radius, enforced server-side (the form's
-      # max attribute mirrors it, but client-side limits are easy to bypass)
-      # so an arbitrarily large radius can't make the watcher query expensive.
-      NEARBY_WATCH_MAX_RADIUS_KM = 1000
+      # Upper bound for the watch radius (1000 km), enforced server-side
+      # (the form's max attribute mirrors it, but client-side limits are
+      # easy to bypass) so an arbitrarily large radius can't make the
+      # watcher query expensive.
+      NEARBY_WATCH_MAX_RADIUS_M = 1_000_000
 
       def self.apply
         unless UserPreference < self
           UserPreference.prepend self
-          UserPreference.safe_attributes 'gtt_watch_nearby', 'gtt_watch_radius'
+          UserPreference.safe_attributes 'gtt_watch_nearby', 'gtt_watch_radius_in_unit'
         end
       end
 
@@ -28,6 +30,8 @@ def self.apply
       def gtt_watch_nearby; self[:gtt_watch_nearby]; end
       def gtt_watch_nearby=(value); self[:gtt_watch_nearby] = value; end
 
+      # Raw stored radius (meters). Not mass-assignable; the account form
+      # goes through gtt_watch_radius_in_unit.
       def gtt_watch_radius; self[:gtt_watch_radius]; end
       def gtt_watch_radius=(value); self[:gtt_watch_radius] = value; end
 
@@ -36,12 +40,37 @@ def gtt_watch_nearby?
         gtt_watch_nearby.to_s == '1'
       end
 
-      # Validated radius in kilometers: a positive Integer capped at
-      # NEARBY_WATCH_MAX_RADIUS_KM, or nil when the preference is unset or
+      # Validated radius in meters: a positive number capped at
+      # NEARBY_WATCH_MAX_RADIUS_M, or nil when the preference is unset or
       # holds a non-numeric/non-positive value.
-      def gtt_watch_radius_km
-        value = Integer(gtt_watch_radius.to_s, exception: false)
-        value.clamp(1, NEARBY_WATCH_MAX_RADIUS_KM) if value&.positive?
+      def gtt_watch_radius_m
+        value = Float(gtt_watch_radius.to_s, exception: false)
+        [value, NEARBY_WATCH_MAX_RADIUS_M].min if value&.positive?
+      end
+
+      # Form-facing virtual attribute: the radius in the configured display
+      # unit. Whole numbers render without a decimal part; fractions keep
+      # 6 decimals (enough for 1 m in any supported unit), so a read->save
+      # round trip cannot drift the stored meters.
+      def gtt_watch_radius_in_unit
+        meters = gtt_watch_radius_m
+        return nil unless meters
+
+        value = DistanceUnit.from_meters(meters)
+        value == value.to_i ? value.to_i : value.round(6)
+      end
+
+      def gtt_watch_radius_in_unit=(value)
+        stripped = value.to_s.strip
+        self.gtt_watch_radius =
+          if stripped.empty?
+            nil
+          elsif (number = Float(stripped, exception: false))
+            DistanceUnit.to_meters(number).round.to_s
+          else
+            # keep the garbage; gtt_watch_radius_m rejects it as before
+            stripped
+          end
       end
 
     end

test/functional/my_account_watch_nearby_test.rb

@@ -30,14 +30,14 @@ class MyAccountWatchNearbyTest < Redmine::ControllerTest
   end
 
   test 'saving other settings without a location leaves the preference untouched' do
-    @user.pref.update(gtt_watch_nearby: '1', gtt_watch_radius: '25')
+    @user.pref.update(gtt_watch_nearby: '1', gtt_watch_radius: '25000')
 
     put :account, params: { user: { firstname: 'Dave' }, pref: { no_self_notified: '1' } }
 
     assert_redirected_to '/my/account'
     pref = User.find(@user.id).pref
     assert pref.gtt_watch_nearby?
-    assert_equal 25, pref.gtt_watch_radius_km
+    assert_equal 25_000, pref.gtt_watch_radius_m
   end
 
   test 'renders the fieldset enabled once a location is stored' do
@@ -56,18 +56,18 @@ class MyAccountWatchNearbyTest < Redmine::ControllerTest
 
     put :account, params: {
       user: { firstname: @user.firstname },
-      pref: { gtt_watch_nearby: '1', gtt_watch_radius: '25' }
+      pref: { gtt_watch_nearby: '1', gtt_watch_radius_in_unit: '25' }
     }
 
     assert_redirected_to '/my/account'
     pref = User.find(@user.id).pref
     assert pref.gtt_watch_nearby?
-    assert_equal 25, pref.gtt_watch_radius_km
+    assert_equal 25, pref.gtt_watch_radius_m
   end
 
   test 'unchecking the box turns the preference off' do
     @user.update_attribute :geojson, example_geojson
-    @user.pref.update(gtt_watch_nearby: '1', gtt_watch_radius: '25')
+    @user.pref.update(gtt_watch_nearby: '1', gtt_watch_radius: '25000')
 
     put :account, params: {
       user: { firstname: @user.firstname },
@@ -78,6 +78,6 @@ class MyAccountWatchNearbyTest < Redmine::ControllerTest
     pref = User.find(@user.id).pref
     assert_not pref.gtt_watch_nearby?
     # the radius survives so re-enabling does not lose the value
-    assert_equal 25, pref.gtt_watch_radius_km
+    assert_equal 25_000, pref.gtt_watch_radius_m
   end
 end

test/unit/nearby_watchers_test.rb

@@ -12,7 +12,7 @@ class NearbyWatchersTest < GttTest
   setup do
     @user = User.find_by_login 'dlopper' # member of the public project 1
     @user.update_attribute :geojson, point_geojson(TOKYO)
-    @user.pref.update(gtt_watch_nearby: '1', gtt_watch_radius: '25')
+    @user.pref.update(gtt_watch_nearby: '1', gtt_watch_radius: '25000')
   end
 
   test 'subscribes an opted-in user within their radius on issue creation' do
@@ -31,7 +31,7 @@ class NearbyWatchersTest < GttTest
 
   test 'subscribes a distant user whose radius is large enough' do
     @user.update_attribute :geojson, point_geojson(OSAKA)
-    @user.pref.update(gtt_watch_radius: '500')
+    @user.pref.update(gtt_watch_radius: '500000')
 
     issue = create_issue_with_geom!(1, NEAR_TOKYO)
 

test/unit/user_preference_patch_test.rb

@@ -7,30 +7,41 @@ class UserPreferencePatchTest < GttTest
     @pref = User.find_by_login('dlopper').pref
   end
 
+  teardown do
+    Setting.plugin_redmine_gtt = Setting.plugin_redmine_gtt.merge(
+      'distance_unit' => 'm'
+    )
+  end
+
   test 'preference is off by default' do
     assert_not @pref.gtt_watch_nearby?
-    assert_nil @pref.gtt_watch_radius_km
+    assert_nil @pref.gtt_watch_radius_m
   end
 
   test 'accessors persist through the serialized others hash' do
     @pref.gtt_watch_nearby = '1'
-    @pref.gtt_watch_radius = '25'
+    @pref.gtt_watch_radius = '25000'
     assert @pref.save
 
     pref = User.find_by_login('dlopper').pref
     assert pref.gtt_watch_nearby?
-    assert_equal 25, pref.gtt_watch_radius_km
+    assert_equal 25_000, pref.gtt_watch_radius_m
   end
 
-  test 'safe_attributes mass-assignment works for the new keys' do
+  test 'safe_attributes mass-assignment works for the form keys' do
     @pref.safe_attributes = {
-      'gtt_watch_nearby' => '1', 'gtt_watch_radius' => '10'
+      'gtt_watch_nearby' => '1', 'gtt_watch_radius_in_unit' => '10'
     }
     assert @pref.save
 
     pref = User.find_by_login('dlopper').pref
     assert pref.gtt_watch_nearby?
-    assert_equal 10, pref.gtt_watch_radius_km
+    assert_equal 10, pref.gtt_watch_radius_m
+  end
+
+  test 'the raw radius is not mass-assignable' do
+    @pref.safe_attributes = { 'gtt_watch_radius' => '123' }
+    assert_nil @pref.gtt_watch_radius_m
   end
 
   test 'gtt_watch_nearby? treats anything but "1" as off' do
@@ -40,16 +51,47 @@ class UserPreferencePatchTest < GttTest
     end
   end
 
-  test 'gtt_watch_radius_km rejects blank, non-numeric and non-positive values' do
-    [nil, '', 'abc', '0', '-5', '2.5'].each do |value|
+  test 'gtt_watch_radius_m rejects blank, non-numeric and non-positive values' do
+    [nil, '', 'abc', '0', '-5'].each do |value|
       @pref.gtt_watch_radius = value
-      assert_nil @pref.gtt_watch_radius_km, "expected #{value.inspect} to be nil"
+      assert_nil @pref.gtt_watch_radius_m, "expected #{value.inspect} to be nil"
     end
   end
 
-  test 'gtt_watch_radius_km caps the radius server-side' do
-    @pref.gtt_watch_radius = '999999'
-    assert_equal RedmineGtt::Patches::UserPreferencePatch::NEARBY_WATCH_MAX_RADIUS_KM,
-      @pref.gtt_watch_radius_km
+  test 'gtt_watch_radius_m caps the radius server-side' do
+    @pref.gtt_watch_radius = '99999999'
+    assert_equal RedmineGtt::Patches::UserPreferencePatch::NEARBY_WATCH_MAX_RADIUS_M,
+      @pref.gtt_watch_radius_m
+  end
+
+  test 'the form attribute converts through the configured display unit' do
+    Setting.plugin_redmine_gtt = Setting.plugin_redmine_gtt.merge(
+      'distance_unit' => 'km'
+    )
+    @pref.gtt_watch_radius_in_unit = '25'
+    assert_equal 25_000, @pref.gtt_watch_radius_m
+    assert_equal 25, @pref.gtt_watch_radius_in_unit
+
+    @pref.gtt_watch_radius_in_unit = '0.5'
+    assert_equal 500, @pref.gtt_watch_radius_m
+    assert_equal 0.5, @pref.gtt_watch_radius_in_unit
+  end
+
+  test 'a read-save round trip does not drift the stored meters' do
+    Setting.plugin_redmine_gtt = Setting.plugin_redmine_gtt.merge(
+      'distance_unit' => 'mi'
+    )
+    @pref.gtt_watch_radius = '500' # meters, not a round mile value
+    displayed = @pref.gtt_watch_radius_in_unit
+    @pref.gtt_watch_radius_in_unit = displayed.to_s
+    assert_equal 500, @pref.gtt_watch_radius_m
+  end
+
+  test 'the form attribute keeps garbage rejected and blank clearing' do
+    @pref.gtt_watch_radius_in_unit = 'abc'
+    assert_nil @pref.gtt_watch_radius_m
+
+    @pref.gtt_watch_radius_in_unit = ' '
+    assert_nil @pref.gtt_watch_radius
   end
 end