[openstreetmap/openstreetmap-website] Add status filter to user's note page (PR #5297)

[Tom Hughes]

@tomhughes requested changes on this pull request.

In addition to the inline comment this could do with tests at the controller or system level rather than just testing the model method.

> @@ -34,6 +34,16 @@ class Note < ApplicationRecord
 
   scope :visible, -> { where.not(:status => "hidden") }
   scope :invisible, -> { where(:status => "hidden") }
+  scope :with_status, lambda { |status|
+    case status
+    when "open"
+      where(:status => "open")
+    when "closed"
+      where(:status => "closed")
+    else
+      all
+    end
+  }

Rather than trying to handle the all case specially here I think I would make this much simpler, like the `with_status` on the issue model, and then add an `unless params[:status] == "all"` guard in the controller so that the filter isn't applied when all statuses are requested.

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

Message ID: <openstreetmap/openstreetmap-website/pull/5297/review/2416462718 at github.com>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.openstreetmap.org/pipermail/rails-dev/attachments/20241105/77331b40/attachment.htm>