@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/2619620...@github.com>
_______________________________________________
rails-dev mailing list
rails-dev@openstreetmap.org
https://lists.openstreetmap.org/listinfo/rails-dev

Reply via email to