Posted by gotheric on June 18, 2008 at 12:20pm
| Project: | Mailhandler |
| Version: | 7.x-1.x-dev |
| Component: | Code |
| Category: | bug report |
| Priority: | normal |
| Assigned: | Unassigned |
| Status: | closed (fixed) |
Issue Summary
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?
Comments
#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.
#12
#180063: No way to flush form errors during iterative programatic form submission has been committed to D6. we can undo the mess in mailhandler_node_submit() now.
#13
@Moshe: How? Is it by calling form_set_error with ($reset = TRUE) after node_validate?
Can you show me the way? (I'll walk it :-) )
#14
Yes, thats the idea
Email me when you have a need because I am rarely reading mailhandler issue mail these days.
#15
It looks like this can all boil down to:
// Reset the static cacheform_set_error(NULL, '', TRUE);
node_validate($node);
$error_messages = form_set_error();
I am working on a 6.2 version which will hopefully be ready enough to commit back to CVS and I've tested the above in it which works. I tested by sending three emails, the first two with validation errors and the last without, and the last one passed validation fine while the first two were correctly found invalid. I'll likely commit this after another point release in 6.1.
#16
Attached is the patch per #15
#17
Applied to 6.1. Should be ported to 7.1
#18
Set to HEAD so the port is not forgotten, also since work on a 7.1 is unlikely as I understood from other issues
#19
Bump, unclear if patch is integrated. If so plz close this issue, if not it would be lovely to have it integrated in main.
#20
#21
Yes, this has been committed to 7.x-1.x.
#22
Automatically closed -- issue fixed for 2 weeks with no activity.