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

[Tom Hughes]

@tomhughes requested changes on this pull request.

I wonder if it's worth leaving the layer selector out for now? There's a lot of work there and presumably once the front page switches to maplibre we'll have a better layer selector available to use instead of rolling our own?

> +    reversible do |dir|
+      dir.up do
+        safety_assured do
+          execute "UPDATE moderation_zones SET ends_at = NOW()"
+        end
+      end
+    end

Is this really reversible without a `down` method? What happens if it's run during a downgrade? Does it throw an exception or is it a no-op?

> @@ -0,0 +1,15 @@
+# frozen_string_literal: true
+
+class TweakModerationZonesStepOne < ActiveRecord::Migration[8.1]
+  def change
+    reversible do |dir|
+      dir.up do
+        safety_assured do

So we're going to a lot of trouble to keep strong migrations happy with the null constraint addition, but then just using `safety_assured` to blow throw the barriers for setting the value?

Obviously none of this is a real issue when the table has no records currently but the official "safe" way to do this would be to do it in batches?

> @@ -0,0 +1,281 @@
+# frozen_string_literal: true
+
+require "test_helper"
+
+class ModerationZonesControllerTest < ActionDispatch::IntegrationTest

This needs our standard route test?

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

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