Skip to content

Commit

Permalink
N°2181 - Sometimes duplicate ticket from mail-to-ticket - retention_p…
Browse files Browse the repository at this point in the history
…eriod become period in hour
  • Loading branch information
accognet committed May 25, 2021
1 parent a7b3531 commit 0976f75
Show file tree
Hide file tree
Showing 2 changed files with 3 additions and 3 deletions.
4 changes: 2 additions & 2 deletions classes/emailbackgroundprocess.class.inc.php
Original file line number Diff line number Diff line change
Expand Up @@ -502,11 +502,11 @@ public function Process($iTimeLimit)
}

// Cleanup the unused replicas based on the pattern of their UIDL, unfortunately this is not possible in NON multi-source mode
$iRetentionPeriod = MetaModel::GetModuleSetting('combodo-email-synchro', 'retention_period', '0');
$iRetentionPeriod = MetaModel::GetModuleSetting('combodo-email-synchro', 'retention_period', '1');
$sOQL = "SELECT EmailReplica WHERE uidl LIKE " . CMDBSource::Quote($oSource->GetName() . '_%') .
" AND mailbox_path = " . CMDBSource::Quote($oSource->GetMailbox()) .
" AND id NOT IN (" . implode(',', CMDBSource::Quote($aIDs)) . ")".
" AND last_seen < DATE_SUB(NOW(), INTERVAL ".$iRetentionPeriod." DAY)";
" AND last_seen < DATE_SUB(NOW(), INTERVAL ".$iRetentionPeriod." HOUR)";
$this->Trace("Searching for unused EmailReplicas: '$sOQL'");
$oUnusedReplicaSet = new DBObjectSet(DBObjectSearch::FromOQL($sOQL));
$oUnusedReplicaSet->OptimizeColumnLoad(array('EmailReplica' => array('uidl')));
Expand Down
2 changes: 1 addition & 1 deletion module.combodo-email-synchro.php
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,7 @@
'notify_errors_from' => '', // mandatory as well (can be set at the same value as notify_errors_to)
'debug' => false, // Set to true to turn on debugging
'periodicity' => 30, // interval at which to check for incoming emails (in s)
'retention_period' => 0, // number of days we keep the replica
'retention_period' => 1, // number of hour we keep the replica
'body_parts_order' => 'text/html,text/plain', // Order in which to read the parts of the incoming emails
'pop3_auth_option' => 'USER',
'imap_options' => array('imap'),
Expand Down

6 comments on commit 0976f75

@jbostoen
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As for the original fix I did in my fork: I made it a longer time. Think of situations where O365 gives IMAP errors for longer periods of time. Sure they will fix it asap, but one hour as default value might not be that much.

@Hipska
Copy link
Collaborator

@Hipska Hipska commented on 0976f75 May 25, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Wow, @piRGoif are you aware of this breaking change?

@jbostoen
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@Hipska Actually it's still an improvement.
In the original situation, the Mail to Ticket process would still not see the email if the IMAP connection returns errors (which occasionally happened with our O365 environment).

This is more a grace period.

@Hipska
Copy link
Collaborator

@Hipska Hipska commented on 0976f75 May 26, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Its a breaking change of default values and even if users did specify a specific value, the meaning of it changes. So a migration would need to be done at least.

#4 is also an enhancement, and even there I needed to create a new additional config option to enable the enhancement.

@Hipska
Copy link
Collaborator

@Hipska Hipska commented on 0976f75 May 26, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sorry, never mind. Wasn't aware this config option wasn't in a released version yet. So there is no impact for existing users. Ignore my comments 😅

@accognet
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, this commit is a fix after testing. In testing, we saw that this period in days is too long, so I changed it to hours.

Please sign in to comment.