Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Added a check in Mailbox preRemove hook call. If the hook is returnin… #203

Merged

Conversation

mfechner
Copy link
Contributor

…g a false, stop the deletion of the mailbox. The plugin has to add a message why the deletion of the mailbox was canceled.

This change is required to further implement pull request #179

@mfechner mfechner mentioned this pull request Aug 27, 2016
@mfechner
Copy link
Contributor Author

I documented the hook mailbox_purge_preRemove on the wiki page which was missing completely.

@barryo
Copy link
Member

barryo commented Aug 27, 2016

I'm not sure I like the idea of plugins being able to prevent deletion. Have we done this anywhere else?

My issue is if we have >1 plugin and the other plugins do their pre-deletion work on the assumption it'll succeed, then your plugin cancels the deletion, we'll have a potentially inconsistent state.

The answer may be just to document this and for plugins that do irreversible pre-deletion tasks to use the preFlush or postFlush hook.

Can you add some documentation to the call covering the above and - also - reverse the styling changes you made? Styling should be consistent to the rest of the file. Thanks a million.

…g a false, stop the deletion of the mailbox. The plugin has to add a message why the deletion of the mailbox was canceled.
@mfechner mfechner force-pushed the mailbox_preRemove_can_stop_removal branch from e50691d to 9d9b0b4 Compare August 27, 2016 10:21
@mfechner
Copy link
Contributor Author

yes, the alias deletion has the same logic in. That is how I understood the hooks, pre is there to do checks before the action is executed and post after the action as executed, please correct me if I understood there something wrong. Therefor I see it very logic that a pre check should be able to stop the action if required.

I updated the wiki.

Sorry for the style change, me IDE is doing it automatically, I reverted it.

@mfechner
Copy link
Contributor Author

I add a new part to the hook documentation:
https://github.com/opensolutions/ViMbAdmin/wiki/Plugins#workflow-of-actions-in-hooks

Hopefully that describes it a little better. Suggestions are welcome ;)

@barryo
Copy link
Member

barryo commented Aug 28, 2016

Cheers @idefix6 - merging now.

@barryo barryo merged commit 0308bed into opensolutions:master Aug 28, 2016
@mfechner mfechner deleted the mailbox_preRemove_can_stop_removal branch August 28, 2016 20:15
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.

2 participants