Adds non-mutable note tags support

app/assets/javascripts/index/new_note.js

@@ -50,7 +50,10 @@ OSM.NewNote = function (map) {
       data: {
         lat: location.lat,
         lon: location.lng,
-        text: $(form.text).val()
+        text: $(form.text).val(),
+        tags: JSON.stringify({
+          created_by: "OpenStreetMap-Website"
+        })
       },
       success: function (feature) {
         noteCreated(feature, marker);

app/controllers/api/notes_controller.rb

@@ -83,12 +83,26 @@ def create
       lat = OSM.parse_float(params[:lat], OSM::APIBadUserInput, "lat was not a number")
       comment = params[:text]
 
+      # Extract the tags, if present
+      tags = []
+      if params[:tags].present?
+        parsed_tags = JSON.parse(params[:tags])
+        parsed_tags.each do |key, value|
+          tags << { :k => key.presence || "", :v => value.presence || "" }
+        end
+      end
+
       # Include in a transaction to ensure that there is always a note_comment for every note
       Note.transaction do
         # Create the note
         @note = Note.create(:lat => lat, :lon => lon)
         raise OSM::APIBadUserInput, "The note is outside this world" unless @note.in_world?
 
+        # Create a NoteTag for each tag in the tags array
+        tags.each do |tag|
+          @note.note_tags.create(:k => tag[:k], :v => tag[:v])
+        end
+
         # Save the note
         @note.save!
 

app/models/note.rb

@@ -26,6 +26,8 @@ class Note < ApplicationRecord
   has_many :subscriptions, :class_name => "NoteSubscription"
   has_many :subscribers, :through => :subscriptions, :source => :user
 
+  has_many :note_tags
+
   validates :id, :uniqueness => true, :presence => { :on => :update },
                  :numericality => { :on => :update, :only_integer => true }
   validates :latitude, :longitude, :numericality => { :only_integer => true }
@@ -92,6 +94,16 @@ def author_ip
     comments.first.author_ip
   end
 
+  def tags
+    unless @tags
+      @tags = {}
+      note_tags.each do |tag|
+        @tags[tag.k] = tag.v
+      end
+    end
+    @tags
+  end
+
   private
 
   # Fill in default values for new notes

app/models/note_tag.rb

@@ -0,0 +1,20 @@
+# == Schema Information
+#
+# Table name: note_tags
+#
+#  note_id :bigint(8)        not null, primary key
+#  k       :string           default(""), not null, primary key
+#  v       :string           default(""), not null
+#
+# Foreign Keys
+#
+#  note_tags_id_fkey  (note_id => notes.id)
+#
+
+class NoteTag < ApplicationRecord
+  belongs_to :note
+
+  validates :note, :associated => true
+  validates :k, :v, :allow_blank => true, :length => { :maximum => 255 }, :characters => true
+  validates :k, :uniqueness => { :scope => :note_id }
+end

app/views/api/notes/_note.gpx.builder

@@ -23,5 +23,9 @@ xml.wpt("lon" => note.lon, "lat" => note.lat) do
     xml.status note.status
 
     xml.date_closed note.closed_at if note.closed?
+
+    note.tags.each do |k, v|
+      xml.tag(:k => k, :v => v)
+    end
   end
 end

app/views/api/notes/_note.json.jbuilder

@@ -20,6 +20,8 @@ json.properties do
   json.status note.status
   json.closed_at note.closed_at.to_s if note.closed?
 
+  json.tags note.tags unless note.tags.empty?
+
   json.comments(note.comments) do |comment|
     json.date comment.created_at.to_s
 

app/views/api/notes/_note.xml.builder

@@ -14,6 +14,10 @@ xml.note("lon" => note.lon, "lat" => note.lat) do
 
   xml.date_closed note.closed_at if note.closed?
 
+  note.tags.each do |k, v|
+    xml.tag(:k => k, :v => v)
+  end
+
   xml.comments do
     note.comments.each do |comment|
       xml.comment do

app/views/notes/show.html.erb

@@ -22,6 +22,8 @@
     </p>
   </div>
 
+  <%= render :partial => "browse/tag_details", :object => @note.tags %>
+
   <% if @note_comments.find { |comment| comment.author.nil? } -%>
     <p class='alert alert-warning'><%= t ".anonymous_warning" %></p>
   <% end -%>

db/migrate/20241030122707_create_note_tags.rb

@@ -0,0 +1,12 @@
+class CreateNoteTags < ActiveRecord::Migration[7.2]
+  def change
+    # Create a table, primary and foreign keys
+    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
+
+      t.foreign_key :notes, :column => :note_id, :name => "note_tags_id_fkey"
+    end
+  end
+end

db/structure.sql

@@ -1061,6 +1061,17 @@ CREATE TABLE public.note_subscriptions (
 );
 
 
+--
+-- Name: note_tags; Type: TABLE; Schema: public; Owner: -
+--
+
+CREATE TABLE public.note_tags (
+    note_id bigint NOT NULL,
+    k character varying DEFAULT ''::character varying NOT NULL,
+    v character varying DEFAULT ''::character varying NOT NULL
+);
+
+
 --
 -- Name: notes; Type: TABLE; Schema: public; Owner: -
 --
@@ -2028,6 +2039,14 @@ ALTER TABLE ONLY public.note_subscriptions
     ADD CONSTRAINT note_subscriptions_pkey PRIMARY KEY (user_id, note_id);
 
 
+--
+-- Name: note_tags note_tags_pkey; Type: CONSTRAINT; Schema: public; Owner: -
+--
+
+ALTER TABLE ONLY public.note_tags
+    ADD CONSTRAINT note_tags_pkey PRIMARY KEY (note_id, k);
+
+
 --
 -- Name: notes notes_pkey; Type: CONSTRAINT; Schema: public; Owner: -
 --
@@ -3210,6 +3229,14 @@ ALTER TABLE ONLY public.note_comments
     ADD CONSTRAINT note_comments_note_id_fkey FOREIGN KEY (note_id) REFERENCES public.notes(id);
 
 
+--
+-- Name: note_tags note_tags_id_fkey; Type: FK CONSTRAINT; Schema: public; Owner: -
+--
+
+ALTER TABLE ONLY public.note_tags
+    ADD CONSTRAINT note_tags_id_fkey FOREIGN KEY (note_id) REFERENCES public.notes(id);
+
+
 --
 -- Name: redactions redactions_user_id_fkey; Type: FK CONSTRAINT; Schema: public; Owner: -
 --
@@ -3397,6 +3424,7 @@ INSERT INTO "schema_migrations" (version) VALUES
 ('23'),
 ('22'),
 ('21'),
+('20241030122707'),
 ('20241023004427'),
 ('20241022141247'),
 ('20240913171951'),

test/controllers/api/notes_controller_test.rb

@@ -1,4 +1,5 @@
 require "test_helper"
+require "json"
 
 module Api
   class NotesControllerTest < ActionDispatch::IntegrationTest
@@ -105,7 +106,18 @@ def test_create_anonymous_success
       assert_difference "Note.count", 1 do
         assert_difference "NoteComment.count", 1 do
           assert_no_difference "NoteSubscription.count" do
-            post api_notes_path(:lat => -1.0, :lon => -1.0, :text => "This is a comment", :format => "json")
+            assert_difference "NoteTag.count", 2 do
+              post api_notes_path(
+                :lat => -1.0,
+                :lon => -1.0,
+                :tags => {
+                  "created_by" => "OSM_TEST",
+                  "삭ÒX~`!@#$%^&*()-=_+,<.>/?;:'\"[{}]\\|傥4ր" => "Ƭ߯ĸá~`!@#$%^&*()-=_+,<.>/?;:'\"[{}]\\|؇Őϋ"
+                }.to_json,
+                :text => "This is a comment",
+                :format => "json"
+              )
+            end
           end
         end
       end
@@ -116,6 +128,9 @@ def test_create_anonymous_success
       assert_equal "Point", js["geometry"]["type"]
       assert_equal [-1.0, -1.0], js["geometry"]["coordinates"]
       assert_equal "open", js["properties"]["status"]
+      assert_equal 2, js["properties"]["tags"].count
+      assert_equal "OSM_TEST", js["properties"]["tags"]["created_by"]
+      assert_equal "Ƭ߯ĸá~`!@#$%^&*()-=_+,<.>/?;:'\"[{}]\\|؇Őϋ", js["properties"]["tags"]["삭ÒX~`!@#$%^&*()-=_+,<.>/?;:'\"[{}]\\|傥4ր"]
       assert_equal 1, js["properties"]["comments"].count
       assert_equal "opened", js["properties"]["comments"].last["action"]
       assert_equal "This is a comment", js["properties"]["comments"].last["text"]
@@ -131,6 +146,9 @@ def test_create_anonymous_success
       assert_equal [-1.0, -1.0], js["geometry"]["coordinates"]
       assert_equal id, js["properties"]["id"]
       assert_equal "open", js["properties"]["status"]
+      assert_equal 2, js["properties"]["tags"].count
+      assert_equal "OSM_TEST", js["properties"]["tags"]["created_by"]
+      assert_equal "Ƭ߯ĸá~`!@#$%^&*()-=_+,<.>/?;:'\"[{}]\\|؇Őϋ", js["properties"]["tags"]["삭ÒX~`!@#$%^&*()-=_+,<.>/?;:'\"[{}]\\|傥4ր"]
       assert_equal 1, js["properties"]["comments"].count
       assert_equal "opened", js["properties"]["comments"].last["action"]
       assert_equal "This is a comment", js["properties"]["comments"].last["text"]
@@ -594,6 +612,8 @@ def test_reopen_fail
 
     def test_show_success
       open_note = create(:note_with_comments)
+      create(:note_tag, :note => open_note, :k => "created_by", :v => "OSM_TEST")
+      create(:note_tag, :note => open_note, :k => "삭ÒX~`!@#$%^&*()-=_+,<.>/?;:'\"[{}]\\|傥4ր", :v => "Ƭ߯ĸá~`!@#$%^&*()-=_+,<.>/?;:'\"[{}]\\|؇Őϋ")
 
       get api_note_path(open_note, :format => "xml")
       assert_response :success
@@ -610,6 +630,8 @@ def test_show_success
             assert_select "comment", :count => 1
           end
         end
+        assert_select "tag[k='created_by'][v='OSM_TEST']", :count => 1
+        assert_select "tag[k='삭ÒX~`!@#$%^&*()-=_+,<.>/?;:\\'\"[{}]\\\\|傥4ր'][v='Ƭ߯ĸá~`!@#$%^&*()-=_+,<.>/?;:\\'\"[{}]\\\\|؇Őϋ']", :count => 1
       end
 
       get api_note_path(open_note, :format => "rss")
@@ -643,6 +665,8 @@ def test_show_success
       assert_equal close_api_note_url(open_note, :format => "json"), js["properties"]["close_url"]
       assert_equal open_note.created_at.to_s, js["properties"]["date_created"]
       assert_equal open_note.status, js["properties"]["status"]
+      assert_equal "OSM_TEST", js["properties"]["tags"]["created_by"]
+      assert_equal "Ƭ߯ĸá~`!@#$%^&*()-=_+,<.>/?;:'\"[{}]\\|؇Őϋ", js["properties"]["tags"]["삭ÒX~`!@#$%^&*()-=_+,<.>/?;:'\"[{}]\\|傥4ր"]
 
       get api_note_path(open_note, :format => "gpx")
       assert_response :success
@@ -658,6 +682,8 @@ def test_show_success
             assert_select "url", api_note_url(open_note, :format => "gpx")
             assert_select "comment_url", comment_api_note_url(open_note, :format => "gpx")
             assert_select "close_url", close_api_note_url(open_note, :format => "gpx")
+            assert_select "tag[k='created_by'][v='OSM_TEST']", :count => 1
+            assert_select "tag[k='삭ÒX~`!@#$%^&*()-=_+,<.>/?;:\\'\"[{}]\\\\|傥4ր'][v='Ƭ߯ĸá~`!@#$%^&*()-=_+,<.>/?;:\\'\"[{}]\\\\|؇Őϋ']", :count => 1
           end
         end
       end

test/controllers/notes_controller_test.rb

@@ -67,6 +67,27 @@ def test_index_success
     assert_response :not_found
   end
 
+  def test_displaying_note_with_tags
+    user = create(:user)
+
+    note = create(:note)
+    create(:note_comment, :note => note, :author => user, :body => "Note description")
+    create(:note_tag, :note => note, :k => "created_by", :v => "OSM_TEST")
+    create(:note_tag, :note => note, :k => "삭ÒX~`!@#$%^&*()-=_+,<.>/?;:'\"[{}]\\|傥4ր", :v => "Ƭ߯ĸá~`!@#$%^&*()-=_+,<.>/?;:'\"[{}]\\|؇Őϋ")
+
+    sidebar_browse_check :note_path, note.id, "notes/show"
+    assert_dom "h2", :text => "Unresolved note ##{note.id}"
+    assert_dom "p", :text => "Note description"
+    assert_dom "tr" do
+      assert_dom "th", :text => "created_by"
+      assert_dom "td", :text => "OSM_TEST"
+    end
+    assert_dom "tr" do
+      assert_dom "th", :text => "삭ÒX~`!@#$%^&*()-=_+,<.>/?;:'\"[{}]\\|傥4ր"
+      assert_dom "td", :text => "Ƭ߯ĸá~`!@#$%^&*()-=_+,<.>/?;:'\"[{}]\\|؇Őϋ"
+    end
+  end
+
   def test_index_paged
     user = create(:user)
 

test/factories/note_tags.rb

@@ -0,0 +1,8 @@
+FactoryBot.define do
+  factory :note_tag do
+    sequence(:k) { |n| "Key #{n}" }
+    sequence(:v) { |n| "Value #{n}" }
+
+    note
+  end
+end

test/models/note_tag_test.rb

@@ -0,0 +1,49 @@
+require "test_helper"
+
+class NoteTagTest < ActiveSupport::TestCase
+  def test_length_key_valid
+    tag = create(:note_tag)
+    [0, 255].each do |i|
+      tag.k = "k" * i
+      assert_predicate tag, :valid?
+    end
+  end
+
+  def test_length_value_valid
+    tag = create(:note_tag)
+    [0, 255].each do |i|
+      tag.v = "v" * i
+      assert_predicate tag, :valid?
+    end
+  end
+
+  def test_length_key_invalid
+    tag = create(:note_tag)
+    tag.k = "k" * 256
+    assert_not_predicate tag, :valid?, "Key should be too long"
+    assert_predicate tag.errors[:k], :any?
+  end
+
+  def test_length_value_invalid
+    tag = create(:note_tag)
+    tag.v = "v" * 256
+    assert_not_predicate tag, :valid?, "Value should be too long"
+    assert_predicate tag.errors[:v], :any?
+  end
+
+  def test_orphaned_tag_invalid
+    tag = create(:note_tag)
+    tag.note = nil
+    assert_not_predicate tag, :valid?, "Orphaned tag should be invalid"
+    assert_predicate tag.errors[:note], :any?
+  end
+
+  def test_uniqueness
+    existing = create(:note_tag)
+    tag = build(:note_tag, :note => existing.note, :k => existing.k, :v => existing.v)
+    assert_predicate tag, :new_record?
+    assert_not_predicate tag, :valid?
+    assert_raise(ActiveRecord::RecordInvalid) { tag.save! }
+    assert_predicate tag, :new_record?
+  end
+end