Error in validating multiple mails
gotheric - June 18, 2008 - 12:20
| Project: | Mailhandler |
| Version: | 5.x-1.2 |
| Component: | Code |
| Category: | bug report |
| Priority: | critical |
| Assigned: | Unassigned |
| Status: | closed |
Description
When mailhandler fetches more than one mail, and validate them, if a mail reports a validation error the same error will be "duplicated" to all messages after that.
The problem in in mailhandler_node_submit function, where it does:
node_validate($node);
$error = form_get_errors();It does the 2 lines above for each mail, but nowhere there is a reset of form_errors, so after a validation for a message reports an error (via form_set_error), that error message will remain for all remaining mails.
The GREAT problem is that i can see no way to reset form_errors...
The only way seems to patch form.inc function "form_set_error" to implements a reset of the static $form variable.
What do you think?
#1
Perhaps we need to submit the node using drupal_execute()? Would that still do all our validation? Would it reset errors? Ugh.
#2
No, i don't think that will solve the problem.
The core doesn't support multiple validations, and i don't think of a "clean" way to do that.
(Or maybe i'm missing something...)
However i submitted a core support request:
#272042: There is no way to reset validation errors
#3
Assuming the problem won't be fixed on 5.x (and i think 6.x too) i see only one way to fix the problem without patching the core.
It's not a "smart" way but it should work.
We must work with error messages set by "drupal_set_message" (form_set_error calls it for each validation error, and drupal_set_message data is contained on $_SESSION['messages'], which is visibile outside the function).
This should be something like that (this is just a quick example, if you think it should be a good way i'll do the patch):
//Before mail loop, i save error messages BEFORE mail validation
$saved_errors = $_SESSION['messages']['error'];
....
// Inside the loop
$_SESSION['messages']['error'] = array();
node_validate($node);
$validation_errors = array();
if (count($_SESSION['messages']['error'])) {
$errors = form_get_errors();
foreach ($_SESSION['messages']['error'] as $message) {
if ($key = array_search($message, $errors)) {
// This is a validation error
$validation_errors[$key] = $message;
} else {
// Not a validation error (but an error, i'll print it)
$saved_errors[] = $message;
}
}
}
// Now in $validation_errors we have the real errors
...
// After the loop, i restore real error messages
$_SESSION['messages']['error'] = $saved_errors;
Only an example that should be tuned (i should also catch errors on "..." code...).
Could that be a solution (hoping and waiting for a real fix on 7.x)?
#4
Yes, that looks like a clever solution. I'd welcome such a patch. Please add a comment so we know why we have to do this madness.
For 7.x, please help on a related patch in. See #254491: Standardize static caching
#5
Sounds interesting, i'll look at it.
I'll check & submit mailhandler patch next week (monday or tuesday).
#6
Here is the patch.
(I think it should be applied easily to 6.x branch too).
#7
I've committed this to 5.x and 6.x. I'd love some feedback in the next few days. If I hear no negative feedback, I will make new releases. I am not setup for testing this now, so I've committed blindly. Please give feedback, folks.
#8
Just a little mod, to avoid having the $_SESSION['messages']['error'] with an empty array (standard theme will display an empty error box).
#9
Committed. Please use braces even on 1 line if() statement. Just part of our coding standards.
I am going to make a new release of mailhandler since I can't seem to get any feedback otherwise.
#10
I've just been setting mailhandler up for the first time, had it checking the inbox but no content was being created. Since I was adding more and more unread variations of the same message with different versions of the commands, I'd obviously ended up triggering this bug, new release of mailhandler fixed it and has probably saved me hours and hours of head scratching. Thanks!
#11
Automatically closed -- issue fixed for two weeks with no activity.