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

Add button for sending test email #20488

Merged
merged 1 commit into from
Mar 8, 2024
Merged

Add button for sending test email #20488

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
18 changes: 18 additions & 0 deletions src/app/application.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -680,6 +680,24 @@ void Application::sendNotificationEmail(const BitTorrent::Torrent *torrent)
content);
}

void Application::sendTestEmail() const
{
const Preferences *pref = Preferences::instance();
if (pref->isMailNotificationEnabled())
{
// Prepare mail content
const QString content = tr("This is a test email.") + u'\n'
+ tr("Thank you for using qBittorrent.") + u'\n';

// Send the notification email
auto *smtp = new Net::Smtp();
Copy link

Choose a reason for hiding this comment

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

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

Copy link

Choose a reason for hiding this comment

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

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

Copy link
Member Author

Choose a reason for hiding this comment

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

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);

Copy link
Member

Choose a reason for hiding this comment

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

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.

Copy link
Member

@glassez glassez Mar 8, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

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

smtp->sendMail(pref->getMailNotificationSender(),
pref->getMailNotificationEmail(),
tr("Test email"),
content);
}
}

void Application::torrentAdded(const BitTorrent::Torrent *torrent) const
{
const Preferences *pref = Preferences::instance();
Expand Down
2 changes: 2 additions & 0 deletions src/app/application.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -127,6 +127,8 @@ class Application final : public BaseApplication, public BaseIApplication
int memoryWorkingSetLimit() const override;
void setMemoryWorkingSetLimit(int size) override;

void sendTestEmail() const override;

#ifdef Q_OS_WIN
MemoryPriority processMemoryPriority() const override;
void setProcessMemoryPriority(MemoryPriority priority) override;
Expand Down
2 changes: 2 additions & 0 deletions src/base/interfaces/iapplication.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -83,6 +83,8 @@ class IApplication
virtual int memoryWorkingSetLimit() const = 0;
virtual void setMemoryWorkingSetLimit(int size) = 0;

virtual void sendTestEmail() const = 0;

#ifdef Q_OS_WIN
virtual MemoryPriority processMemoryPriority() const = 0;
virtual void setProcessMemoryPriority(MemoryPriority priority) = 0;
Expand Down
5 changes: 5 additions & 0 deletions src/gui/optionsdialog.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -707,6 +707,11 @@ void OptionsDialog::loadDownloadsTabOptions()
connect(m_ui->groupMailNotifAuth, &QGroupBox::toggled, this, &ThisType::enableApplyButton);
connect(m_ui->mailNotifUsername, &QLineEdit::textChanged, this, &ThisType::enableApplyButton);
connect(m_ui->mailNotifPassword, &QLineEdit::textChanged, this, &ThisType::enableApplyButton);
connect(m_ui->sendTestEmail, &QPushButton::clicked, this, [this]
{
app()->sendTestEmail();
QMessageBox::information(this, tr("Test email"), tr("Attempted to send email. Check your inbox to confirm success"));
});

connect(m_ui->groupBoxRunOnAdded, &QGroupBox::toggled, this, &ThisType::enableApplyButton);
connect(m_ui->lineEditRunOnAdded, &QLineEdit::textChanged, this, &ThisType::enableApplyButton);
Expand Down
14 changes: 14 additions & 0 deletions src/gui/optionsdialog.ui
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1566,6 +1566,19 @@ readme[0-9].txt: filter 'readme1.txt', 'readme2.txt' but not 'readme10.txt'.</st
</layout>
</widget>
</item>
<item>
<widget class="QPushButton" name="sendTestEmail">
<property name="sizePolicy">
<sizepolicy hsizetype="Fixed" vsizetype="Fixed">
<horstretch>0</horstretch>
<verstretch>0</verstretch>
</sizepolicy>
</property>
<property name="text">
<string>Send test email</string>
</property>
</widget>
</item>
</layout>
</widget>
</item>
Expand Down Expand Up @@ -3935,6 +3948,7 @@ Use ';' to split multiple entries. Can use wildcard '*'.</string>
<tabstop>groupMailNotifAuth</tabstop>
<tabstop>mailNotifUsername</tabstop>
<tabstop>mailNotifPassword</tabstop>
<tabstop>sendTestEmail</tabstop>
<tabstop>checkSmtpSSL</tabstop>
<tabstop>lineEditRunOnAdded</tabstop>
<tabstop>lineEditRunOnFinished</tabstop>
Expand Down
5 changes: 5 additions & 0 deletions src/webui/api/appcontroller.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1107,6 +1107,11 @@ void AppController::defaultSavePathAction()
setResult(BitTorrent::Session::instance()->savePath().toString());
}

