From 9a7681b75fc77db71fdf6d443f52599043f18aaa Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Javi=20Mart=C3=ADn?= Date: Fri, 14 Mar 2025 15:43:18 +0100 Subject: [PATCH] Don't hide records during a system test As mentioned in commits like a586ba806, a7664ad81, 006128da5, b41fbfa52 and c480cdd91, accessing the database after starting the browser with the `visit` method sometimes results in database corruption and failing tests on our CI due to the process running the test accessing the database after the process running the browser has started. In this case, we were hiding a proposal after starting the process running the browser to check what happens when accessing a notification for a hidden proposal. We can avoid database access in the middle of the test by hidding a proposal before starting the browser. The process to create a notification using the browser is already tested in other specs, so we don't need to do it here as well. Note that, to simplify the test, we're extracting the `notify_users` method. I wonder whether this method should be called in an `after_create` callback instead... That's a topic for another time, though. --- .../proposal_notifications_controller.rb | 5 ++--- app/models/proposal.rb | 6 ++++++ spec/system/proposal_notifications_spec.rb | 17 +++-------------- 3 files changed, 11 insertions(+), 17 deletions(-) diff --git a/app/controllers/proposal_notifications_controller.rb b/app/controllers/proposal_notifications_controller.rb index 96ca04a75..0a2128435 100644 --- a/app/controllers/proposal_notifications_controller.rb +++ b/app/controllers/proposal_notifications_controller.rb @@ -11,9 +11,8 @@ class ProposalNotificationsController < ApplicationController @notification = ProposalNotification.new(proposal_notification_params) @proposal = Proposal.find(proposal_notification_params[:proposal_id]) if @notification.save - @proposal.users_to_notify.each do |user| - Notification.add(user, @notification) - end + @proposal.notify_users(@notification) + redirect_to @notification, notice: I18n.t("flash.actions.create.proposal_notification") else render :new diff --git a/app/models/proposal.rb b/app/models/proposal.rb index 6cdcc5979..ef4bcbfb5 100644 --- a/app/models/proposal.rb +++ b/app/models/proposal.rb @@ -232,6 +232,12 @@ class Proposal < ApplicationRecord followers - [author] end + def notify_users(notification) + users_to_notify.each do |user| + Notification.add(user, notification) + end + end + def self.proposals_orders(user) orders = %w[hot_score confidence_score created_at relevance archival_date] diff --git a/spec/system/proposal_notifications_spec.rb b/spec/system/proposal_notifications_spec.rb index 057d5a425..dff45c273 100644 --- a/spec/system/proposal_notifications_spec.rb +++ b/spec/system/proposal_notifications_spec.rb @@ -280,25 +280,14 @@ describe "Proposal Notifications" do expect(page).to have_css ".notification", count: 0 end - scenario "Proposal hidden" do + scenario "Hidden proposal" do author = create(:user) user = create(:user) proposal = create(:proposal, author: author, voters: [user], followers: [user]) - - login_as(author) - - visit new_proposal_notification_path(proposal_id: proposal.id) - - fill_in "proposal_notification_title", with: "Thank you for supporting my proposal" - fill_in "proposal_notification_body", with: "Please share it with " \ - "others so we can make it happen!" - click_button "Send notification" - - expect(page).to have_content "Your message has been sent correctly." - + notification = create(:proposal_notification, author: author, proposal: proposal) + proposal.notify_users(notification) proposal.hide - logout login_as user visit root_path