diff --git a/Gemfile.lock b/Gemfile.lock index 696d40d..cccf153 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -174,7 +174,7 @@ GIT PATH remote: . specs: - decidim-cleaner (4.0.0) + decidim-cleaner (4.1.0) decidim-core (~> 0.28.dev) GEM diff --git a/README.md b/README.md index d850cf4..5520aff 100644 --- a/README.md +++ b/README.md @@ -22,6 +22,19 @@ bundle exec rails decidim_cleaner:install:migrations bundle exec rails db:migrate ``` +You can then modify the default values of the cleaner in your .ENV with the following variables: + +```bash +# Delay until a user is considered inactive and receive a warning email (in days) +DECIDIM_CLEANER_INACTIVE_USERS_MAIL= + +# Delay until a user is deleted after receiving an email (in days) +DECIDIM_CLEANER_DELETE_INACTIVE_USERS= + +# Delay until an admin log is deleted (in days) +DECIDIM_CLEANER_DELETE_ADMIN_LOGS= +``` + ### Sidekiq Scheduler [Further documentation](https://github.com/sidekiq-scheduler/sidekiq-scheduler) diff --git a/app/helpers/decidim/cleaner/delays_helper.rb b/app/helpers/decidim/cleaner/delays_helper.rb new file mode 100644 index 0000000..c440c7e --- /dev/null +++ b/app/helpers/decidim/cleaner/delays_helper.rb @@ -0,0 +1,15 @@ +# frozen_string_literal: true + +module Decidim + module Cleaner + module DelaysHelper + def email_inactive_after(organization) + organization.delete_inactive_users_email_after || Decidim::Cleaner.delete_inactive_users_email_after + end + + def delete_inactive_after(organization) + organization.delete_inactive_users_after || Decidim::Cleaner.delete_inactive_users_after + end + end + end +end diff --git a/app/jobs/decidim/cleaner/clean_admin_logs_job.rb b/app/jobs/decidim/cleaner/clean_admin_logs_job.rb index b31b63f..23a88f1 100644 --- a/app/jobs/decidim/cleaner/clean_admin_logs_job.rb +++ b/app/jobs/decidim/cleaner/clean_admin_logs_job.rb @@ -10,10 +10,16 @@ def perform next unless organization.delete_admin_logs? Decidim::ActionLog.where(organization:) - .where("created_at < ?", Time.zone.now - (organization.delete_admin_logs_after || 365).days) + .where("created_at < ?", delete_admin_logs_before_date(organization)) .delete_all end end + + private + + def delete_admin_logs_before_date(organization) + Time.zone.now - (organization.delete_admin_logs_after || Decidim::Cleaner.delete_admin_logs_after).days + end end end end diff --git a/app/jobs/decidim/cleaner/clean_inactive_users_job.rb b/app/jobs/decidim/cleaner/clean_inactive_users_job.rb index 7381bf6..7ffef48 100644 --- a/app/jobs/decidim/cleaner/clean_inactive_users_job.rb +++ b/app/jobs/decidim/cleaner/clean_inactive_users_job.rb @@ -12,25 +12,31 @@ def perform send_warning(Decidim::User.where(organization:) .not_deleted .where.not(email: "") - .where("last_sign_in_at < ?", Time.zone.now - (organization.delete_inactive_users_email_after || 365).days) - .where("last_sign_in_at > ?", Time.zone.now - (organization.delete_inactive_users_email_after || 365).days - 1.day)) - + .where("last_sign_in_at < ?", email_inactive_before_date(organization))) delete_user_and_send_email(Decidim::User.where(organization:) .not_deleted .where.not(email: "") - .where("last_sign_in_at < ?", Time.zone.now - (organization.delete_inactive_users_after || 390).days)) + .where("warning_date < ?", delete_inactive_before_date(organization))) end end def send_warning(users) users.find_each do |user| - InactiveUsersMailer.warning_inactive(user).deliver_now + next if user.warning_date.present? + + user.update!(warning_date: Time.zone.now) if InactiveUsersMailer.warning_inactive(user).deliver_now Rails.logger.info "Inactive warning sent to #{user.email}" end end def delete_user_and_send_email(users) users.find_each do |user| + if user.last_sign_in_at > user.warning_date + user.update!(warning_date: nil) + Rails.logger.info "User with id #{user.id} has logged in again, warning date reset" + next + end + InactiveUsersMailer.warning_deletion(user).deliver_now Rails.logger.info "Deletion warning sent to #{user.email}" @@ -38,6 +44,16 @@ def delete_user_and_send_email(users) Rails.logger.info "User with id #{user.id} destroyed" end end + + private + + def email_inactive_before_date(organization) + Time.zone.now - (organization.delete_inactive_users_email_after || Decidim::Cleaner.delete_inactive_users_email_after).days + end + + def delete_inactive_before_date(organization) + Time.zone.now - (organization.delete_inactive_users_after || Decidim::Cleaner.delete_inactive_users_after).days + end end end end diff --git a/app/mailers/decidim/cleaner/inactive_users_mailer.rb b/app/mailers/decidim/cleaner/inactive_users_mailer.rb index b694eb9..a279334 100644 --- a/app/mailers/decidim/cleaner/inactive_users_mailer.rb +++ b/app/mailers/decidim/cleaner/inactive_users_mailer.rb @@ -5,6 +5,7 @@ module Cleaner # A custom mailer for Decidim so we can notify users # when their account was blocked class InactiveUsersMailer < Decidim::ApplicationMailer + helper Decidim::Cleaner::DelaysHelper def warning_inactive(user) with_user(user) do @user = user diff --git a/app/views/decidim/cleaner/inactive_users_mailer/warning_deletion.html.erb b/app/views/decidim/cleaner/inactive_users_mailer/warning_deletion.html.erb index 54cfea9..02ebead 100644 --- a/app/views/decidim/cleaner/inactive_users_mailer/warning_deletion.html.erb +++ b/app/views/decidim/cleaner/inactive_users_mailer/warning_deletion.html.erb @@ -1,6 +1,6 @@
<%= t ".hello" %>
-<%= t(".body_1", organization_name: h(@organization.name), organization_url: decidim.root_url(host: @organization.host), days: @organization.delete_inactive_users_after||390).html_safe %>
+<%= t(".body_1", organization_name: h(@organization.name), organization_url: decidim.root_url(host: @organization.host), days: email_inactive_after(@organization) + delete_inactive_after(@organization)).html_safe %>
<%= t ".body_2" %>
diff --git a/app/views/decidim/cleaner/inactive_users_mailer/warning_inactive.html.erb b/app/views/decidim/cleaner/inactive_users_mailer/warning_inactive.html.erb index d57101e..41674f7 100644 --- a/app/views/decidim/cleaner/inactive_users_mailer/warning_inactive.html.erb +++ b/app/views/decidim/cleaner/inactive_users_mailer/warning_inactive.html.erb @@ -1,7 +1,7 @@<%= t ".hello" %>
-<%= t ".body_1", organization_name: h(@organization.name), days: @organization.delete_inactive_users_email_after %>
+<%= t ".body_1", organization_name: h(@organization.name), days: email_inactive_after(@organization) %>
-<%= t(".body_2", remaining_days: (@organization.delete_inactive_users_after||390) - (@organization.delete_inactive_users_email_after||365), organization_url: decidim.root_url(host: @organization.host)).html_safe %>
+<%= t(".body_2", remaining_days: delete_inactive_after(@organization), organization_url: decidim.root_url(host: @organization.host)).html_safe %>
<%= t(".greetings", organization_name: h(@organization.name), organization_url: decidim.root_url(host: @organization.host)).html_safe %>
diff --git a/config/locales/en.yml b/config/locales/en.yml index 26983b1..fd985c3 100644 --- a/config/locales/en.yml +++ b/config/locales/en.yml @@ -6,7 +6,8 @@ en: delete_admin_logs: Enable admin logs deletion delete_admin_logs_after: Delete admin logs after (days, default 365) delete_inactive_users: Enable inactive users deletion - delete_inactive_users_after: Delete inactive users after (days, default 390) + delete_inactive_users_after: Delete inactive users x days after the mail(days, + default 30) delete_inactive_users_email_after: Send email to inactive users before deletion (days, default 365) decidim: diff --git a/config/locales/fr.yml b/config/locales/fr.yml index b041268..100af2d 100644 --- a/config/locales/fr.yml +++ b/config/locales/fr.yml @@ -7,8 +7,8 @@ fr: delete_admin_logs_after: Supprimer l'historique d'administration après (jours, par défaut 365) delete_inactive_users: Activer la suppression des utilisateurs inactifs - delete_inactive_users_after: Supprimer les utilisateurs inactifs après (jours, - par défaut 390) + delete_inactive_users_after: Supprimer les utilisateurs inactifs au bout de + x jours après le courriel (jours, par défaut 30) delete_inactive_users_email_after: Envoyer un courriel aux utilisateurs inactifs avant la suppression (jours, par défaut 365) decidim: diff --git a/db/migrate/20230328094652_add_warning_date_to_users.rb b/db/migrate/20230328094652_add_warning_date_to_users.rb new file mode 100644 index 0000000..4264114 --- /dev/null +++ b/db/migrate/20230328094652_add_warning_date_to_users.rb @@ -0,0 +1,7 @@ +# frozen_string_literal: true + +class AddWarningDateToUsers < ActiveRecord::Migration[6.1] + def change + add_column :decidim_users, :warning_date, :datetime + end +end diff --git a/lib/decidim/cleaner.rb b/lib/decidim/cleaner.rb index 25bcdfa..16470aa 100644 --- a/lib/decidim/cleaner.rb +++ b/lib/decidim/cleaner.rb @@ -7,5 +7,18 @@ module Decidim # This namespace holds the logic of the `Cleaner` module. module Cleaner + include ActiveSupport::Configurable + + config_accessor :delete_admin_logs_after do + ENV.fetch("DECIDIM_CLEANER_DELETE_ADMIN_LOGS", "365").to_i + end + + config_accessor :delete_inactive_users_after do + ENV.fetch("DECIDIM_CLEANER_DELETE_INACTIVE_USERS", "30").to_i + end + + config_accessor :delete_inactive_users_email_after do + ENV.fetch("DECIDIM_CLEANER_INACTIVE_USERS_MAIL", "365").to_i + end end end diff --git a/lib/decidim/cleaner/version.rb b/lib/decidim/cleaner/version.rb index c576c07..e86efb1 100644 --- a/lib/decidim/cleaner/version.rb +++ b/lib/decidim/cleaner/version.rb @@ -5,7 +5,7 @@ module Decidim # This holds the decidim-meetings version. module Cleaner def self.version - "4.0.0" + "4.1.0" end def self.compatible_decidim_version diff --git a/spec/jobs/decidim/cleaner/clean_inactive_users_job_spec.rb b/spec/jobs/decidim/cleaner/clean_inactive_users_job_spec.rb index 9d27602..161b9c7 100644 --- a/spec/jobs/decidim/cleaner/clean_inactive_users_job_spec.rb +++ b/spec/jobs/decidim/cleaner/clean_inactive_users_job_spec.rb @@ -6,9 +6,9 @@ subject { described_class } context "when the delay is specified" do - let!(:organization) { create(:organization, delete_inactive_users: true, delete_inactive_users_email_after: 25, delete_inactive_users_after: 30) } - let!(:pending_user) { create(:user, organization:, last_sign_in_at: 25.days.ago - 1.hour) } - let!(:inactive_user) { create(:user, organization:, last_sign_in_at: 50.days.ago) } + let!(:organization) { create(:organization, delete_inactive_users: true, delete_inactive_users_email_after: 25, delete_inactive_users_after: 5) } + let!(:pending_user) { create(:user, organization:, last_sign_in_at: 27.days.ago) } + let!(:inactive_user) { create(:user, organization:, last_sign_in_at: 35.days.ago, warning_date: 10.days.ago) } let!(:user) { create(:user, organization:) } it "enqueues job in queue 'cleaner'" do @@ -32,8 +32,8 @@ end context "when users have destroyed his/her account" do - let!(:pending_user) { create(:user, :deleted, organization:, last_sign_in_at: 25.days.ago - 1.hour) } - let!(:inactive_user) { create(:user, :deleted, organization:, last_sign_in_at: 50.days.ago) } + let!(:pending_user) { create(:user, :deleted, organization:, last_sign_in_at: 27.days.ago) } + let!(:inactive_user) { create(:user, :deleted, organization:, last_sign_in_at: 35.days.ago, warning_date: 10.days.ago) } it "doesn't send email" do expect(Decidim::Cleaner::InactiveUsersMailer).not_to receive(:warning_inactive).with(pending_user).and_call_original @@ -51,5 +51,27 @@ expect(user.reload).not_to be_deleted end end + + context "when user reconnect after warning" do + let!(:inactive_user) { create(:user, organization:, last_sign_in_at: 7.days.ago, warning_date: 10.days.ago) } + + it "doesn't send email" do + expect(Decidim::Cleaner::InactiveUsersMailer).not_to receive(:warning_deletion).with(inactive_user).and_call_original + + subject.perform_now + end + + it "doesn't destroy user" do + subject.perform_now + + expect(inactive_user.reload).not_to be_deleted + end + + it "reset warning date" do + subject.perform_now + + expect(inactive_user.reload.warning_date).to be_nil + end + end end end diff --git a/spec/lib/decidim/cleaner/version_spec.rb b/spec/lib/decidim/cleaner/version_spec.rb index abdabeb..264c515 100644 --- a/spec/lib/decidim/cleaner/version_spec.rb +++ b/spec/lib/decidim/cleaner/version_spec.rb @@ -7,7 +7,7 @@ module Decidim subject { described_class } it "has version" do - expect(subject.version).to eq("4.0.0") + expect(subject.version).to eq("4.1.0") end it "has decidim version compatibility" do diff --git a/spec/system/admin_manages_organization_cleaning_spec.rb b/spec/system/admin_manages_organization_cleaning_spec.rb index 262c972..31ea14f 100644 --- a/spec/system/admin_manages_organization_cleaning_spec.rb +++ b/spec/system/admin_manages_organization_cleaning_spec.rb @@ -18,13 +18,13 @@ expect(page).to have_content("Enable admin logs deletion") expect(page).to have_content("Delete admin logs after") expect(page).to have_content("Enable inactive users deletion") - expect(page).to have_content("Delete inactive users after") + expect(page).to have_content("Delete inactive users x days after") expect(page).to have_content("Send email to inactive users before deletion") find(:css, "input[name='organization[delete_admin_logs]'").set(true) fill_in "Delete admin logs after", with: 365 find(:css, "input[name='organization[delete_inactive_users]'").set(true) - fill_in "Delete inactive users after", with: 390 + fill_in "Delete inactive users x days after", with: 30 fill_in "Send email to inactive users before deletion", with: 365 click_button "Update"