[#37540] - Allow onValidateContact plugin to show error message
- Fixed in Code Base
- 30 Aug 2023
- Medium
- Build: 4.1-dev
- # 37540
- beefcakefu:patch-1
Pull Request for Issue #37584 .
Summary of Changes
This pull request copies actions performed on contact form submissions that fail native form validation, and applies it to submissions that fail onValidateContact plugins.
Testing Instructions
This dummy plugin always fail contact form submissions and should demonstrate the issue:
<?php
defined('_JEXEC') or die;
class plgContactDummyValidation extends JPlugin {
protected $autoloadLanguage = true;
function onValidateContact($contact, $data) {
return new InvalidArgumentException("Exception message is not shown.");
}
}
- Install dummy plugin above.
(System > Manage > Extensions > Install Extensions > Upload Package File) - Enable dummy plugin.
(System > Manage > Plugins > Contact - Dummy Validation > Enable plugin) - Create a contact form in admin panel.
(Menus > Main Menu > Add Site Menu Item > Menu Item Type > Contacts > Single Contact > Select Contact [create if there isn't already one], and give it a title) - Set up valid mail credentials if not already present.
(System > Setup > Global Configuration > Server > Mail) - Fill and submit contact form on the public site. Observe how user is redirected to a blank page.
- Apply patch.
- Fill and submit contact form again. Observe how user is redirected to contact form pre-filled with previously entered values, with exception message in an alert banner which plugins can use to explain why the submission was unsuccessful.
Actual result BEFORE applying this Pull Request
If user submits a contact form with parameters failing an onValidateContact triggered plugin, plugin returns an exception, and user is redirected to a blank contact form page, stripped of all form elements with no error message indicating why the submission was unsuccessful.
Expected result AFTER applying this Pull Request
User is redirected to contact form page, pre-filled with submitted values and displaying exception message explanation of why submission failed. Same as if submission had failed native form validation.
Documentation Changes Required
None.
joomla-cms-bot
- | Category | ⇒ | Front End com_contact |
| Labels |
Added:
?
|
||
@beefcakefu I've noticed you have just used the "Update branch" button because the branch was shown as not up to date. That's not a problem. But it is also not necessary as long as there are not shown conflicting files in addition. So I just wanted to let you know it's not necessary to always do that.
Regarding your review request: Code style looks ok, but for a deeper review I don't have the time now.
Thanks @richard67! As you know, I'm very new to this, and your guidance is much appreciated.
@richard67 Sorry to bug you again, but it looks like this PR has stalled. I hope I don't come off sounding demanding, because that's not my intention, and I totally understand everyone here is volunteering their free time for the Joomla! project.
I was hoping you could enlightening me about the typical lifecycle of a PR. At this point, I've addressed 2 reviews by yourself and @PhilETaylor. Does something need to happen from you guys, like to acknowledge that your feedbacks were satisfactorily resolved? Or do we just wait for 2 human testers to confirm the PR works and we're ready to commit?
I've actually authored a plugin to validate that enquirers' email addresses are working before allowing a contact form submission. It opens a connection to the enquirer's SMTP server and checks that the mailbox exists and isn't full and everything. I think it'll be a great plugin to combat prolific spammers like eric.jones.z.mail@gmail.com. I hope to release the plugin as open source, and the only thing stopping me from putting it on the Joomla! Extensions Directory is that exception messages currently cannot be shown to an enquirer without this PR.
This PR and my plugin also work in Joomla! 3.x. What do I have to do so this PR is also committed to the 3.x branch?
Thank you for your attention, and your patience when I opened the unnecessary Issue, and teaching me it's not necessary to always "update branch" everytime it shows not up to date.
The Joomla project is well known for leaving PR contributions open as long as 5+ years… so 22 days open for this PR is nothing. If a PR is not tested and merged within 3 days then it normally stalls for months or even years. This is not opinion, this is fact. You can do no more except wait for someone to test your PR and for a maintainer to accept or reject it.
Or do we just wait for 2 human testers to confirm the PR works and we're ready to commit?
@beefcakefu That's the case.
What do I have to do so this PR is also committed to the 3.x branch?
You would have to make a PR for the 3.10-dev branch and mention this PR here in the description of the other PR so it's clear they do the same.
But let's ask the 3.10 release lead @zero-24 first if this is a bug fix which he would like to have in 3.10.
Thank you both for your input! I'll tidy up the code and share my plugin on the Joomla! Forum and see if anyone adventurous wants to take it for a test drive together with this PR (and pray they also have a GitHub account to mark it as working on the issue tracker).
If a PR is not tested and merged within 3 days then it normally stalls for months or even years.
I've actually suspected that might be the case after skimming through the issue tracker. Since making this PR, I've also set up a test environment and shared my first test result. I intend to start contributing as a patch tester and hopefully make a difference in this unclosed PR contribution situation.
I intend to start contributing as a patch tester and hopefully make a difference in this unclosed PR contribution situation.
@beefcakefu That would indeed be a good help because we are so few people doing that. Thanks in advance.
Hey @beefcakefu very cool addition
I moved it to the 4.2 branch, as it's not really a bug fix but an enhancement.
So it just needs now two valid tests and we're good to go.
I moved it to the 4.2 branch, as it's not really a bug fix but an enhancement.
Which also means that it wont be accepted for j3
Unable to report failed test result from Joomla 4.2.0-alpha3-dev of 6 May. Is that caused by the out-of-date message "This branch is out-of-date with the base branch"?
This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/37540.
| Labels |
Added:
?
Maintainers Checked
Removed: ? |
||
Unable to report failed test result from Joomla 4.2.0-alpha3-dev of 6 May. Is that caused by the out-of-date message "This branch is out-of-date with the base branch"?
This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/37540.
I think that was it! Have updated branch and it's now passing. Thanks toivo!
Unable to report failed test result from Joomla 4.2.0-alpha3-dev of 6 May. Is that caused by the out-of-date message "This branch is out-of-date with the base branch"?
@beefcakefu What @toivo means is that he can't use the "Test this" button in the issue tracker here https://issues.joomla.org/tracker/joomla-cms/37540 to mark his test result.
@toivo That's a but which the issue tracker sometimes has. For some reason it did not see that this PR is a PR and not an issue, and so the status is "New" and not "Pending" in the issue tracker, and the button is missing. What you can do is you can report back your test result in a normal comment and provide some information about what has failed so @beefcakefu can fix his PR if necessary.
@beefcakefu What @toivo means is that he can't use the "Test this" button in the issue tracker here https://issues.joomla.org/tracker/joomla-cms/37540 to mark his test result.
I see.
Thank you for taking the time to test this PR, @toivo. Where are you running into problems?
@bembelimen I don't see the "Test" button in the Joomla! Issue Tracker?
IMO this can be considered a bugfix (entered data is kept on Exception that a 3rd party extension could trigger) and should also be added to Joomla! 3!
@Kubik-Rubik If you would scroll a bit up you would find my comment regarding the issue tracker problem.
This pull requests has been automatically converted to the PSR-12 coding standard.
This pull request has been automatically rebased to 4.3-dev.
Tested successfully on 4.3.5-dev
This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/37540.
Tested successfully on 4.3.5-dev
This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/37540.
| Status | New | ⇒ | Ready to Commit |
RTC (Tests where added as comments as issue tracker had issues)
This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/37540.
| Labels |
Added:
?
PR-4.3-dev
Removed: ? |
||
| Status | Ready to Commit | ⇒ | Fixed in Code Base |
| Closed_Date | 0000-00-00 00:00:00 | ⇒ | 2023-08-30 16:11:16 |
| Closed_By | ⇒ | obuisard | |
| Labels |
Added:
bug
|
||
Thank you @beefcakefu for this PR.
I consider it a long lasting bug fix and took the responsibility to merge it.
@beefcakefu I've noticed you have just used the "Update branch" button because the branch was shown as not up to date. That's not a problem. But it is also not necessary as long as there are not shown conflicting files in addition. So I just wanted to let you know it's not necessary to always do that.
Regarding your review request: Code style looks ok, but for a deeper review I don't have the time now.