diff --git a/app/models/article.rb b/app/models/article.rb index e6de3937f..241fbde41 100644 --- a/app/models/article.rb +++ b/app/models/article.rb @@ -19,12 +19,13 @@ class Article < ApplicationRecord delegate :name, to: :user, prefix: true delegate :username, to: :user, prefix: true - belongs_to :user - belongs_to :organization, optional: true # touch: true was removed because when an article is updated, the associated collection # is touched along with all its articles(including this one). This causes eventually a deadlock. belongs_to :collection, optional: true + belongs_to :organization, optional: true + belongs_to :user + counter_culture :user counter_culture :organization diff --git a/app/models/email_message.rb b/app/models/email_message.rb index 6c730dafd..ea00da3a4 100644 --- a/app/models/email_message.rb +++ b/app/models/email_message.rb @@ -1,4 +1,6 @@ class EmailMessage < Ahoy::Message + belongs_to :feedback_message, optional: true + # So far this is mostly used to be compatible with administrate gem, # which doesn't seem to play nicely with namespaces. But there could be other # reasons to define behavior here, similar to how we use the Tag model. diff --git a/app/models/feedback_message.rb b/app/models/feedback_message.rb index 70d3764d5..cfed8d557 100644 --- a/app/models/feedback_message.rb +++ b/app/models/feedback_message.rb @@ -4,6 +4,8 @@ class FeedbackMessage < ApplicationRecord belongs_to :offender, class_name: "User", optional: true, inverse_of: :offender_feedback_messages belongs_to :reporter, class_name: "User", optional: true, inverse_of: :reporter_feedback_messages belongs_to :affected, class_name: "User", optional: true, inverse_of: :affected_feedback_messages + + has_one :email_message, dependent: :nullify has_many :notes, as: :noteable, inverse_of: :noteable, dependent: :destroy validates :feedback_type, :message, presence: true diff --git a/db/migrate/20200914143753_add_missing_foreign_keys_to_ahoy_messages.rb b/db/migrate/20200914143753_add_missing_foreign_keys_to_ahoy_messages.rb new file mode 100644 index 000000000..eaeb30479 --- /dev/null +++ b/db/migrate/20200914143753_add_missing_foreign_keys_to_ahoy_messages.rb @@ -0,0 +1,5 @@ +class AddMissingForeignKeysToAhoyMessages < ActiveRecord::Migration[6.0] + def change + add_foreign_key :ahoy_messages, :feedback_messages, on_delete: :nullify, validate: false + end +end diff --git a/db/migrate/20200914144033_validate_add_missing_foreign_keys_to_ahoy_messages.rb b/db/migrate/20200914144033_validate_add_missing_foreign_keys_to_ahoy_messages.rb new file mode 100644 index 000000000..95a1fa422 --- /dev/null +++ b/db/migrate/20200914144033_validate_add_missing_foreign_keys_to_ahoy_messages.rb @@ -0,0 +1,5 @@ +class ValidateAddMissingForeignKeysToAhoyMessages < ActiveRecord::Migration[6.0] + def change + validate_foreign_key :ahoy_messages, :feedback_messages + end +end diff --git a/db/migrate/20200914144157_add_missing_foreign_keys_to_articles.rb b/db/migrate/20200914144157_add_missing_foreign_keys_to_articles.rb new file mode 100644 index 000000000..c2536e40b --- /dev/null +++ b/db/migrate/20200914144157_add_missing_foreign_keys_to_articles.rb @@ -0,0 +1,5 @@ +class AddMissingForeignKeysToArticles < ActiveRecord::Migration[6.0] + def change + add_foreign_key :articles, :collections, on_delete: :nullify, validate: false + end +end diff --git a/db/migrate/20200914145500_validate_add_missing_foreign_keys_to_articles.rb b/db/migrate/20200914145500_validate_add_missing_foreign_keys_to_articles.rb new file mode 100644 index 000000000..d32c3d560 --- /dev/null +++ b/db/migrate/20200914145500_validate_add_missing_foreign_keys_to_articles.rb @@ -0,0 +1,5 @@ +class ValidateAddMissingForeignKeysToArticles < ActiveRecord::Migration[6.0] + def change + validate_foreign_key :articles, :collections + end +end diff --git a/db/schema.rb b/db/schema.rb index 63a98dfcb..b9498d84e 100644 --- a/db/schema.rb +++ b/db/schema.rb @@ -10,7 +10,7 @@ # # It's strongly recommended that you check this file into your version control system. -ActiveRecord::Schema.define(version: 2020_09_11_140318) do +ActiveRecord::Schema.define(version: 2020_09_14_145500) do # These are extensions that must be enabled in order to support this database enable_extension "citext" @@ -1379,9 +1379,11 @@ ActiveRecord::Schema.define(version: 2020_09_11_140318) do add_foreign_key "ahoy_events", "ahoy_visits", column: "visit_id", on_delete: :cascade add_foreign_key "ahoy_events", "users", on_delete: :cascade + add_foreign_key "ahoy_messages", "feedback_messages", on_delete: :nullify add_foreign_key "ahoy_messages", "users", on_delete: :cascade add_foreign_key "ahoy_visits", "users", on_delete: :cascade add_foreign_key "api_secrets", "users", on_delete: :cascade + add_foreign_key "articles", "collections", on_delete: :nullify add_foreign_key "articles", "organizations", on_delete: :nullify add_foreign_key "articles", "users", on_delete: :cascade add_foreign_key "audit_logs", "users" diff --git a/spec/factories/email_messages.rb b/spec/factories/email_messages.rb new file mode 100644 index 000000000..fed9a0f36 --- /dev/null +++ b/spec/factories/email_messages.rb @@ -0,0 +1,8 @@ +FactoryBot.define do + factory :email_message do + to { Faker::Internet.email } + subject { Faker::Lorem.sentence } + + content { Faker::Lorem.paragraph } + end +end diff --git a/spec/models/article_spec.rb b/spec/models/article_spec.rb index 18b1edadf..9ea40286d 100644 --- a/spec/models/article_spec.rb +++ b/spec/models/article_spec.rb @@ -14,9 +14,9 @@ RSpec.describe Article, type: :model do it_behaves_like "UserSubscriptionSourceable" describe "validations" do - it { is_expected.to belong_to(:user) } - it { is_expected.to belong_to(:organization).optional } it { is_expected.to belong_to(:collection).optional } + it { is_expected.to belong_to(:organization).optional } + it { is_expected.to belong_to(:user) } it { is_expected.to have_many(:buffer_updates).dependent(:destroy) } it { is_expected.to have_many(:comments).dependent(:nullify) } diff --git a/spec/models/email_message_spec.rb b/spec/models/email_message_spec.rb new file mode 100644 index 000000000..eeda19949 --- /dev/null +++ b/spec/models/email_message_spec.rb @@ -0,0 +1,9 @@ +require "rails_helper" + +RSpec.describe EmailMessage, type: :model do + describe "validations" do + subject { create(:email_message) } + + it { is_expected.to belong_to(:feedback_message).optional } + end +end diff --git a/spec/models/feedback_message_spec.rb b/spec/models/feedback_message_spec.rb index f128f63bb..f569d68e3 100644 --- a/spec/models/feedback_message_spec.rb +++ b/spec/models/feedback_message_spec.rb @@ -3,6 +3,8 @@ require "rails_helper" RSpec.describe FeedbackMessage, type: :model do subject(:feedback_message) { create(:feedback_message) } + it { is_expected.to have_one(:email_message).dependent(:nullify).optional } + it { is_expected.to validate_presence_of(:feedback_type) } it { is_expected.to validate_presence_of(:message) } it { is_expected.to validate_length_of(:reported_url).is_at_most(250) } diff --git a/spec/requests/admin/articles_spec.rb b/spec/requests/admin/articles_spec.rb index 42ab5c8e1..ce78b0a04 100644 --- a/spec/requests/admin/articles_spec.rb +++ b/spec/requests/admin/articles_spec.rb @@ -9,21 +9,28 @@ RSpec.describe "/admin/articles", type: :request do context "when updating an Article via /admin/articles" do let(:super_admin) { create(:user, :super_admin) } let(:article) { create(:article) } + let(:second_user) { create(:user) } + let(:third_user) { create(:user) } before { sign_in super_admin } it "allows an Admin to add a co-author to an individual article" do get request + expect do - article.update_columns(second_user_id: 1) - end.to change(article, :second_user_id).from(nil).to(1) + article.update_columns(second_user_id: second_user.id) + end.to change(article, :second_user_id).from(nil).to(second_user.id) end - it "allows an Admin to add co-authora to an individual article" do - article.update_columns(second_user_id: 2, third_user_id: 3) + it "allows an Admin to add multiple co-authors to an individual article" do + article.update_columns(second_user_id: second_user.id, third_user_id: third_user.id) + get request - expect(article.second_user_id).to eq(2) - expect(article.third_user_id).to eq(3) + + article.reload + + expect(article.second_user_id).to eq(second_user.id) + expect(article.third_user_id).to eq(third_user.id) end end end diff --git a/spec/services/users/delete_spec.rb b/spec/services/users/delete_spec.rb index 180b25216..060e50193 100644 --- a/spec/services/users/delete_spec.rb +++ b/spec/services/users/delete_spec.rb @@ -163,7 +163,7 @@ RSpec.describe Users::Delete, type: :service do described_class.call(user) aggregate_failures "associations should not exist" do user_associations.each do |user_association| - expect { user_association.reload }.to raise_error(ActiveRecord::RecordNotFound) + expect { user_association.reload }.to raise_error(ActiveRecord::RecordNotFound), user_association end end end