Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix email collector datetimelast ok and email fetching #32456

Open
wants to merge 4 commits into
base: develop
Choose a base branch
from
Open

Fix email collector datetimelast ok and email fetching #32456

Changes from 1 commit
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
f8f292c
Fix email collector datetimelast ok and email fetching
Dec 23, 2024
e33b46c
Merge branch 'develop' into fix-email-collector
eldy Dec 23, 2024
a276b7b
Merge branch 'develop' into fix-email-collector
eldy Dec 23, 2024
4b0879d
Merge branch 'develop' into fix-email-collector
eldy Dec 24, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 4 additions & 2 deletions htdocs/emailcollector/class/emailcollector.class.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1165,6 +1165,7 @@ public function doCollectOneCollector($mode = 0)
$arrayofemail = array();

$now = dol_now();
$datelastok = $now;

if (empty($this->host)) {
$this->error = $langs->trans('ErrorFieldRequired', $langs->transnoentitiesnoconv('EMailHost'));
Expand Down Expand Up @@ -1690,7 +1691,7 @@ public function doCollectOneCollector($mode = 0)
if ($mode > 0) {
$Query->leaveUnread();
}
$arrayofemail = $Query->limit($this->maxemailpercollect)->setFetchOrder("asc")->get();
$arrayofemail = $Query->limit($this->maxemailpercollect)->setFetchOrder("desc")->get();
Copy link
Member

@eldy eldy Dec 23, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

We must keep the order "asc" because we must process the email from the older one to the new one. In same order than they are received.
Does the fix change if we restore "asc" ?

Copy link
Contributor Author

@Hystepik Hystepik Dec 23, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

Yes as said in the description if we set setFetchOrder as asc the mail are not proceed the same way.
For exmple we want 2 mails to be proceed in a mail chain of 10 mails sent

  • if we keep asc, the last 2 mails of the chain mail are proceed first because in this case 'asc' means we start at the end of the mail inbox
  • if we set to desc, the 2 first mails are proceed because desc means we start from top of inbox.

And if we proceed the 10 emails of the mail chain then if we set desc or asc is the same

Copy link
Member

@eldy eldy Dec 24, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

Strange. I have got the opposite :

I have 1 email on 19 dec and another one of 20 dec.

If i set limit to 10, the result is the same, i agree.

But if i set limit to 1, i get the email of the 19 dec when sort is asc, and i get the 20 dec if sort is asc.
So on my side, I need asc.

According to this Webklex/php-imap#71, the sort is to define which bloc to receive when the limit is spliting the result into several blocks. According to exemple in this issue, when whe use the sort desc, we got the last recent block. And we want the older block.

So my result matches the explanation and the example of weklex. If max limit is higher than the number of results, then the sort has no effect. But if the max limit is lower, then we should use asc to get the block that is lower so older emails (the date that is the lower).

What weblex and its example says is that, inside a bloc, the sorting is always asc (great this is what we want), so trouble is only to get the oldest bloc when number of record is higher than limit.

And according to tests done with a block of 1, 'asc' means the last ancient block when there is several blocks.

We may contact each other later to make test on a real environment to see why you got a different behaviour...

dol_syslog("EmailCollector::doCollectOneCollector nb arrayofemail ".(is_array($arrayofemail) ? count($arrayofemail) : 'Not array')); // @phpstan-ignore-line
//var_dump($arrayofemail);
} catch (Exception $e) {
Expand Down Expand Up @@ -3635,6 +3636,7 @@ public function doCollectOneCollector($mode = 0)
// Stop the loop to process email if we reach maximum collected per collect
if ($this->maxemailpercollect > 0 && $nbemailok >= $this->maxemailpercollect) {
dol_syslog("EmailCollect::doCollectOneCollector We reach maximum of ".$nbemailok." collected with success, so we stop this collector now.");
$datelastok = strtotime($headers['Date']); // Set datetime
break;
}
} else {
Expand Down Expand Up @@ -3736,7 +3738,7 @@ public function doCollectOneCollector($mode = 0)
}

if (empty($error) && empty($mode)) {
$this->datelastok = $now;
$this->datelastok = $datelastok;
}

if (!empty($this->errors)) {
Expand Down
Loading