[openstreetmap/openstreetmap-website] Adds non-mutable note tags support (PR #5344)

[Tom Hughes]

@tomhughes commented on this pull request.



> +# 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

Why are we allowing blank values? Possibly there's an argument for a blank value but certainly not for a key?

> @@ -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

Do these really need a default value?

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

Message ID: <openstreetmap/openstreetmap-website/pull/5344/review/2619620938 at github.com>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.openstreetmap.org/pipermail/rails-dev/attachments/20250216/5d531d35/attachment-0001.htm>