Page MenuHomePhabricator

Improve error handling when editing or deleting events
Closed, ResolvedPublic

Description

Error handling of EditEventCommand and DeleteEventCommand (and related code) is slightly confusing:

  • EventStore::deleteRegistration is guaranteed to succeed, whereas ::saveRegistration returns a StatusValue, suggesting it may fail while it really can't (that is, it could fail but then it will throw an exception that should not be caught by us)
  • EditEventCommand has dead code for handling the return value of ::saveRegistration, which again can never really fail
  • DeleteEventCommand::deleteUnsafe returns a StatusValue, suggesting that the operation could fail, but it presently can't, nor is it documented to do so.
  • The page deletion hook handler implicitly assumes that ::deleteUnsafe will succeed, but that's not guaranteed.

Acceptance criteria

  • (no need to QA) All methods in EventStore should be guaranteed to succeed, for consistency
    • (no need to QA) Their callers should be updated accordingly
  • (no need to QA) The documentation of DeleteEventCommand::deleteUnsafe should make it clear that the operation could possibly fail
  • (no need to QA) When an event page is deleted, we should use the onPageDelete hook instead of onPageDeleteComplete, which allows us to abort the deletion of the page if there's an error
  • Editing event registrations should still be functional
  • Deleting event registrations should still be functional
  • Deleting an event page should also delete the associated registration, as before

Event Timeline

Change 900385 had a related patch set uploaded (by Daimona Eaytoy; author: Daimona Eaytoy):

[mediawiki/extensions/CampaignEvents@master] Improve error handling when editing or deleting events

https://gerrit.wikimedia.org/r/900385

Change 900385 merged by jenkins-bot:

[mediawiki/extensions/CampaignEvents@master] Improve error handling when editing or deleting events

https://gerrit.wikimedia.org/r/900385

🚫 Editing event registrations should still be functional
Editing is functioning correctly, except for bug reported at T334483#8848280


✅ Deleting event registrations should still be functional

Screen Recording 2023-05-12 at 3.05.27 PM.gif (1×2 px, 503 KB)


✅ Deleting an event page should also delete the associated registration, as before

Screenshot 2023-05-12 at 2.56.34 PM.png (556×1 px, 109 KB)

Screenshot 2023-05-12 at 2.59.25 PM.png (530×2 px, 136 KB)

Screenshot 2023-05-12 at 2.59.36 PM.png (558×2 px, 51 KB)

🚫 Editing event registrations should still be functional
Editing is functioning correctly, except for bug reported at T334483#8848280

(Cross-referencing my reply T334483#8848524 for completeness: that's a pre-existing bug)

vaughnwalters closed this task as Resolved.EditedMay 12 2023, 9:35 PM

🚫 Editing event registrations should still be functional
Editing is functioning correctly, except for bug reported at T334483#8848280

(Cross-referencing my reply T334483#8848524 for completeness: that's a pre-existing bug)

Okay thx @Daimona, moving this to done/resolved then, as that was the only remaining issue.