[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