MIRROR PR: feat(sharebymail): dispatch BeforeShare*MailSentEvent before each mail send - #277
Draft
printminion-co wants to merge 46 commits into
Draft
MIRROR PR: feat(sharebymail): dispatch BeforeShare*MailSentEvent before each mail send#277printminion-co wants to merge 46 commits into
printminion-co wants to merge 46 commits into
Conversation
printminion-co
force-pushed
the
feat/sharebymail-before-mail-sent-event
branch
from
July 9, 2026 14:25
c09ac83 to
f2b7041
Compare
There was a problem hiding this comment.
Pull request overview
This PR adds PSR-14 events in apps/sharebymail that are dispatched immediately before each native mailer->send() call in ShareByMailProvider, allowing listeners to suppress/replace SMTP delivery for share-by-mail notifications (share link, password, and note mails).
Changes:
- Introduce
BeforeShare*MailSentEventclasses (with a shared abstract base) to carry the preparedIMessageand recipients and allow suppressing native sending viamarkMailHandled(). - Dispatch these events from
ShareByMailProvider::sendEmail(),sendPassword(),sendPasswordToOwner(), andsendNote(), skipping native send when handled. - Update unit tests and app-local composer autoload classmaps to include the new event classes.
Reviewed changes
Copilot reviewed 8 out of 8 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| apps/sharebymail/lib/ShareByMailProvider.php | Dispatch new events before each mail send and skip native send when handled; refactor template data arrays. |
| apps/sharebymail/tests/ShareByMailProviderTest.php | Extend tests to assert event dispatch and updated template data; add tests around “mail handled” behavior for password flows. |
| apps/sharebymail/lib/Event/AbstractBeforeShareMailSentEvent.php | Add common event base carrying share, recipients, and rendered message plus “handled” flag. |
| apps/sharebymail/lib/Event/BeforeShareMailSentEvent.php | Add event for share-link notification mails. |
| apps/sharebymail/lib/Event/BeforeSharePasswordMailSentEvent.php | Add event for password notification mails (recipient + owner cases). |
| apps/sharebymail/lib/Event/BeforeShareNoteMailSentEvent.php | Add event for note-update notification mails. |
| apps/sharebymail/composer/composer/autoload_static.php | Register new event classes in the app-local composer autoloader map. |
| apps/sharebymail/composer/composer/autoload_classmap.php | Register new event classes in the app-local composer classmap. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
+26
to
+28
| /** | ||
| * @param string[] $resolvedEmails validated recipients | ||
| */ |
There was a problem hiding this comment.
Fixed in commit fix(sharebymail): fix docstring and add isMailHandled tests for sendEmail and sendNote. The @param docstring now reads "resolved recipient addresses (not guaranteed to be validated)" to accurately reflect that call sites like sendNote() and sendPasswordToOwner() do not validate addresses before dispatching.
Comment on lines
+411
to
+415
| $event = new BeforeShareMailSentEvent($share, $emails, $message, $templateData); | ||
| $this->eventDispatcher->dispatchTyped($event); | ||
| if ($event->isMailHandled()) { | ||
| return; | ||
| } |
There was a problem hiding this comment.
Added testSendEmailDoesNotSendWhenMailHandledByListener in commit fix(sharebymail): fix docstring and add isMailHandled tests for sendEmail and sendNote. The test sets up a listener that calls markMailHandled() on the dispatched BeforeShareMailSentEvent and asserts mailer->send() is never called.
Comment on lines
+585
to
+589
| $event = new BeforeShareNoteMailSentEvent($share, [$recipient], $message, $templateData); | ||
| $this->eventDispatcher->dispatchTyped($event); | ||
| if ($event->isMailHandled()) { | ||
| return; | ||
| } |
There was a problem hiding this comment.
Added testSendNoteDoesNotSendWhenMailHandledByListener in commit fix(sharebymail): fix docstring and add isMailHandled tests for sendEmail and sendNote. The test marks BeforeShareNoteMailSentEvent as handled via a listener and asserts mailer->send() is never invoked.
printminion-co
force-pushed
the
feat/sharebymail-before-mail-sent-event
branch
from
July 9, 2026 14:47
6274dc9 to
3391f42
Compare
printminion-co
force-pushed
the
feat/sharebymail-before-mail-sent-event
branch
2 times, most recently
from
July 10, 2026 07:21
d928ae7 to
edc6c9c
Compare
printminion-co
changed the title
printminion-co
marked this pull request as draft
July 10, 2026 08:52
Signed-off-by: Robin Appelman <robin@icewind.nl> Signed-off-by: Carl Schwan <carlschwan@kde.org>
…rd protected in IShare->isPasswordProtected() Signed-off-by: Tobias Knöppler <tobias@knoeppler.org>
Cache::copyFromCache() rebuilt the target's cache row via cacheEntryToArray(), which only carried over a boolean `encrypted` flag and dropped the real `encryptedVersion` count. Since View::copy() unconditionally calls copyFromCache() after the storage copy completes, this silently overwrote the correct encryptedVersion that Encryption::updateEncryptedVersion() had just set, collapsing it back to 0/1 on every copy of a file whose encryptedVersion was above 1. cacheEntryToArray() now includes the source entry's encryptedVersion when it is encrypted. The existing encrypted-to-non-encrypted-storage override in copyFromCache() also clears encryptedVersion alongside `encrypted`, since normalizeData() prefers encryptedVersion over `encrypted` when both are present in the update data. Signed-off-by: Kent Delante <kent@delante.me> Assisted-by: ClaudeCode:claude-sonnet-5
Signed-off-by: Kent Delante <kent@delante.me> Assisted-by: ClaudeCode:claude-sonnet-5
printminion-co
force-pushed
the
feat/sharebymail-before-mail-sent-event
branch
from
July 17, 2026 14:03
edc6c9c to
9ad378d
Compare
There was a bug in the filepicker (dialogs library) that passed always the current folder as selected node even if not pickable (`setCanPick`). So updated the library. But we also must check if we have any nodes selected to enable the copy and move buttons. Signed-off-by: Ferdinand Thiessen <opensource@fthiessen.de>
Signed-off-by: Ferdinand Thiessen <opensource@fthiessen.de>
Signed-off-by: Ferdinand Thiessen <opensource@fthiessen.de>
…r-share-is-pw-protected Refactor: Centralize logic for checking if a share is password protected
…ize-fail fix: don't die if we can't encode file metadata on upload
Robin Appleman's SMB library is designed so that 'null' is to be used
when no explicit workgroup is set:
final class BasicAuth implements IAuth {
/** @var string */
private $username;
/** @var string|null */
private $workgroup;
/** @var string */
private $password;
//...
However, it previously was loose with its checks and would still treat
any falsy value (e.g. an empty string) as "not specified" when forming
arguments for the various underlying utilities, such as `smbclient`.
icewind/SMB@4a93467905 updated it's handling to be more strict and as
such will now treat empty strings as distinct from null values for
workgroups. An empty value for workgroup often doesn't make sense,
unless the backend tool happens to convert it to "WORKGROUP". In the
case of the `smbclient` utility, passing an empty string for the
$workgroup paramters results in `-W ''` being used which is invalid
syntax and so the invocation fails resulting in the user seeing an
`[Icewind\SMB\Exception\ConnectionRefusedException]` error. Therefore,
it is important to always pass 'null' to this constructor for $workgroup
when there isn't one.
Most paths in the 'files_external' app already handle this correctly,
but this now altered path originally just took the raw value from the
app backend and passed it as-is. Assuming the user left the "Domain"
box empty in the frontend, this would be an empty string. Now the
the SMB library's interface is correctly honored in that case.
It's arguable that the SMB library should simply be updated to
formally accept '' as a valid input meaning "no workgroup", restoring
the old behavior in a canonical fashion, as there is almost no
situation where one would want to pass an empty string and have it
mean anything else; but regardless, there is no harm in being more
explicit on the nextcloud/server side as well.
Fixes nextcloud#58445.
Signed-off-by: Christian Heimlich <chris@pcserenity.com>
The previous implementation executes "ps" with flags -p and -o to check whether a job process was still running. This approach has portability issues regarding e.g. BusyBox ps (Alpine Linux) where `ps -p` fails or BSD/macOS where `ps -o` fails. Replace with `posix_kill($pid, 0)`, the standard POSIX way to probe process existence without delivering a signal. EPERM is treated as "process exists but not owned by current user". This should be available on all supported platforms. Fixes: 60ce92a Suggested-by: Michele Marcionelli Signed-off-by: Stefan Kalscheuer <stefan@stklcode.de>
Snowflake ID in `JobRuns:deleteBefore()` is computed from given timestamp, but immediately overwritten by a literal value making the expiration setting have no effect. This looks like a leftover artifact from development that should be cleaned up. Remove the hardcoded ID to restore the desired behavior. Fixes: dc5499a Suggested-by: Michele Marcionelli Signed-off-by: Stefan Kalscheuer <stefan@stklcode.de>
fix(jobs): resolve portability issues with background job cleanup
…iles-smb-auth fix(files_external/SMB): Use 'null' explicitly for no workgroup
- Do not use hard coded user-id. - Properly clean-up created user in `finally` clause Signed-off-by: Ferdinand Thiessen <opensource@fthiessen.de>
Signed-off-by: Nextcloud bot <bot@nextcloud.com>
fix(files): properly handle folders without permissions
Signed-off-by: Nextcloud bot <bot@nextcloud.com>
Signed-off-by: GitHub <noreply@github.com>
Signed-off-by: Nextcloud bot <bot@nextcloud.com>
Signed-off-by: provokateurin <kate@provokateurin.de>
Signed-off-by: provokateurin <kate@provokateurin.de>
Signed-off-by: provokateurin <kate@provokateurin.de>
Signed-off-by: provokateurin <kate@provokateurin.de>
Signed-off-by: provokateurin <kate@provokateurin.de>
…or-changes Apply rector changes
Add Unified Sharing
Installing Nextcloud might take some time. Marking those tests as slow to prevent flaky CI. Signed-off-by: Ferdinand Thiessen <opensource@fthiessen.de>
Signed-off-by: provokateurin <kate@provokateurin.de>
Signed-off-by: provokateurin <kate@provokateurin.de>
test(users): prevent flaky tests with hardcoded values
…che-include-encrypted-version fix(files): preserve encryptedVersion when copying cache entries
Signed-off-by: provokateurin <kate@provokateurin.de>
Signed-off-by: provokateurin <kate@provokateurin.de>
test(files): wait for actual requests when deleting files
test(files): wait for PUT before continue with tests
test: mark installation tests as slow
Signed-off-by: Joas Schilling <coding@schilljs.com>
…me-parameters-as-sensitive fix: Mark some parameters as sensitive
…lay-name perf(ShareUser): Only fetch display name instead of user object
Signed-off-by: Nextcloud bot <bot@nextcloud.com>
…e-constructor-properties Use constructor properties in Sharing
…ateData variables Extract the inline array literals passed to each createEMailTemplate() call into named $templateData variables. For sendNote(), which previously passed no data array, add a minimal array so all four send methods are consistent. Zero behaviour change — $templateData is passed straight through to createEMailTemplate() with identical contents. Signed-off-by: Misha M.-Kupriyanov <kupriyanov@strato.de>
…ch mail send
Dispatch a typed PSR-14 event immediately before every mailer->send() call
in ShareByMailProvider so that external listeners can intercept and replace
Nextcloud's native SMTP delivery.
Event hierarchy:
AbstractBeforeShareMailSentEvent (base: share, resolvedEmails, message,
markMailHandled / isMailHandled)
├── BeforeShareMailSentEvent – sendEmail()
├── BeforeSharePasswordMailSentEvent – sendPassword() + sendPasswordToOwner()
└── BeforeShareNoteMailSentEvent – sendNote()
Each concrete class holds its own typed $templateData (psalm array-shape)
and exposes named getters (getSenderUserId(), getFileName(), …) instead of
a generic getMailData(): array<string,mixed>. This avoids defensive
is_string() / null guards in listeners.
$templateData reuses the array already passed to createEMailTemplate(), with
one extra key added for sendEmail(): senderUserId (the raw user ID, distinct
from the display name stored under 'initiator').
The native mailer->send() is skipped when a listener calls markMailHandled().
If the listener's own send throws, the exception propagates and the native
send is also skipped — no silent SMTP fallback.
sendEmail() and sendPassword() are flattened from nested
if (!isMailHandled()) { ... } pyramids to early-return style.
createPasswordSendActivity() in sendPassword() and sendPasswordToOwner() now runs
only when the mail was actually sent: it moved inside the isMailHandled() early
return, so a listener that suppresses the SMTP send no longer produces a false
password-share activity entry.
Signed-off-by: Misha M.-Kupriyanov <kupriyanov@strato.de>
printminion-co
force-pushed
the
feat/sharebymail-before-mail-sent-event
branch
from
July 21, 2026 07:32
9ad378d to
3ff2742
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Upstream-ready extract of IONOS PR #262, prepared for contribution to
nextcloud/server.Introduces a PSR-14 event dispatched immediately before every
mailer->send()inShareByMailProvider, so listeners can intercept/replace native SMTP delivery for share-by-mailnotifications.
apps/sharebymail/lib/Event/classes:AbstractBeforeShareMailSentEvent(base) +BeforeShareMailSentEvent,BeforeSharePasswordMailSentEvent,BeforeShareNoteMailSentEventsendEmail(),sendPassword(),sendPasswordToOwner(),sendNote();markMailHandled()skips native send (no silent fallback)$templateDatarefactor + fix socreatePasswordSendActivityno longer fires when a listenersuppresses the mail
Scope / cleanup vs PR #262
apps-custom/nc_ionos_processessubmodule bump and thehand-edited generated autoload noise. Diff is
apps/sharebymail/**only.IONOS(...)/HDNEXTmarkers); rebased onto currentmaster. Drift fixes: dropped an unused import; replacedwithConsecutive()(removed inPHPUnit 10) with a
willReturnCallbackidiom; autoload entries inksortorder.Verification (dev container)
./autotest.sh sqlite apps/sharebymail/tests/ShareByMailProviderTest.php→ OK (40 tests, 440 assertions)composer cs:checkclean on the changed filesFollow-ups
refactor(sharebymail)commit is missing aSigned-off-by— will be added + force-pushed.nextcloud/serverPR to be opened separately by the human contributor (AI policy).AI assistance
Branch prepared with AI assistance (cherry-pick, reword, drift-conflict resolution). Underlying
code is human-authored (PR #262).