Skip to content

Commit

Permalink
Prevent notifications for blocked users (decidim#13689)
Browse files Browse the repository at this point in the history
* Block notifications for blocked users

* Add tests for fix_blocked_user_notification task

* Remove trailing whitespaces

* Update RELEASE_NOTES.md

* Update decidim-core/lib/tasks/upgrade/clean.rake

* RELEASE_NOTES.md lint fix

* Remove line 24 from RELEASE_NOTES.md

* Apply suggestions from code review

Co-authored-by: Andrés Pereira de Lucena <[email protected]>

---------

Co-authored-by: Alexandru Emil Lupu <[email protected]>
Co-authored-by: Andrés Pereira de Lucena <[email protected]>
  • Loading branch information
3 people authored Jan 7, 2025
1 parent f2f89e2 commit 7ac8c00
Show file tree
Hide file tree
Showing 6 changed files with 52 additions and 1 deletion.
1 change: 1 addition & 0 deletions decidim-admin/app/commands/decidim/admin/block_user.rb
Original file line number Diff line number Diff line change
Expand Up @@ -77,6 +77,7 @@ def block!
form.user.block_id = @current_blocking.id
form.user.extended_data["user_name"] = form.user.name
form.user.name = "Blocked user"
form.user.notifications_sending_frequency = "none"
form.user.save!
end
end
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,7 @@ module Decidim::Admin
expect(form.user.blocked).to be(true)
expect(form.user.extended_data["user_name"]).to eq(user_name)
expect(form.user.name).to eq("Blocked user")
expect(form.user.notifications_sending_frequency).to eq("none")
end

it "original username is stored in the action log entry's resource title" do
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -38,12 +38,14 @@ def generate

followers.each do |recipient|
next unless ["all", "followed-only"].include?(recipient.notification_types)
next if recipient.blocked?

send_email_to(recipient, user_role: :follower)
end

affected_users.each do |recipient|
next unless ["all", "own-only"].include?(recipient.notification_types)
next if recipient.blocked?

send_email_to(recipient, user_role: :affected_user)
end
Expand Down
16 changes: 15 additions & 1 deletion decidim-core/lib/tasks/upgrade/clean.rake
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,8 @@ namespace :decidim do
:"decidim:upgrade:clean:follows",
:"decidim:upgrade:clean:categories",
:"decidim:upgrade:clean:action_logs",
:"decidim:upgrade:clean:clean_deleted_users"
:"decidim:upgrade:clean:clean_deleted_users",
:"decidim:upgrade:clean:fix_blocked_user_notification"
]

desc "Remove data from deleted users"
Expand Down Expand Up @@ -126,6 +127,19 @@ namespace :decidim do
logger.info("===== Deleted #{invalid} invalid resources")
end

desc "Update all blocked users notifications_sending_frequency setting"
task fix_blocked_user_notification: :environment do
logger.info("=== Updating all blocked users notifications_sending_frequency ...")
blocked_users = 0
Decidim::User.blocked.where.not("notifications_sending_frequency = ?", "none").find_each do |blocked_user|
unless blocked_user.notifications_sending_frequency == "none"
blocked_user.update(notifications_sending_frequency: "none")
blocked_users += 1
end
end
logger.info("===== Updated #{blocked_users} blocked users")
end

def logger
@logger ||= Logger.new($stdout)
end
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -105,6 +105,15 @@

it_behaves_like "does not enqueue the job"
end

context "when the user is blocked" do
before do
recipient.update!(blocked: true, blocked_at: Time.now.utc)
follower.update!(blocked: true, blocked_at: Time.now.utc)
end

it_behaves_like "does not enqueue the job"
end
end
end

Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,24 @@
# frozen_string_literal: true

require "spec_helper"

describe "rake decidim:upgrade:clean:fix_blocked_user_notification", type: :task do
context "when executing task" do
it "does not throw exceptions keys" do
expect do
Rake::Task[:"decidim:upgrade:clean:fix_blocked_user_notification"].invoke
end.not_to raise_exception
end
end

context "when there are blocked users" do
let!(:organization) { create(:organization) }
let!(:users) { create_list(:user, 4, :blocked, organization:) }

it "update all blocked users" do
expect(Decidim::User.blocked.where.not("notifications_sending_frequency = ?", "none").count).to eq(4)
expect { task.execute }.not_to raise_error
expect(Decidim::User.blocked.where.not("notifications_sending_frequency = ?", "none").count).to eq(0)
end
end
end

0 comments on commit 7ac8c00

Please sign in to comment.