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