Skip to content

Conversation

@abhinavohri
Copy link
Contributor

Summary

This change allows to use the initiator's personal email as the sender email when sending share notifications, provided that the mail_providers_enabled configuration is active.

TODO

  • ...

Checklist

@abhinavohri abhinavohri requested a review from a team as a code owner December 14, 2025 15:30
@abhinavohri abhinavohri requested review from ArtificialOwl, CarlSchwan, come-nc and icewind1991 and removed request for a team December 14, 2025 15:30
Copy link
Member

@tcitworld tcitworld left a comment

Choose a reason for hiding this comment

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

@abhinavohri
Copy link
Contributor Author

Hi @tcitworld, I have implemented the same functionality using mail manager api. Could you please confirm if this is what you asked for?

@susnux susnux requested a review from tcitworld December 22, 2025 01:05
@susnux susnux added enhancement 3. to review Waiting for reviews labels Dec 22, 2025
@susnux susnux added this to the Nextcloud 33 milestone Dec 22, 2025
*
* If mail providers are enabled, uses the specified user's email address.
* If disabled or the user has no valid email, falls back to the system default.
* @since 31.0.12
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
* @since 31.0.12
* @since 33.0.0

* If disabled or the user has no valid email, falls back to the system default.
* @since 31.0.12
*/
public static function getEmailAddressForUser(?IUser $user, string $fallback_user_part): string {
Copy link
Contributor

Choose a reason for hiding this comment

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

The check $user !== null should be done outside for a cleaner api.

return self::getDefaultEmailAddress($fallback_user_part);
}

$config = \OCP\Server::get(serviceName: IConfig::class);
Copy link
Contributor

Choose a reason for hiding this comment

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

Use IAppConfig


$config = \OCP\Server::get(serviceName: IConfig::class);

$mailProvidersEnabled = $config->getAppValue('core', 'mail_providers_enabled', '0') === '1';
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
$mailProvidersEnabled = $config->getAppValue('core', 'mail_providers_enabled', '0') === '1';
$mailProvidersEnabled = $appConfig->getValueBool('core', 'mail_providers_enabled', true)

);
}
$message->setFrom([Util::getDefaultEmailAddress($instanceName) => $senderName]);
$fromAddress = Util::getDefaultEmailAddress(user_part: $instanceName);
Copy link
Contributor

Choose a reason for hiding this comment

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

I assume you want to use the new function you added here?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I have implemented appConfig directly here instead of creating a new function. If you prefer, I can shift this to a new function.

Comment on lines 389 to 415
$mailProvidersEnabled = $this->appConfig->getValueBool('core', 'mail_providers_enabled');
if ($mailProvidersEnabled && $this->mailManager->has()) {
if ($initiatorEmail !== null) {
$service = $this->mailManager->findServiceByAddress($initiator, $initiatorEmail);
if ($service !== null) {
$fromAddress = $service->getPrimaryAddress()->getAddress();
}
}
}
$message->setFrom([$fromAddress => $senderName]);

// The "Reply-To" is set to the sharer if an mail address is configured
// also the default footer contains a "Do not reply" which needs to be adjusted.
if ($initiatorUser && $this->settingsManager->replyToInitiator()) {
$initiatorEmail = $initiatorUser->getEMailAddress();
if ($initiatorEmail !== null) {
$message->setReplyTo([$initiatorEmail => $initiatorDisplayName]);
$emailTemplate->addFooter($instanceName . ($this->defaults->getSlogan() !== '' ? ' - ' . $this->defaults->getSlogan() : ''));
} else {
$emailTemplate->addFooter();
}
} else {
$emailTemplate->addFooter();
}

$message->useTemplate($emailTemplate);
$failedRecipients = $this->mailer->send($message);
Copy link
Contributor

Choose a reason for hiding this comment

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

Okay, so you are pulling the mail service, but then you are just pulling the from address not actually sending the message using the mail provider. Your code is still using the system mailer to send the message, this will not work correctly.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

3. to review Waiting for reviews enhancement

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Allow user-level sender addresses for “Share by mail” (file sharing)

5 participants