After enabling saving to the dashboard, mails will stack up and are never cleaned up. This can cause an the database to be overloaded with old mails. Feature request to add a periodic cron clean up, based on the settings the user can provide.

CommentFileSizeAuthor
#10 mail_safety_cron_cleanup-2894814-10.patch2.3 KBsteven buteneers
#10 interdiff-2894814-8-10.txt374 bytessteven buteneers
#8 interdiff-2894814-3-8.txt2.08 KBsteven buteneers
#8 mail_safety_cron_cleanup-2894814-8.patch2.29 KBsteven buteneers
#6 interdiff-2894814-3-6.txt1.74 KBsteven buteneers
#6 mail_safety_cron_cleanup-2894814-6.patch1.9 KBsteven buteneers
#3 mail_safety_cron_cleanup-2894814-3.patch1.92 KBsteven buteneers

Comments

StevenButeneers created an issue. See original summary.

steven buteneers’s picture

steven buteneers commented
Issue summary: View changes
steven buteneers’s picture

steven buteneers commented
StatusFileSize
new mail_safety_cron_cleanup-2894814-3.patch1.92 KB

Applied patch to add cron functionality to que old mails to be deleted, using the already available delete callback as worker callback for the que items.

steven buteneers’s picture

steven buteneers commented
Status: Active » Needs review
dmsmidt’s picture

dmsmidt
he/him
Primary language Dutch
Location Amsterdam 🇳🇱🇪🇺
commented
Assigned: steven buteneers » Unassigned
Status: Needs review » Needs work

Some nits:

  1. +++ b/mail_safety.admin.inc
@@ -40,6 +40,14 @@ function mail_safety_admin_settings_form() {
+    '#title' => t('Clean up dashboard mails older than the given period'),

    Change to:
    Remove stored mails older than the given age. 

  2. +++ b/mail_safety.admin.inc
@@ -40,6 +40,14 @@ function mail_safety_admin_settings_form() {
+    '#default_value' => variable_get('mail_safety_clean_up_dashboard', 0),

    I would rename this variable to: mail_safety_mail_expire.

    This variable should also be deleted on module uninstall. 

  3. +++ b/mail_safety.module
@@ -283,3 +283,38 @@ function theme_mail_safety_details($variables) {
+  $queues['mail_safety_cleanup'] = array(

    Rename to: mail_safety_expire_mails? 

  4. +++ b/mail_safety.module
@@ -283,3 +283,38 @@ function theme_mail_safety_details($variables) {
+  // Que mail safety mails older than the expiration setting to be deleted.

    Typ0's:
    Que/Queue
    mail safety/Mail Safety

steven buteneers’s picture

steven buteneers commented
StatusFileSize
new mail_safety_cron_cleanup-2894814-6.patch1.9 KB
new interdiff-2894814-3-6.txt1.74 KB

Added new patch + interdiff

steven buteneers’s picture

steven buteneers commented
Status: Needs work » Needs review
steven buteneers’s picture

steven buteneers commented
StatusFileSize
new mail_safety_cron_cleanup-2894814-8.patch2.29 KB
new interdiff-2894814-3-8.txt2.08 KB

Forgot to add the variable_del on uninstall. Re-added patch + interdiff

dmsmidt’s picture

dmsmidt
he/him
Primary language Dutch
Location Amsterdam 🇳🇱🇪🇺
commented
Status: Needs review » Reviewed & tested by the community

Looking good!

steven buteneers’s picture

steven buteneers commented
Status: Reviewed & tested by the community » Needs review
StatusFileSize
new interdiff-2894814-8-10.txt374 bytes
new mail_safety_cron_cleanup-2894814-10.patch2.3 KB

Found out that the queue retrieved for creating items was wrong, added a new patch + interdiff

dmsmidt’s picture

dmsmidt
he/him
Primary language Dutch
Location Amsterdam 🇳🇱🇪🇺
commented
Version: 7.x-1.0 » 7.x-1.x-dev
Status: Needs review » Reviewed & tested by the community

A that I missed that. Good catch. Tested and works.

  • barthje committed c727c36 on 7.x-1.x 
    Issue #2894814 by StevenButeneers: Periodic cleanup.
Issue #2147923 by...
barthje’s picture

barthje commented
Status: Reviewed & tested by the community » Fixed

Added to the new release

Status: Fixed » Closed (fixed)

Automatically closed - issue fixed for 2 weeks with no activity.