@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/2416462...@github.com>_______________________________________________
rails-dev mailing list
rails-dev@openstreetmap.org
https://lists.openstreetmap.org/listinfo/rails-dev
