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

NEW hook formConfirm always called if hooked #9298

Merged
merged 2 commits into from Sep 1, 2018

Conversation

ksar-ksar
Copy link
Contributor

Actualy the hook formConfirm is called only if the variable $formconfirm is empty.
So it is called when there is NO formConfirm.
I think that the test should be if the $formconfirm is not empty

The hook should be called when the $formconfirm is not empty
@frederic34
Copy link
Contributor

A module can add a formconfirm if $formconfirm is empty, if you change that module can't add formconfirm 😟

@ksar-ksar
Copy link
Contributor Author

Dear Frederic,

I do not see when you could use this, and I do not think this was the goal of the Hook.
When you have a look on the rest of the code, you have the possibility to add a code to the existing formconfirm : if (empty($reshook)) $formconfirm.=$hookmanager->resPrint;
So if formconfirm is allways empty this line is there for nothing !

For me the test should be changed !

@eldy
Copy link
Member

eldy commented Aug 27, 2018

Yes the hook is called only if the variable $formconfirm is empty.
This is the expected behaviour. So if it is empty, we call the hook to implement the case when it is empty to fill it.
What is goal of calling it when it is already defined ?

@eldy eldy added the Discussion Some questions or discussions are opened and wait answers of author or other people to be processed label Aug 27, 2018
After discussion with Frederic, the best solution is to remove the test
@ksar-ksar
Copy link
Contributor Author

Following the discussion we have, the best approch will be to remove the test if (! $formconfirm) and to allways call the hook.

@eldy eldy changed the title Update hook formConfirm NEW hook formConfirm always called Sep 1, 2018
@eldy eldy changed the title NEW hook formConfirm always called NEW hook formConfirm always called if hooked Sep 1, 2018
@eldy eldy merged commit c2548b8 into Dolibarr:develop Sep 1, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Discussion Some questions or discussions are opened and wait answers of author or other people to be processed
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants