[openstreetmap/openstreetmap-website] CRUD interface for moderation zones (PR #7148)

[Tom Hughes]

@tomhughes commented on this pull request.



> @@ -0,0 +1,83 @@
+# frozen_string_literal: true
+
+class ModerationZonesController < ApplicationController
+  layout :site_layout
+
+  before_action :authorize_web
+  before_action :set_locale

This needs a `check_database_readable` as everything, including `index` needs to read the database.

> +
+module ModerationZonesHelper
+  def options_for_moderation_zone_period
+    ModerationZone::PERIODS.collect do |h|
+      [block_duration_in_words(h.hours), h.to_s]
+    end
+  end
+
+  def selected_option_for_moderation_zone_period(moderation_zone)
+    param_value = params.dig(:moderation_zone, :period)
+    if param_value
+      ModerationZone::PERIODS.min_by do |h|
+        (param_value.to_i - h).abs
+      end
+    elsif moderation_zone.ends_at
+      value_to_compare = ((moderation_zone.ends_at - Time.now.utc) / 1.hour).ceil.to_s

Why are we converting this to a string only to then convert it back to an integer when we use it?

I think I'd also be inclined to move the loop later so we only do it once after we've decided what value to search for given the two loops are basically the same.

> 

Should we rename this to `falls_within_active?` or `falls_within_any_active?` now that it is also filtering out inactive zones?

> +  def test_active?
+    modzone1 = create(:moderation_zone, :ends_at => 1.day.from_now)
+    assert_predicate modzone1, :active?
+
+    modzone2 = create(:moderation_zone, :ends_at => 1.day.ago)
+    assert_not_predicate modzone2, :active?
   end

I think this test is in the wrong commit? It should probably be one commit earlier when the `active?` method is added to the model?

-- 
Reply to this email directly or view it on GitHub:
https://github.com/openstreetmap/openstreetmap-website/pull/7148#pullrequestreview-4781833111
You are receiving this because you are subscribed to this thread.

Message ID: <openstreetmap/openstreetmap-website/pull/7148/review/4781833111 at github.com>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.openstreetmap.org/pipermail/rails-dev/attachments/20260726/127b9105/attachment.htm>