diff --git a/MIGRATION_NOTES.md b/MIGRATION_NOTES.md new file mode 100644 index 000000000..680788df7 --- /dev/null +++ b/MIGRATION_NOTES.md @@ -0,0 +1,94 @@ +# Migration Notes for Issue #849 + +## Overview +This PR converts the `quotable_item_quotes` join table pattern to a direct polymorphic relationship between `quotes` and quotable items (workshops, reports, workshop_logs). + +## Database Migrations + +Two migrations have been added: + +### 1. Add Quotable Columns to Quotes (20260215071524) +```ruby +add_reference :quotes, :quotable, polymorphic: true, index: true +``` +This adds `quotable_id` and `quotable_type` columns to the `quotes` table. + +### 2. Migrate Data (20260215071525) +```sql +UPDATE quotes +INNER JOIN quotable_item_quotes ON quotes.id = quotable_item_quotes.quote_id +SET quotes.quotable_id = quotable_item_quotes.quotable_id, + quotes.quotable_type = quotable_item_quotes.quotable_type +``` +This copies the associations from the join table to the quotes table. + +## Running Migrations + +```bash +# Development +bundle exec rails db:migrate + +# Docker +docker compose exec web bundle exec rails db:migrate + +# Or using mise +mise docker-exec rails db:migrate +``` + +## Rollback Plan + +The data migration is reversible: +```bash +bundle exec rails db:rollback STEP=2 +``` + +This will: +1. Clear the `quotable_id` and `quotable_type` columns on quotes +2. Remove the columns from the quotes table + +The original `quotable_item_quotes` table data is preserved and can be used to restore the relationship. + +## Testing the Changes + +```bash +# Run model tests +bundle exec rspec spec/models/quote_spec.rb +bundle exec rspec spec/models/workshop_spec.rb +bundle exec rspec spec/models/report_spec.rb + +# Run all tests +bundle exec rspec +``` + +## Future Work + +After verifying this works in production: +- [ ] Remove the `quotable_item_quotes` table with a new migration +- [ ] Archive or remove the `QuotableItemQuote` model +- [ ] Remove the `quotable_item_quotes` factory + +## Changes Made + +### Models +- **Quote**: Added `belongs_to :quotable, polymorphic: true, optional: true` +- **Workshop**: Changed from `has_many :quotable_item_quotes` to `has_many :quotes, as: :quotable` +- **Report**: Changed from `has_many :quotable_item_quotes` to `has_many :quotes, as: :quotable` +- **Report**: Updated nested attributes from `quotable_item_quotes` to `quotes` + +### Controllers +- **WorkshopLogsController**: Simplified form building logic to use direct quote associations +- **WorkshopLogsController**: Updated permitted params to use `quotes_attributes` + +### Views +- **workshop_logs/_form.html.erb**: Updated to use `:quotes` instead of `:quotable_item_quotes` +- **workshop_logs/_quote_fields.html.erb**: New simplified partial for quote fields +- **quotes/index.html.erb**: Updated to use direct `quote.quotable` instead of iterating through `quotable_item_quotes` +- **quotes/show.html.erb**: Updated to use direct `@quote.quotable` instead of iterating through `quotable_item_quotes` + +### Decorators +- **QuoteDecorator**: Simplified to use `object.quotable` instead of `object.quotable_item_quotes.last&.quotable` + +### Tests +- **spec/models/quote_spec.rb**: Added tests for polymorphic association +- **spec/models/workshop_spec.rb**: Updated association tests +- **spec/models/report_spec.rb**: Updated association tests diff --git a/app/controllers/workshop_logs_controller.rb b/app/controllers/workshop_logs_controller.rb index 4a4952bc6..af4f919eb 100644 --- a/app/controllers/workshop_logs_controller.rb +++ b/app/controllers/workshop_logs_controller.rb @@ -143,17 +143,8 @@ def set_form_variables .order(title: :asc) # Build one blank quote if none exists - @workshop_log.quotable_item_quotes.each do |qiq| - qiq.build_quote unless qiq.quote - qiq.quotable = @workshop_log - end - - # Always build at least one new blank quotable_item_quote - if @workshop_log.quotable_item_quotes.empty? - qiq = @workshop_log.quotable_item_quotes.build - qiq.build_quote - qiq.quotable = @workshop_log - end + # Always build at least one new blank quote + @workshop_log.quotes.build if @workshop_log.quotes.empty? # @sectors = Sector.published.map{ |si| [ si.id, si.name ] } # @files = MediaFile.where(["workshop_log_id = ?", @workshop_log.id]) @@ -199,12 +190,7 @@ def workshop_log_params :children_ongoing, :children_first_time, :teens_ongoing, :teens_first_time, :adults_ongoing, :adults_first_time, :owner_id, :owner_type, :user_id, :organization_id, :date, :workshop_name, :workshop_id, :windows_type_id, :other_description, :external_workshop_title, # :user, - quotable_item_quotes_attributes: [ - :id, :quotable_type, :quotable_id, :_destroy, - quote_attributes: [ :id, :quote, :age, :workshop_id, :_destroy ] ], - all_quotable_item_quotes_attributes: [ - :id, :quotable_type, :quotable_id, :_destroy, - quote_attributes: [ :id, :quote, :age, :workshop_id, :_destroy ] ], + quotes_attributes: [ :id, :quote, :age, :speaker_name, :workshop_id, :_destroy ], report_form_field_answers_attributes: [ :id, :form_field_id, :answer_option_id, :answer, :report_id, :_destroy ], gallery_assets_attributes: [ :id, :file, :_destroy ]) diff --git a/app/decorators/quote_decorator.rb b/app/decorators/quote_decorator.rb index a7903aebb..8cce05d79 100644 --- a/app/decorators/quote_decorator.rb +++ b/app/decorators/quote_decorator.rb @@ -9,7 +9,7 @@ def detail(length: nil) end def created_by # TODO - add to model and quote creation - object.quotable_item_quotes.last&.quotable&.decorate&.created_by + object.quotable&.decorate&.created_by end def quote diff --git a/app/models/quote.rb b/app/models/quote.rb index d3601ef70..15b8f32ca 100644 --- a/app/models/quote.rb +++ b/app/models/quote.rb @@ -2,8 +2,8 @@ class Quote < ApplicationRecord include Publishable, TagFilterable, Trendable, WindowsTypeFilterable belongs_to :workshop, optional: true + belongs_to :quotable, polymorphic: true, optional: true has_many :bookmarks, as: :bookmarkable, dependent: :destroy - has_many :quotable_item_quotes, dependent: :destroy has_many :categorizable_items, dependent: :destroy, inverse_of: :categorizable, as: :categorizable has_many :sectorable_items, dependent: :destroy, inverse_of: :sectorable, as: :sectorable # Asset associations diff --git a/app/models/report.rb b/app/models/report.rb index 98e76dc25..21c07227d 100644 --- a/app/models/report.rb +++ b/app/models/report.rb @@ -7,7 +7,7 @@ class Report < ApplicationRecord has_one :form, as: :owner has_many :bookmarks, as: :bookmarkable, dependent: :destroy has_many :notifications, as: :noticeable, dependent: :destroy - has_many :quotable_item_quotes, as: :quotable, dependent: :nullify, inverse_of: :quotable + has_many :quotes, as: :quotable, dependent: :destroy has_many :report_form_field_answers, foreign_key: :report_id, inverse_of: :report, dependent: :destroy @@ -25,20 +25,13 @@ class Report < ApplicationRecord # has_many through has_many :form_fields, through: :form - has_many :all_quotable_item_quotes, - ->(wl) { where(quotable_id: wl.id, - quotable_type: %w[WorkshopLog Report]) }, # needed bc some are stored w type Report - class_name: "QuotableItemQuote", - inverse_of: :quotable - has_many :quotes, through: :all_quotable_item_quotes, dependent: :nullify has_many :sectors, through: :sectorable_items, dependent: :destroy # Nested attributes accepts_nested_attributes_for :media_files, allow_destroy: true, reject_if: :all_blank accepts_nested_attributes_for :primary_asset, allow_destroy: true, reject_if: :all_blank accepts_nested_attributes_for :gallery_assets, allow_destroy: true, reject_if: :all_blank - accepts_nested_attributes_for :all_quotable_item_quotes, allow_destroy: true, reject_if: :all_blank - accepts_nested_attributes_for :quotable_item_quotes, allow_destroy: true, reject_if: :all_blank + accepts_nested_attributes_for :quotes, allow_destroy: true, reject_if: :all_blank accepts_nested_attributes_for :report_form_field_answers, reject_if: proc { |object| object["_create"].to_i == 0 && object["answer"].nil? } diff --git a/app/models/workshop.rb b/app/models/workshop.rb index 106eb5975..16b7f9bb7 100644 --- a/app/models/workshop.rb +++ b/app/models/workshop.rb @@ -27,7 +27,7 @@ def self.mentionable_rich_text_fields has_many :bookmarks, as: :bookmarkable, dependent: :destroy has_many :categorizable_items, dependent: :destroy, inverse_of: :categorizable, as: :categorizable - has_many :quotable_item_quotes, as: :quotable, dependent: :destroy + has_many :quotes, as: :quotable, dependent: :destroy has_many :associated_resources, class_name: "Resource", foreign_key: "workshop_id", dependent: :restrict_with_error has_many :sectorable_items, dependent: :destroy, inverse_of: :sectorable, as: :sectorable has_many :workshop_logs, dependent: :destroy, as: :owner @@ -51,7 +51,6 @@ def self.mentionable_rich_text_fields has_many :categories, through: :categorizable_items has_many :category_types, through: :categories has_many :organizations, through: :user - has_many :quotes, through: :quotable_item_quotes has_many :resources, through: :workshop_resources, source: :resource has_many :sectors, through: :sectorable_items diff --git a/app/views/quotes/index.html.erb b/app/views/quotes/index.html.erb index 02200f6e4..9dc81bbc1 100644 --- a/app/views/quotes/index.html.erb +++ b/app/views/quotes/index.html.erb @@ -57,12 +57,10 @@ <%= "@ " + quote.created_at.strftime("%m-%d-%-Y") %> <%= ("re " + link_to(quote.workshop.title, workshop_path(quote.workshop), class: "hover:underline") if quote.workshop).to_s.html_safe %> - <% quote.quotable_item_quotes.each do |qiq| %> - <% if qiq.quotable %> - from: <%= link_to qiq.quotable.title, - polymorphic_path(qiq.quotable), - class: "hover:underline" %> - <% end %> + <% if quote.quotable %> + from: <%= link_to quote.quotable.title, + polymorphic_path(quote.quotable), + class: "hover:underline" %> <% end %> diff --git a/app/views/quotes/show.html.erb b/app/views/quotes/show.html.erb index 6d00355c4..af6badb9b 100644 --- a/app/views/quotes/show.html.erb +++ b/app/views/quotes/show.html.erb @@ -72,11 +72,11 @@ <% end %>
- <% @quote.quotable_item_quotes.each do |qiq| %> + <% if @quote.quotable %>
-

