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.

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

Bug: Attach files in email #16423

Closed
leninrivas opened this issue Feb 24, 2021 · 9 comments
Closed

Bug: Attach files in email #16423

leninrivas opened this issue Feb 24, 2021 · 9 comments
Labels
Bug need more information This bug needs more information to be processed Bug This is a bug (something does not work as expected)

Comments

@leninrivas
Copy link
Contributor

Bug

Delete file or attach file does not work when sending email

Environment

  • Version: 13.0.1
  • OS: Linux
  • Web server: Apache
  • PHP: cgi-fcgi 5.6.40
  • Database: MySQL or MariaDB 5.5.5-10.3.23-MariaDB
  • URL(s): /comm/propal/card.php?id=1070&action=presend&mode=init
@leninrivas leninrivas changed the title Bug: Atachh files on email Bug: Attach files in email Feb 24, 2021
@cfoellmann
Copy link
Contributor

duplicate of #16420

@ksar-ksar ksar-ksar added the Bug This is a bug (something does not work as expected) label Feb 25, 2021
@daraelmin
Copy link
Contributor

#16420 is yet closed... @leninrivas Does the fix e0785ac solve this issue too ?

@matDOTviguier
Copy link

no @daraelmin but it is the same bug as #16420 !

daraelmin added a commit to daraelmin/dolibarr that referenced this issue Feb 27, 2021
#Fix Dolibarr#16423 Dolibarr#16420
clear_attached__files should be lauch only on init form.
daraelmin added a commit to daraelmin/dolibarr that referenced this issue Feb 27, 2021
daraelmin added a commit to daraelmin/dolibarr that referenced this issue Feb 28, 2021
eldy added a commit that referenced this issue Feb 28, 2021
@ksar-ksar
Copy link
Contributor

Seems to be Fixed ?

@daraelmin
Copy link
Contributor

@ksar-ksar I'm not sure, but I 'm on a the road for 4 days and I can not test.

Here are my details to explain my doubts :

First of all, I had some diffulties when changing the mail model. In this case the attached file by default are cleared and it remain impossible to join more file.

To me it was the bug cause all files are cleared when mailmodel is not '0' or '-1' but when reading the new comment of eldy it seems to the expected workflow :
// Clear temp files. Must be done before call of triggers, at beginning (mode = init), or when we select a new template (line 332)

Secondly, @xbloq in #16420 reported that in some module when sending mail with mailmodel by default, the files were cleared before the sending. It may be du to an appeal to the function getform() before executing the sendmail but this should not happen.

I don't know if the addition of the global conf help when keeping the mail model and adding some new file and I have not tested if the files are really sent with the last fix.

Finally, I don't understand why there's a difference in the condition to add attached files (line 385) and to clear attached files (line 333)

Line 333
if (GETPOST('mode', 'alpha') == 'init' || (GETPOST('modelselected') && GETPOST('modelmailselected', 'alpha') && GETPOST('modelmailselected', 'alpha') != '-1')) {

Line 385
if (GETPOST('mode', 'alpha') == 'init' || (GETPOST('modelmailselected', 'alpha') && GETPOST('modelmailselected', 'alpha') != '-1')) {

I have asked @eldy why should we restrict the possibility of attaching files orwhy we should delete them when choosing an email model in my PR, but I still don't know why these restrictions are maintened.

To me we just need to remove condition on mailmodel on line 333 and 385. So should we open this thread as "New feature" ?

Sorry for my english and the style. I'm writing on a smartphone.

@xbloq
Copy link
Contributor

xbloq commented Mar 1, 2021

all it's working for me now

@eldy
Copy link
Member

eldy commented Mar 1, 2021

A fix has been pushed into 13 branch. Thanks to test.

@ksar-ksar ksar-ksar added the Bug need more information This bug needs more information to be processed label Mar 3, 2021
@leninrivas
Copy link
Contributor Author

Test Ok. Thank you.

@daraelmin
Copy link
Contributor

This one c41b59b fixed the last part for me.
Thanks to @atm-greg .

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Bug need more information This bug needs more information to be processed Bug This is a bug (something does not work as expected)
Projects
None yet
Development

Successfully merging a pull request may close this issue.

7 participants