Plugin saving and hook workflow should be reviewed
Plugin saving and hook workflow should be reviewed
There are some problems with the plugin saving workflow and hook system. Here is how it should work in my opinion: For an object creation: Run the precreate hooks of main tab-> if one fails, show the error and cancel the creation. (problem: in FD code, at this point it’s difficult to salvage the data, we would end up on the management class as we do for LDAP errors) Run the ldap save of main tab -> if it fails, cancel the saving (same as above) Run the postcreate hook of main tab -> failure has no effect other than showing an error For each tab:
- Run the precreate hooks of the tab -> if it fails, cancel the activation of this tab? (this will be complicated with tab dependencies and all this)
- Run the ldap save of the tab -> if it fails, cancel the ldap save of this tab (meaning do not run post saving stuff)
- Run the postcreate hooks of this tab -> failure has no effect other than showing an error (It’s almost the same for modification but with modify hooks)
What I mean by failure: If a script returns an other code than 0, it’s considered an error. It if outputs texts while returning an other code than 0 this text is considered the error text. If it outputs text but still returns 0 code it’s considered success but the text is shown in an info pop-up (All this is already in place as far as I know)
So the main changes from present situation would be:
- Failed precreate hook on main tab should cancel creation.
- Failed LDAP save should cancel postcreate/postmodify hooks from happening, and logging should not log the creation/modification, only the error.
- Failed LDAP save of main tab should cancel the save of other tabs (as they cannot succeed anyway).
I will add information in this ticket as the planning, thinking, coding or testing evolves.
(from redmine: issue id 4893, created on 2016-06-21, closed on 2017-02-23)
- Relations:
- relates #2126 (closed)
- relates #4891 (closed)
- relates #3789 (closed)
- relates #4917
- relates #5187
- relates #5411
- relates #5551
- relates #5565
- Changesets:
- Revision 30a5d2ac on 2016-07-11T09:55:48.000Z:
Fixes #4893 Removed unused cleanup attribute from simplePlugin::ldap_save
- Revision dacddb21 on 2016-07-12T05:25:08.000Z:
Fixes #4893 changed simplePlugin::save behavior
- Revision d1f598b2 on 2016-07-12T07:02:42.000Z:
Fixes #4893 Refactored tabs saving and checking
- Revision a4a0c445 on 2016-07-12T07:19:56.000Z:
Fixes #4893 ldap_save should always return an array even if empty
- Revision 0a678cfa on 2016-07-12T10:45:01.000Z:
Fixes #4893 Oops
- Revision 25278f63 on 2016-07-12T10:55:52.000Z:
Fixes #4893 Added missing global $config in prepare_save
- Revision 81826968 on 2016-07-12T11:18:06.000Z:
Fixes #4893 Fixed var name in simpleTabs
- Revision b571d4ed on 2016-07-13T05:05:43.000Z:
Fixes #4893 Returning errors from ldap_save
- Revision 7c7e8fb8 on 2016-07-13T05:06:55.000Z:
Fixes #4893 Fixes in userRoles about new errors handling
- Revision e8715659 on 2016-07-13T05:28:11.000Z:
Fixes #4893 first go at fixing copy/paste for new save workflow
- Revision e298e4d3 on 2016-07-13T05:41:48.000Z:
Fixes #4893 Fixed syntax errors
- Revision 4ba9d5e2 on 2016-07-13T05:58:05.000Z:
Fixes #4893 Removed last call to check() from copyPaste
- Revision a35cf153 on 2016-07-13T07:05:58.000Z:
Fixes #4893 fixed userRoles
- Revision 8fc857d2 on 2016-07-27T10:14:57.000Z:
Fixes #4893 Fixed remove workflow too
- Revision 28758603 on 2016-07-28T15:46:34.000Z:
Fixes #4893 Fixed configuration tabs save method
- Revision d21673db by Côme Chilliet on 2016-09-21T12:45:50.000Z:
Fixes #4893 Fixed a few cases of error handling
- Custom Fields:
- Bug in version: 1.0.13