From 1f7e22efdb816d72a57555a5334e724c86e14001 Mon Sep 17 00:00:00 2001
From: Zee Spencer <50284+zspencer@users.noreply.github.com>
Date: Thu, 13 Jul 2023 15:23:30 -0700
Subject: [PATCH] Apply feedback from Ana's code review:
- Rename to `Keyword`
- Use an `array` field!
- validate!
---
app/furniture/journal/entry.rb | 7 +++----
app/furniture/journal/journal.rb | 2 +-
app/furniture/journal/keyword.rb | 18 ++++++++++++++++++
app/furniture/journal/renderer.rb | 1 -
app/furniture/journal/term.rb | 18 ------------------
.../20230704064303_add_journal_keywords.rb | 10 ++++++++++
db/migrate/20230704064303_add_journal_terms.rb | 8 --------
db/schema.rb | 10 +++++-----
spec/furniture/journal/entry_spec.rb | 14 +++++++-------
spec/furniture/journal/journal_spec.rb | 2 +-
spec/furniture/journal/keyword_spec.rb | 9 +++++++++
spec/furniture/journal/term_spec.rb | 7 -------
12 files changed, 54 insertions(+), 52 deletions(-)
create mode 100644 app/furniture/journal/keyword.rb
delete mode 100644 app/furniture/journal/term.rb
create mode 100644 db/migrate/20230704064303_add_journal_keywords.rb
delete mode 100644 db/migrate/20230704064303_add_journal_terms.rb
create mode 100644 spec/furniture/journal/keyword_spec.rb
delete mode 100644 spec/furniture/journal/term_spec.rb
diff --git a/app/furniture/journal/entry.rb b/app/furniture/journal/entry.rb
index 0e2a60f60..2670b2731 100644
--- a/app/furniture/journal/entry.rb
+++ b/app/furniture/journal/entry.rb
@@ -30,8 +30,7 @@ class Entry < ApplicationRecord
belongs_to :journal, inverse_of: :entries
has_one :room, through: :journal
has_one :space, through: :journal
- has_many :terms, through: :journal
- after_save :extract_terms, if: :saved_change_to_body?
+ after_save :extract_keywords, if: :saved_change_to_body?
def published?
published_at.present?
@@ -52,8 +51,8 @@ def self.renderer
)
end
- def extract_terms
- journal.terms.extract_and_create_from!(body)
+ def extract_keywords
+ journal.keywords.extract_and_create_from!(body)
end
def to_param
diff --git a/app/furniture/journal/journal.rb b/app/furniture/journal/journal.rb
index 8a0a13d80..a7ee1410c 100644
--- a/app/furniture/journal/journal.rb
+++ b/app/furniture/journal/journal.rb
@@ -3,5 +3,5 @@ class Journal::Journal < Furniture
extend StripsNamespaceFromModelName
has_many :entries, inverse_of: :journal, dependent: :destroy
- has_many :terms, inverse_of: :journal, dependent: :destroy
+ has_many :keywords, inverse_of: :journal, dependent: :destroy
end
diff --git a/app/furniture/journal/keyword.rb b/app/furniture/journal/keyword.rb
new file mode 100644
index 000000000..bff92119e
--- /dev/null
+++ b/app/furniture/journal/keyword.rb
@@ -0,0 +1,18 @@
+class Journal
+ class Keyword < ApplicationRecord
+ location(parent: :journal)
+
+ self.table_name = "journal_keywords"
+ validates :canonical_keyword, presence: true, uniqueness: {case_sensitive: false, scope: :journal_id}
+ belongs_to :journal, inverse_of: :keywords
+
+ def self.extract_and_create_from!(body)
+ body.scan(/#(\w+)/)&.flatten&.each do |keyword|
+ next if where(":aliases = ANY (aliases)", aliases: keyword)
+ .or(where(canonical_keyword: keyword)).exists?
+
+ find_or_create_by!(canonical_keyword: keyword)
+ end
+ end
+ end
+end
diff --git a/app/furniture/journal/renderer.rb b/app/furniture/journal/renderer.rb
index 200936763..a3d07cfa5 100644
--- a/app/furniture/journal/renderer.rb
+++ b/app/furniture/journal/renderer.rb
@@ -8,7 +8,6 @@ def autolink(link, link_type)
def postprocess(doc)
doc
.gsub(/@([a-zA-Z\d]*)@(.*\.[a-zA-Z]*)/, '@\1@\2')
- # .gsub(/#(\w+)/, '#\1')
end
end
end
diff --git a/app/furniture/journal/term.rb b/app/furniture/journal/term.rb
deleted file mode 100644
index f9d067aa5..000000000
--- a/app/furniture/journal/term.rb
+++ /dev/null
@@ -1,18 +0,0 @@
-class Journal
- class Term < ApplicationRecord
- location(parent: :journal)
-
- self.table_name = "journal_terms"
- validates :canonical_term, presence: true
- belongs_to :journal, inverse_of: :terms
-
- def self.extract_and_create_from!(body)
- body.scan(/#(\w+)/)&.flatten&.each do |term|
- next if where("aliases ILIKE ?", term)
- .or(where(canonical_term: term)).exists?
-
- find_or_create_by!(canonical_term: term)
- end
- end
- end
-end
diff --git a/db/migrate/20230704064303_add_journal_keywords.rb b/db/migrate/20230704064303_add_journal_keywords.rb
new file mode 100644
index 000000000..351b87df4
--- /dev/null
+++ b/db/migrate/20230704064303_add_journal_keywords.rb
@@ -0,0 +1,10 @@
+class AddJournalKeywords < ActiveRecord::Migration[7.0]
+ def change
+ create_table :journal_keywords, id: :uuid do |t|
+ t.references :journal, type: :uuid, foreign_key: {to_table: :furnitures}
+ t.string :canonical_keyword
+ t.string :aliases, array: true
+ t.timestamps
+ end
+ end
+end
diff --git a/db/migrate/20230704064303_add_journal_terms.rb b/db/migrate/20230704064303_add_journal_terms.rb
deleted file mode 100644
index 2e0ad6643..000000000
--- a/db/migrate/20230704064303_add_journal_terms.rb
+++ /dev/null
@@ -1,8 +0,0 @@
-class AddJournalTerms < ActiveRecord::Migration[7.0]
- create_table :journal_terms, id: :uuid do |t|
- t.references :journal, type: :uuid, foreign_key: {to_table: :furnitures}
- t.string :canonical_term
- t.text :aliases
- t.timestamps
- end
-end
diff --git a/db/schema.rb b/db/schema.rb
index 602a104da..a52718609 100644
--- a/db/schema.rb
+++ b/db/schema.rb
@@ -117,13 +117,13 @@
t.index ["journal_id"], name: "index_journal_entries_on_journal_id"
end
- create_table "journal_terms", id: :uuid, default: -> { "gen_random_uuid()" }, force: :cascade do |t|
+ create_table "journal_keywords", id: :uuid, default: -> { "gen_random_uuid()" }, force: :cascade do |t|
t.uuid "journal_id"
- t.string "canonical_term"
- t.text "aliases"
+ t.string "canonical_keyword"
+ t.string "aliases", array: true
t.datetime "created_at", null: false
t.datetime "updated_at", null: false
- t.index ["journal_id"], name: "index_journal_terms_on_journal_id"
+ t.index ["journal_id"], name: "index_journal_keywords_on_journal_id"
end
create_table "marketplace_cart_products", id: :uuid, default: -> { "gen_random_uuid()" }, force: :cascade do |t|
@@ -285,7 +285,7 @@
add_foreign_key "active_storage_attachments", "active_storage_blobs", column: "blob_id"
add_foreign_key "active_storage_variant_records", "active_storage_blobs", column: "blob_id"
add_foreign_key "journal_entries", "furnitures", column: "journal_id"
- add_foreign_key "journal_terms", "furnitures", column: "journal_id"
+ add_foreign_key "journal_keywords", "furnitures", column: "journal_id"
add_foreign_key "marketplace_cart_products", "marketplace_orders", column: "cart_id"
add_foreign_key "marketplace_cart_products", "marketplace_products", column: "product_id"
add_foreign_key "marketplace_delivery_areas", "furnitures", column: "marketplace_id"
diff --git a/spec/furniture/journal/entry_spec.rb b/spec/furniture/journal/entry_spec.rb
index 6001fcd6d..f8ee56790 100644
--- a/spec/furniture/journal/entry_spec.rb
+++ b/spec/furniture/journal/entry_spec.rb
@@ -9,7 +9,6 @@
it { is_expected.to belong_to(:journal).inverse_of(:entries) }
it { is_expected.to have_one(:room).through(:journal) }
it { is_expected.to have_one(:space).through(:journal) }
- it { is_expected.to have_many(:terms).through(:journal) }
describe "#to_html" do
subject(:to_html) { entry.to_html }
@@ -23,18 +22,19 @@
describe "#save" do
let(:entry) { create(:journal_entry, body: "#GoodTimes") }
+ let(:journal) { entry.journal }
context "when the body is changing" do
- it "idempotently creates terms in the journal" do
- bad_apple = entry.journal.terms.create!(canonical_term: "BadApple", aliases: "BadApples")
- good_times = entry.journal.terms.create!(canonical_term: "GoodTimes")
+ it "idempotently creates `Keywords` in the `Journal`" do
+ bad_apple = entry.journal.keywords.create!(canonical_keyword: "BadApple", aliases: ["BadApples"])
+ good_times = entry.journal.keywords.find_by!(canonical_keyword: "GoodTimes")
expect do
entry.update!(body: "#GoodTimes #HardCider #BadApples")
end.not_to change { "#{bad_apple.reload.updated_at} - #{good_times.reload.updated_at}" }
- expect(Journal::Term.where(canonical_term: "GoodTimes")).to exist
- expect(Journal::Term.where(canonical_term: "HardCider")).to exist
- expect(Journal::Term.where(canonical_term: "BadApples")).not_to exist
+ expect(journal.keywords.where(canonical_keyword: "GoodTimes")).to exist
+ expect(journal.keywords.where(canonical_keyword: "HardCider")).to exist
+ expect(journal.keywords.where(canonical_keyword: "BadApples")).not_to exist
end
end
end
diff --git a/spec/furniture/journal/journal_spec.rb b/spec/furniture/journal/journal_spec.rb
index bc34a0741..87df44dfc 100644
--- a/spec/furniture/journal/journal_spec.rb
+++ b/spec/furniture/journal/journal_spec.rb
@@ -4,5 +4,5 @@
RSpec.describe Journal::Journal, type: :model do
it { is_expected.to have_many(:entries).inverse_of(:journal).dependent(:destroy) }
- it { is_expected.to have_many(:terms).inverse_of(:journal).dependent(:destroy) }
+ it { is_expected.to have_many(:keywords).inverse_of(:journal).dependent(:destroy) }
end
diff --git a/spec/furniture/journal/keyword_spec.rb b/spec/furniture/journal/keyword_spec.rb
new file mode 100644
index 000000000..dba78f742
--- /dev/null
+++ b/spec/furniture/journal/keyword_spec.rb
@@ -0,0 +1,9 @@
+# frozen_string_literal: true
+
+require "rails_helper"
+
+RSpec.describe Journal::Keyword, type: :model do
+ it { is_expected.to validate_presence_of(:canonical_keyword) }
+ it { is_expected.to validate_uniqueness_of(:canonical_keyword).case_insensitive.scoped_to(:journal_id) }
+ it { is_expected.to belong_to(:journal).inverse_of(:keywords) }
+end
diff --git a/spec/furniture/journal/term_spec.rb b/spec/furniture/journal/term_spec.rb
deleted file mode 100644
index d7f9f4a18..000000000
--- a/spec/furniture/journal/term_spec.rb
+++ /dev/null
@@ -1,7 +0,0 @@
-# frozen_string_literal: true
-
-require "rails_helper"
-
-RSpec.describe Journal::Term, type: :model do
- it { is_expected.to belong_to(:journal).inverse_of(:terms) }
-end