Say which address change actually fires the event (3.3) - #220
Open
nursoda wants to merge 1 commit into
Open
Conversation
nursoda
added
the
documentation
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
The class docblock claimed that every path clearing the address used for
delivery goes through setSystemEMailAddress() and therefore reaches this
listener. It does not. Deleting the additional address a user had picked as
their notification address calls setPrimaryEMailAddress(''), and
`occ user:setting <uid> settings primary_email` writes or deletes that user
value directly — only `email` and `display_name` are special-cased there. Both
drop settings/primary_email without dispatching anything, so delivery falls back
to the system address, or stops altogether when the account has none.
The account data does emit UserUpdatedEvent on the first path, but before the
primary address is reset, so getEMailAddress() read from it still yields the old
address. That event is therefore no hook for this, which is why the listener
ignores it — a behaviour the test suite already pins.
Behaviour is unchanged: there is nothing to react to, and a listener that never
runs leaves the provider enabled, which is stricter than disabling it.
This is the 3.3 counterpart of the main-line commit. It carries no reference to
the threat model: this line ships no doc/ directory.
Signed-off-by: Olav Seyfarth <olav@seyfarth.de>
nursoda
force-pushed
the
docs/say-which-address-fires-the-event-3.3
branch
from
August 27, 2026 10:58
ce0e495 to
9e4bfff
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
1 participant
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.
3.3 counterpart of #219.
The class docblock of
EMailDeletedclaimed that every path clearing the address used for delivery goes throughIUser::setSystemEMailAddress()and therefore reaches this listener. It does not.Two paths change the delivery address without dispatching anything usable:
IUser::setPrimaryEMailAddress('')(apps/provisioning_api/lib/Controller/UsersController.php, branchCOLLECTION_EMAIL), which only removes thesettings/primary_emailuser value.occ user:setting <uid> settings primary_emailwrites or deletes that user value directly — onlyemailanddisplay_nameare special-cased incore/Command/User/Setting.php.After either,
getEMailAddress()falls back to the system address, or returnsnullwhen the account has none. The account data does emitUserUpdatedEventon the first path, but before the primary address is reset, sogetEMailAddress()read from it still yields the old address. That event is no hook for this, which is why the listener ignores it — a behaviourEMailDeletedTestalready pins.No behaviour changes. There is nothing to react to, and a listener that never runs leaves the provider enabled. That is stricter than disabling it, never a downgrade to password-only.
Deliberately different from #219: no reference to the threat model entry, because this line ships no
doc/directory. It can therefore merge independently of #214.🤖 Generated with Claude Code, verified, tweaked and approved by @nursoda.