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

[Pablo Brasero]

@pablobm commented on this pull request.



> +  end
+
+  def check_revocation(original, changes)
+    original.revoker = current_user if original.active? && !projected_record(original, changes).active?
+  end
+
+  def updating_without_revoking?(original, changes)
+    original.active? && projected_record(original, changes).active?
+  end
+
+  def reactivating?(original, changes)
+    !original.active? && projected_record(original, changes).active?
+  end
+
+  def projected_record(original, changes)
+    original.dup.tap do |duplicate|

Alternative solution:

```ruby
  def update
    if cannot?(:update, @moderation_zone)
      flash[:error] = @moderation_zone.revoker ? t(".only_creator_or_revoker_can_edit") : t(".only_creator_can_edit")
      redirect_to moderation_zones_url
      return
    end

    original_was_active = @moderation_zone.active?
    @moderation_zone.assign_attributes(moderation_zone_params)

    check_revocation(@moderation_zone, original_was_active)

    if current_user != @moderation_zone.creator && updating_without_revoking?(@moderation_zone, original_was_active)
      flash[:error] = t(".only_creator_can_edit_without_revoking")
      redirect_to moderation_zones_url
    elsif reactivating?(@moderation_zone, original_was_active)
      flash[:error] = t(".no_reactivation")
      redirect_to moderation_zones_url
    elsif @moderation_zone.save
      redirect_to moderation_zones_url, :notice => t(".success"), :status => :see_other
    else
      render :edit, :status => :unprocessable_content
    end
  end

  private

  def set_moderation_zone
    @moderation_zone = ModerationZone.find(params.expect(:id))
  end

  def moderation_zone_params
    params.expect(:moderation_zone => [:name, :reason, :zone, :period]).tap do |safe_params|
      safe_params[:ends_at] = safe_params.delete("period").to_i.hours.from_now
    end
  end

  def check_revocation(edited_record, original_was_active)
    edited_record.revoker = current_user if original_was_active && !edited_record.active?
  end

  def updating_without_revoking?(edited_record, original_was_active)
    original_was_active && edited_record.active?
  end

  def reactivating?(edited_record, original_was_active)
    !original_was_active && edited_record.active?
  end
```

I'm not set on a specific solution. The main thing I wanted was to make it easier to follow than the equivalent code at `UserBlocksController`. If we agree on a good solution, we can then refactor `UserBlocksController` to do the same thing.

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

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