[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