ledger123 / runmyaccounts

SQL-Ledger Customizations by Run My Accounts
GNU General Public License v2.0
16 stars 9 forks source link

Notes regarding sent email #300

Closed sweitmann closed 1 year ago

sweitmann commented 2 years ago

Hi Armaghan,

Philipp has informed me that SQL-Ledger writes the note regarding sent email before the email is actually sent. So, if there is a problem with the mailserver, there could be a note in the booking that says that it was sent, but that is actually not true.

Would it be possible to make SQL-Ledger write the note only after the email has been sent instead?

Regards

ledger123 commented 2 years ago

Hi Sebastian,

This is not possible as per my knowledge as we hand over email to postfix and there is no way we can know if was sent or not.

Regards

phkroll commented 2 years ago

@ledger123 Hey Armaghan, that might be true, but as far as I remember my research back when I noticed that issue (like 1 or 2 years ago) the intnotes are saved before even the PDF is generated. So, maybe we are not able to tell if the E-Mail was sent yes. But we are able to tell if we successfully could add it to postfix or API.

I jsut checked the code and tried to find it again, I'm not 100% sure if I found the correct location but it looks to me like: In io.pl Line 1746 we update the intnotes with the generated E-Mail text. In io.pl Line 1794 we call parse_template which also sends the E-Mail / passes it to postfix/API.

So IMO we should be able add some note that we start sending in the beginning and in the end, we update if it was susccessfull added or not. Here my full description I added back then to the Issue:

Email sending log is produced in the wrong order. This makes it difficult to track errors.

The field internal notes should be updated as follows:

before sending starts: Start Time: Date / Time when sending begins

when sending succeeded: (just what is written now before sending started, except 1 new line) To: Email CC: Email BCC: Email Subject: Content: Finish Time: Date / Time when sending has successfully finished (This is new)

Also when sending succeeded: DB Status update: success / no success

When sending failed: SENDING FAILED Finish Time: Date / Time

ledger123 commented 2 years ago

Hi @phkroll,

I get what you are trying to explain. I am aware of all code which is reponsible for delivery and how notes are updated. That is not a problem.

But as I had written, postfix command line sendmail does not return any value which we can use to determine if the mail was sent or not and that is because of its design.

When we handover email postfix command sendmail queues email for delivery and it can be delivered right away, after 5 minutes, or after one hour or after 1 day depending upon many delivery factors.

So postfix integration is not synchronous with sql-ledger and cannot help us decide if the mail was delivered or not.

Regards

phkroll commented 2 years ago

Hey @ledger123

No matter if it's possible or if not, IMo it doesn't change anything on the fact, that the Text, that the E-Mail was sended, should be added after it was added to postfix/API. Since now it is added in the beginning and so many things can fail till the E-Mail is rlly created and handover to postfix/API.

ledger123 commented 2 years ago

Hi @phkroll ,

Yes, your idea makes sense. But did you have had any such complain from any customer? I personally try to avoid any changes which may seem wrong but every one is ok with that.

@sweitmann , kindly share your experience too regarding this.

Thanks

Regards

phkroll commented 2 years ago

Hey @ledger123

Yes, we had multiple clients complaining about this. That's why it even popped up by me. They solve it currently by adding themself to bcc, but some of them have over 1'000 invoices per month and in that cases this workaround is nor rlly a real solution for them, since it takes a lot of time for them and 'SPAMMING' there inbox.

ledger123 commented 2 years ago

Hi @phkroll ,

As I had pointed out earlier, the issue reported by your client cannot be sorted out no matter where we update internal notes because postfix does not return the delivery status to the calling problem.

Regards

phkroll commented 2 years ago

Hey @ledger123

True that. But, it is not only about intnotes, but also the status 'emailed' is set before the mail is put to Postfix/API. And if anything failes till the mail is created for postfix / API, it is marked as sent and mentioned like that in the intnotes. Most of the mails are not sended because of issues there. If an email is not sended because of a none existing recipient or something like that, they are actually saved to a certain location on file system and our provider regulary informs us about that. Also for the API, if something is wrong, the API might already return an error. If not, we have a tool to check it.

So these both things should just be after everything was done without error. IMO it just should be like. "Inform that it starts doing something" "Do it" if (fail){ "inform about error" } else { "inform about success" }

and not like now: "inform about success" "Do it"

sweitmann commented 1 year ago

Hi Armaghan, let's discuss this in our call tomorrow.

Regards

ledger123 commented 1 year ago

Hi Sebastian,

Good morning.

This has been fixed by moving status update code and int notes update code after the email is sent.

Code has been pushed in this branch for your testing.

Regards

sweitmann commented 1 year ago

Hi Armaghan,

Thanks for sorting this out. Tested and works fine. I see that it has already been released to develop, so I'm closing this ticket now.

Regards