void AppController::sendTestEmailAction()
{
app()->sendTestEmail();
}

void AppController::networkInterfaceListAction()
{
QJsonArray ifaceList;
Expand Down
1 change: 1 addition & 0 deletions src/webui/api/appcontroller.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -48,6 +48,7 @@ private slots:
void preferencesAction();
void setPreferencesAction();
void defaultSavePathAction();
void sendTestEmailAction();

void networkInterfaceListAction();
void networkInterfaceAddressListAction();
Expand Down
3 changes: 2 additions & 1 deletion src/webui/webapplication.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -149,19 +149,20 @@ class WebApplication final : public ApplicationComponent<QObject>
const QHash<std::pair<QString, QString>, QString> m_allowedMethod =
{
// <<controller name, action name>, HTTP method>
{{u"app"_s, u"sendTestEmail"_s}, Http::METHOD_POST},
{{u"app"_s, u"setPreferences"_s}, Http::METHOD_POST},
{{u"app"_s, u"shutdown"_s}, Http::METHOD_POST},
{{u"auth"_s, u"login"_s}, Http::METHOD_POST},
{{u"auth"_s, u"logout"_s}, Http::METHOD_POST},
{{u"rss"_s, u"addFeed"_s}, Http::METHOD_POST},
{{u"rss"_s, u"setFeedURL"_s}, Http::METHOD_POST},
{{u"rss"_s, u"addFolder"_s}, Http::METHOD_POST},
{{u"rss"_s, u"markAsRead"_s}, Http::METHOD_POST},
{{u"rss"_s, u"moveItem"_s}, Http::METHOD_POST},
{{u"rss"_s, u"refreshItem"_s}, Http::METHOD_POST},
{{u"rss"_s, u"removeItem"_s}, Http::METHOD_POST},
{{u"rss"_s, u"removeRule"_s}, Http::METHOD_POST},
{{u"rss"_s, u"renameRule"_s}, Http::METHOD_POST},
{{u"rss"_s, u"setFeedURL"_s}, Http::METHOD_POST},
{{u"rss"_s, u"setRule"_s}, Http::METHOD_POST},
{{u"search"_s, u"delete"_s}, Http::METHOD_POST},
{{u"search"_s, u"enablePlugin"_s}, Http::METHOD_POST},
Expand Down
18 changes: 18 additions & 0 deletions src/webui/www/private/views/preferences.html
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -263,6 +263,9 @@
</tr>
</table>
</fieldset>
<div class="formRow">
<input type="button" id="mail_test_button" value="QBT_TR(Send test email)QBT_TR[CONTEXT=OptionsDialog]" onclick="qBittorrent.Preferences.sendTestEmail();" />
</div>
</fieldset>

<fieldset class="settings">
Expand Down Expand Up @@ -1553,6 +1556,7 @@
changeWatchFolderSelect: changeWatchFolderSelect,
updateMailNotification: updateMailNotification,
updateMailAuthSettings: updateMailAuthSettings,
sendTestEmail: sendTestEmail,
updateAutoRun: updateAutoRun,
updateAutoRunOnTorrentAdded: updateAutoRunOnTorrentAdded,
generateRandomPort: generateRandomPort,
Expand Down Expand Up @@ -1705,6 +1709,7 @@
$('smtp_server_txt').setProperty('disabled', !isMailNotificationEnabled);
$('mail_ssl_checkbox').setProperty('disabled', !isMailNotificationEnabled);
$('mail_auth_checkbox').setProperty('disabled', !isMailNotificationEnabled);
$('mail_test_button').setProperty('disabled', !isMailNotificationEnabled);

if (!isMailNotificationEnabled) {
$('mail_auth_checkbox').setProperty('checked', !isMailNotificationEnabled);
Expand All @@ -1718,6 +1723,19 @@
$('mail_password_text').setProperty('disabled', !isMailAuthEnabled);
};

const sendTestEmail = function() {
new Request({
url: 'api/v2/app/sendTestEmail',
method: 'post',
onFailure: function() {
alert("QBT_TR(Could not contact qBittorrent)QBT_TR[CONTEXT=HttpServer]");
},
onSuccess: function() {
alert("QBT_TR(Attempted to send email. Check your inbox to confirm success)QBT_TR[CONTEXT=OptionsDialog]");
}
}).send();
};

const updateAutoRunOnTorrentAdded = function() {
const isAutoRunOnTorrentAddedEnabled = $('autorunOnTorrentAddedCheckbox').getProperty('checked');
$('autorunOnTorrentAddedProgram').setProperty('disabled', !isAutoRunOnTorrentAddedEnabled);
Expand Down