[openstreetmap/openstreetmap-website] Add a gpx_tracks table and convert trace points into linestrings (PR #7348)

Pablo Brasero notifications at github.com
Wed Sep 2 11:41:26 UTC 2026


@pablobm commented on this pull request.

Are we ok with losing the points without timestamp? Has this been discussed?

Incidentally, I realised that the `Gpx` module doesn't have tests, and we don't have any fixtures for files with more than one `<trkpt>`. Not for this PR, but we should add something.

> +  def test_coordinates_are_saved_in_degrees
+    trace = create(:trace)
+    timestamp = Time.utc(2026, 1, 1)
+    create(:tracepoint, :trace => trace, :latitude => 123_456_789, :longitude => -770_123_456,
+                        :altitude => 100.5, :timestamp => timestamp)
+    create(:tracepoint, :trace => trace, :latitude => 123_456_790, :longitude => -770_123_455,
+                        :altitude => 101.5, :timestamp => timestamp + 1)
+
+    TraceLinestringJob.perform_now(trace)
+
+    lon, lat, altitude, time = first_point(trace)
+
+    assert_in_delta(-77.0123456, lon, 0.0000001)
+    assert_in_delta(12.3456789, lat, 0.0000001)
+    assert_in_delta(100.5, altitude, 0.001)
+    assert_in_delta(timestamp.to_i, time, 0.001)

I think this one can be `assert_equal` as it's integers, even if `time` is a float with `.0`:


```suggestion
    assert_equal(timestamp.to_i, time)
```

> +    segments = ApplicationRecord.transaction do
+      trace.gpx_tracks.delete_all
+
+      ApplicationRecord.connection.exec_update(sql, "InsertGpxTracks")
+    end
+
+    logger.info "No segments inserted for trace #{trace.id}" if segments.zero?
+
+    segments

Tiny thing, but making sure that the variable name doesn't read like it's a list, when it's a number:

```suggestion
    segment_count = ApplicationRecord.transaction do
      trace.gpx_tracks.delete_all

      ApplicationRecord.connection.exec_update(sql, "InsertGpxTracks")
    end

    logger.info "No segments inserted for trace #{trace.id}" if segment_count.zero?

    segment_count
```

> @@ -61,6 +61,8 @@ max_note_query_limit: 10000
 max_issues_count: 99
 # Maximum number of points in a GPX trace
 max_trace_size: 1000000
+# Maximum number of points in one gpx_tracks segment
+max_points_per_track_segment: 500

Is the number arbitrary or do we have some idea of what is appropriate?

> @@ -0,0 +1,16 @@
+# frozen_string_literal: true
+
+class CreateGpxTracks < ActiveRecord::Migration[8.1]
+  def change
+    create_table :gpx_tracks, :primary_key => [:gpx_id, :trackid, :segment] do |t|
+      t.bigint :gpx_id, :null => false
+      t.integer :trackid, :null => false
+      t.integer :segment, :null => false
+      t.column :geom, "geometry(GeometryZM,4326)", :null => false

```suggestion
      t.column :geom, "geometry(GeometryZM,4326)", :null => false
      
```

Very annoying nitpick sorry 😅  but I think it reads better if we separate the columns from the other information.

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

Message ID: <openstreetmap/openstreetmap-website/pull/7348/review/5088362156 at github.com>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.openstreetmap.org/pipermail/rails-dev/attachments/20260902/3a7c267d/attachment.htm>


More information about the rails-dev mailing list