[openstreetmap/openstreetmap-website] On-site notifications (PR #7030)

[Tom Hughes]

@tomhughes commented on this pull request.

So I think my main question is about the "view tests" which is not something we've had before...

What was the reason for rendering the view directly rather than going through the controller in the way most of our controller tests do? If we're going to do this sort of test instead then is `test/controllers` the normal rails place to put them?

> +  def each(&)
+    notification_records.each(&)
+  end

Is anything using this method now? It was in use before the final commit to add pagination but I'm not sure it is now? Certainly it's behaviour changed when pagination is added as the objects it yields are no longer wrapped...

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

We should probably have our normal `test_routes` test here.

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

Message ID: <openstreetmap/openstreetmap-website/pull/7030/review/4620119454 at github.com>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.openstreetmap.org/pipermail/rails-dev/attachments/20260702/0c9a3629/attachment.htm>