Skip to content

Conversation

@bhngupta
Copy link
Contributor

Fixes #6700

Checks if start date is prior to current date and then uses a flag to change notify logic

Added a new parameter associated with past events in

  • RequestDefinitionBase / withdraw()
  • Request Notifications / notify_withdrawn_request()

@bhngupta bhngupta requested a review from ThiefMaster March 12, 2025 14:11
@ThiefMaster
Copy link
Member

ThiefMaster commented Apr 4, 2025

Hey, while testing this I noticed that unfortunately adding the new arg to withdraw would have required any plugins overriding that method to be updated.

While this would have been doable (I doubt anyone besides us has plugins implementing service requests with custom actions during withdraw), it still feels a bit unclean.

Because of this I changed your PR a bit to simply not trigger withdrawal notifications at all if the event has finished.

(I'll keep you credited for the contribution nonetheless, since you could not have known this, and even I only realized this during testing.)

@ThiefMaster ThiefMaster merged commit 1adbb9e into indico:master Apr 4, 2025
10 checks passed
@ThiefMaster ThiefMaster added this to the v3.3 milestone Apr 4, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Do not trigger service request notifications when deleting past events

2 participants