[openstreetmap/openstreetmap-website] Simplify GPS trace visibility levels (PR #7146)

Pablo Brasero notifications at github.com
Wed Jul 8 12:32:43 UTC 2026


@pablobm commented on this pull request.

Almost there! 🤞 

> @@ -28,7 +28,7 @@ def test_routes
     end
 
     def test_tracepoints

I think worth renaming as it's a legacy value, and it can confuse readers as to what exactly is being tested here (I was confused), and why a legacy value appears to be a "default" in the sense that the test name doesn't say anything special.

```suggestion
    def test_tracepoints_public
```

> @@ -18,4 +18,15 @@ def trace_icon(trace, options = {})
   def trace_picture(trace, options = {})
     trace_image(trace, :animated => true, :size => 250, **options)
   end
+
+  # Options for the visibility dropdown. Keeps an old value in the list so
+  # editing the trace does not change it by mistake.
+  def trace_visibility_options(trace)
+    visibilities = Trace::VISIBILITIES.dup
+    # If the trace still has an old visibility (private or public), add it so it stays selected.
+    visibilities.unshift(trace.visibility) if Trace.legacy_visibility?(trace.visibility)
+    visibilities.map do |visibility|
+      [t("traces.visibility.#{visibility}"), visibility]
+    end
+  end

Instead of the comments, I would provide a test. And since `test/helpers/trace_helper_test.rb` would be a new test file, you can use the `test "long description"` form to make it more readable.

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

Message ID: <openstreetmap/openstreetmap-website/pull/7146/review/4654034106 at github.com>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.openstreetmap.org/pipermail/rails-dev/attachments/20260708/26a6671b/attachment.htm>


More information about the rails-dev mailing list