<%= qiq.quotable.class %>

- <%= link_to qiq.quotable.title, - polymorphic_path(qiq.quotable), +

<%= @quote.quotable.class %>

+ <%= link_to @quote.quotable.title, + polymorphic_path(@quote.quotable), class: "btn btn-secondary-outline" %>
<% end %> diff --git a/app/views/workshop_logs/_form.html.erb b/app/views/workshop_logs/_form.html.erb index 21b308925..67fa04a3b 100644 --- a/app/views/workshop_logs/_form.html.erb +++ b/app/views/workshop_logs/_form.html.erb @@ -99,10 +99,10 @@ What did your participants share about their experience? Please write quotes in first person.

- <%= f.simple_fields_for :all_quotable_item_quotes do |qiq_form| %> - <%= render 'quotable_item_quote_fields', f: qiq_form %> + <%= f.simple_fields_for :quotes do |quote_form| %> + <%= render 'quote_fields', f: quote_form %> <% end %> - <%= link_to_add_association "Add Quote", f, :quotable_item_quotes, class: "text-blue-600 hover:underline text-sm" %> + <%= link_to_add_association "Add Quote", f, :quotes, class: "text-blue-600 hover:underline text-sm" %>
<%= render "shared/form_image_fields", f: f, include_primary_asset: false, diff --git a/app/views/workshop_logs/_quote_fields.html.erb b/app/views/workshop_logs/_quote_fields.html.erb new file mode 100644 index 000000000..0bd295020 --- /dev/null +++ b/app/views/workshop_logs/_quote_fields.html.erb @@ -0,0 +1,35 @@ +
+ <%= f.input :workshop_id, as: :hidden, input_html: { value: @workshop&.id || @workshop_log&.workshop_id } %> + +
+ +
+ <%= f.input :quote, + label: "Quote", + placeholder: "Ex. 'Coming here makes me feel happy!'", + wrapper: false, + input_html: { + rows: 3, + class: "w-full rounded-md border-gray-300 shadow-sm focus:ring-blue-500 focus:border-blue-500" + }, + label_html: { class: "block text-sm font-medium text-gray-700 mb-1" } %> +
+
+ <%= f.input :age, + label: "Participant age", + hint: "If unknown, enter: child, teen, or adult.", + wrapper: false, + input_html: { + class: "w-full rounded-md border-gray-300 shadow-sm focus:ring-blue-500 focus:border-blue-500" + }, + label_html: { class: "block text-sm font-medium text-gray-700 mb-1 whitespace-nowrap" } %> +
+
+ + +
+ <%= link_to_remove_association "Delete", + f, + class: "text-sm text-red-600 hover:text-red-800 font-medium underline" %> +
+
diff --git a/db/migrate/20260215071524_add_quotable_to_quotes.rb b/db/migrate/20260215071524_add_quotable_to_quotes.rb new file mode 100644 index 000000000..91ee2d8bc --- /dev/null +++ b/db/migrate/20260215071524_add_quotable_to_quotes.rb @@ -0,0 +1,5 @@ +class AddQuotableToQuotes < ActiveRecord::Migration[8.1] + def change + add_reference :quotes, :quotable, polymorphic: true, index: true + end +end diff --git a/db/migrate/20260215071525_migrate_quotable_item_quotes_to_quotes.rb b/db/migrate/20260215071525_migrate_quotable_item_quotes_to_quotes.rb new file mode 100644 index 000000000..455d9c1e3 --- /dev/null +++ b/db/migrate/20260215071525_migrate_quotable_item_quotes_to_quotes.rb @@ -0,0 +1,22 @@ +class MigrateQuotableItemQuotesToQuotes < ActiveRecord::Migration[8.1] + def up + # Copy quotable associations from quotable_item_quotes to quotes + execute <<-SQL.squish + UPDATE quotes + INNER JOIN quotable_item_quotes ON quotes.id = quotable_item_quotes.quote_id + SET quotes.quotable_id = quotable_item_quotes.quotable_id, + quotes.quotable_type = quotable_item_quotes.quotable_type + SQL + end + + def down + # Rollback: clear quotable associations from quotes + # Note: This doesn't restore the quotable_item_quotes records + # as they should still exist for rollback purposes + execute <<-SQL.squish + UPDATE quotes + SET quotes.quotable_id = NULL, + quotes.quotable_type = NULL + SQL + end +end diff --git a/spec/models/quote_spec.rb b/spec/models/quote_spec.rb index aebba119f..6d133790b 100644 --- a/spec/models/quote_spec.rb +++ b/spec/models/quote_spec.rb @@ -4,10 +4,8 @@ # pending "add some examples to (or delete) #{__FILE__}" describe 'associations' do - # it { should have_many(:quotable_item_quotes) } # Model missing has_many - # Through associations can sometimes be tricky, test if needed - # it { should have_many(:reports).through(:quotable_item_quotes).source(:quotable).source_type('Report') } - # it { should have_many(:workshops).through(:quotable_item_quotes).source(:quotable).source_type('Workshop') } + it { should belong_to(:quotable).optional } + it { should belong_to(:workshop).optional } end describe 'validations' do diff --git a/spec/models/report_spec.rb b/spec/models/report_spec.rb index df5af7b71..cf69633d3 100644 --- a/spec/models/report_spec.rb +++ b/spec/models/report_spec.rb @@ -11,8 +11,7 @@ it { should have_many(:gallery_assets) } it { should have_many(:form_fields).through(:form) } it { should have_many(:report_form_field_answers).dependent(:destroy) } - it { should have_many(:quotable_item_quotes).dependent(:nullify) } - it { should have_many(:quotes).through(:all_quotable_item_quotes).dependent(:nullify) } + it { should have_many(:quotes).dependent(:destroy) } it { should have_many(:notifications).dependent(:destroy) } it { should have_many(:sectorable_items).dependent(:destroy) } it { should have_many(:sectors).through(:sectorable_items).dependent(:destroy) } @@ -20,7 +19,7 @@ it { should accept_nested_attributes_for(:media_files) } it { should accept_nested_attributes_for(:report_form_field_answers) } - it { should accept_nested_attributes_for(:quotable_item_quotes) } + it { should accept_nested_attributes_for(:quotes) } end describe "validations" do diff --git a/spec/models/workshop_spec.rb b/spec/models/workshop_spec.rb index 3dd86f3c7..28e410544 100644 --- a/spec/models/workshop_spec.rb +++ b/spec/models/workshop_spec.rb @@ -19,8 +19,7 @@ it { should have_many(:categorizable_items).dependent(:destroy) } # As categorizable it { should have_many(:categories).through(:categorizable_items) } it { should have_many(:category_types).through(:categories) } - it { should have_many(:quotable_item_quotes).dependent(:destroy) } # As quotable - it { should have_many(:quotes).through(:quotable_item_quotes) } + it { should have_many(:quotes).dependent(:destroy) } # As quotable it { should have_many(:workshop_resources).dependent(:destroy) } it { should have_many(:resources).through(:workshop_resources) } it { should have_many(:age_ranges) }