@tomhughes requested changes on this pull request.
> @@ -9,6 +9,13 @@ SET xmloption = content;
SET client_min_messages = warning;
SET row_security = off;
+--
+-- Name: public; Type: SCHEMA; Schema: -; Owner: -
+--
+
+-- *not* creating schema, since initdb creates it
+
+
This block is noise created by the version of `pg_dump` you're using and has
nothing to do with the actual changes in this PR so should be removed.
> @@ -0,0 +1,13 @@
+class CreateNoteTags < ActiveRecord::Migration[7.2]
+ def change
+ # Create a table and primary key
+ create_table :note_tags, :primary_key => [:note_id, :k] do |t|
+ t.column "note_id", :bigint, :null => false
+ t.column "k", :string, :default => "", :null => false
+ t.column "v", :string, :default => "", :null => false
+ end
+
+ # Add foreign key without validation
+ add_foreign_key :note_tags, :notes, :column => :note_id, :name =>
"note_tags_id_fkey", :validate => false
Would moving this inside the `create_table` as `t.foreign_key` avoid the need
to create it with validation disabled? Not sure if the strong migrations is
clever enough to know that is safe?
If not then you can just add a second migration to enable validation as it's
safe to do on an empty table.
> @@ -50,7 +50,8 @@ OSM.NewNote = function (map) {
data: {
lat: location.lat,
lon: location.lng,
- text: $(form.text).val()
+ text: $(form.text).val(),
+ tags: "created_by:OpenStreetMaps-Website"
There is no `s` on the end of OpenStreetMap ;-) I'm not sure what is the best
value for the tag here but it definitely shouldn't have the trailing s.
> @@ -83,12 +85,30 @@ def create
lat = OSM.parse_float(params[:lat], OSM::APIBadUserInput, "lat was not a
number")
comment = params[:text]
+ # Extract the tags parameter (if it exists)
+ tags = []
+ if params[:tags].present?
+ # Split by commas to get individual key-value pairs
+ pairs = params[:tags].split(",")
+
+ # For each pair in parameters, store it in tags variable
+ pairs.each do |pair|
+ key, value = pair.split(":", 2)
+ tags << { :k => sanitize(key), :v => sanitize(value) } if key &&
value
+ end
+ end
Encoding all the tags in a string like this is nasty and creates all sorts of
problems if people use colons or commas in tag names or values - colons in
particular are historically common in tag names for other object types.
I'm not sure what the solution is as this API is unusual in using form
parameters rather than an XML or JSON body to create an object.
--
Reply to this email directly or view it on GitHub:
https://github.com/openstreetmap/openstreetmap-website/pull/5344#pullrequestreview-2456745482
You are receiving this because you are subscribed to this thread.
Message ID:
<openstreetmap/openstreetmap-website/pull/5344/review/2456745...@github.com>_______________________________________________
rails-dev mailing list
rails-dev@openstreetmap.org
https://lists.openstreetmap.org/listinfo/rails-dev
