Skip to content

Commit

Permalink
Apply feedback from Ana's code review:
Browse files Browse the repository at this point in the history
- Rename to `Keyword`
- Use an `array` field!
- validate!
  • Loading branch information
zspencer committed Jul 13, 2023
1 parent e3fd349 commit 1f7e22e
Show file tree
Hide file tree
Showing 12 changed files with 54 additions and 52 deletions.
7 changes: 3 additions & 4 deletions app/furniture/journal/entry.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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?
Expand All @@ -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
Expand Down
2 changes: 1 addition & 1 deletion app/furniture/journal/journal.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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
18 changes: 18 additions & 0 deletions app/furniture/journal/keyword.rb
Original file line number Diff line number Diff line change
@@ -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
1 change: 0 additions & 1 deletion app/furniture/journal/renderer.rb
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,6 @@ def autolink(link, link_type)
def postprocess(doc)
doc
.gsub(/@([a-zA-Z\d]*)@(.*\.[a-zA-Z]*)/, '<a href="https://\2/@\1">@\1@\2</a>')
# .gsub(/#(\w+)/, '<a href="../terms/\1">#\1</a>')
end
end
end
18 changes: 0 additions & 18 deletions app/furniture/journal/term.rb

This file was deleted.

10 changes: 10 additions & 0 deletions db/migrate/20230704064303_add_journal_keywords.rb
Original file line number Diff line number Diff line change
@@ -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
8 changes: 0 additions & 8 deletions db/migrate/20230704064303_add_journal_terms.rb

This file was deleted.

10 changes: 5 additions & 5 deletions db/schema.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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|
Expand Down Expand Up @@ -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"
Expand Down
14 changes: 7 additions & 7 deletions spec/furniture/journal/entry_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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 }
Expand All @@ -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
Expand Down
2 changes: 1 addition & 1 deletion spec/furniture/journal/journal_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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
9 changes: 9 additions & 0 deletions spec/furniture/journal/keyword_spec.rb
Original file line number Diff line number Diff line change
@@ -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
7 changes: 0 additions & 7 deletions spec/furniture/journal/term_spec.rb

This file was deleted.

0 comments on commit 1f7e22e

Please sign in to comment.