Add button for sending test email

[Chocobo1]

This is re-submission of PR #20194 due to the author of #20194 wasn't responsive.

• The "Support translation of WebUI error message" commit was not needed anymore due to recent refactorings.
• Added a new commit 'Enforce 'send test email' endpoint to be POST only'
• This PR resolved merge conflicts and has been rebased on master.

---

This allows for easily testing whether the provided email configuration is correct. Testing otherwise requires downloading a torrent until completion.

Note that the email is sent using the values currently saved to the config file; NOT necessarily the information currently entered into the options dialog. I'm open to ideas on how to make this more apparent to the user.

[glassez]

• Added a new commit 'Enforce 'send test email' endpoint to be POST only'

What's the point? It has no parameters and it isn't supposed to change the state of the application.

[glassez]

Note that the email is sent using the values currently saved to the config file; NOT necessarily the information currently entered into the options dialog. I'm open to ideas on how to make this more apparent to the user.

Make sendTestEmail() a free helper function independent from anything except its parameters and use it with currently entered settings as parameters.

[glassez]

BTW, it looks like a really good idea to be able to send a test email with the currently entered settings without requiring them to be applied. Otherwise, the user may be confused.

[Chocobo1]

What's the point? It has no parameters and it isn't supposed to change the state of the application.

It is submitting a request to do something, not just fetching some data/resources.

[glassez]

It is submitting a request to do something, not just fetching some data/resources.

Ok.

[Chocobo1]

Note that the email is sent using the values currently saved to the config file; NOT necessarily the information currently entered into the options dialog. I'm open to ideas on how to make this more apparent to the user.

Make sendTestEmail() a free helper function independent from anything except its parameters and use it with currently entered settings as parameters.

That paragraph was copied from the original PR. I consider it a good idea too but I would not like to do it in this PR due to the last PR was already approved in this state.

BTW, it looks like a really good idea to be able to send a test email with the currently entered settings without requiring them to be applied. Otherwise, the user may be confused.

I don't mind but I hope I can leave it for others to handle it.

[glassez]

That paragraph was copied from the original PR. I consider it a good idea too but I would not like to do it in this PR due to the last PR was already approved in this state.

I thought it was initially introduced in this PR. Apparently, I overlooked it in the original PR. So OK, let's leave it for another day.

[Piccirello]

Hmm, I would much prefer to merge my original PR than to have my contribution attributed to someone else. Is this now standard procedure in this repo?

[Chocobo1]

Hmm, I would much prefer to merge my original PR than to have my contribution attributed to someone else.

You are still the author: 0fd7fce

If you insist I'll reopen yours and close mine. But do mind there are unresolved comments (and merge conflicts) in the original PR.

Is this now standard procedure in this repo?

We sometimes 'help'/adopt other PRs in case the original author is not responsive or the required changes is too much.

[glassez]

Hmm, I would much prefer to merge my original PR than to have my contribution attributed to someone else.

Why do you think that "my contribution attributed to someone else"? It still contains the commit that is authored by you. And it is you who will be mentioned in the changelog.

[Piccirello]

My mistake, I didn't realize the commit was still attributed to me.

[Chocobo1]

@glassez
PR updated, squashed commits manually. If using github 'squash and merge', Piccirello will not be the sole author.

[Chocobo1]

@glassez @sledgehammer999
The merge situation seems strange for this PR so I tested it in another repo. I found out that 'squash and merge' will make me the commit author and therefore it can't be used here. 'Merge commit' and 'rebase and merge' both will retain Piccirello as the author.
So which merge strategy to use?

[glassez]

The merge situation seems strange for this PR so I tested it in another repo. I found out that 'squash and merge' will make me the commit author and therefore it can't be used here.

IIRC, GitHub assigns the PR author as the author of the squashed commit (so there are no problems with "squash and merge" original PRs).

[glassez]

So which merge strategy to use?

I don't mind using 'rebase and merge' in this case if you first insert PR #20488. into the commit message.

[Chocobo1]

I don't mind using 'rebase and merge' in this case if you first insert PR #20488. into the commit message.

Done.

[lighterowl]

You're not passing a parent to the constructor, so this is going to leak.

[lighterowl]

Nevermind - Smtp is set to call deleteLater when its socket disconnects.

[Chocobo1]

You're not passing a parent to the constructor, so this is going to leak.

(clarification: the code was from #20194) You are probably right. It will happen when the procedure is still running and user exited qbt. Also Smtp destructor seems to be missing closing the socket.
Do you mind submitting a patch?

Also note this:

qBittorrent/src/base/net/smtp.cpp

Line 122 in c06817f

connect(m_socket, &QAbstractSocket::disconnected, this, &QObject::deleteLater);

[glassez]

It will happen when the procedure is still running and user exited qbt.

What will happen?

Also Smtp destructor seems to be missing closing the socket.

Socket is closed in its destructor which is called by Smtp destructor.

[glassez]

But I still find it a confusing design when an explicitly created object destroys itself "without the knowledge of its creator/owner".