Problem/Motivation

The Webform Remote POST handler provides great coverage for processing error messages, but what about success messages? It appears that the success message is only covered by the webform confirmation settings, which appear to always execute regardless of the success/failure of the handler.

Proposed resolution

Expand current functionality to not be "error-only" and also include successful POST scenarios, especially when the handler is the primary processor for the webform.

Remaining tasks

1. Discuss options
2. Determine next steps
3. Add tests and submit for review

User interface changes

Slight changes to config form field labels

API changes

N/A

Data model changes

N/A

Support from Acquia helps fund testing for Drupal Acquia logo

Comments

bighappyface created an issue. See original summary.

bighappyface’s picture

Attached is a quick patch to apply the custom error message by status code handling to successful response codes too. No tests, but I want to get the conversation started.

jrockowitz’s picture

Status: Active » Needs review

@bighappyface Your request and patch make a lot of sense and it probably just needs test coverage.

jrockowitz’s picture

The safest approach is to provide support custom 2xx response messages but do not display the default custom error message for 2xx response. My concern is someone might have configured a custom response error message which would now be displayed for all 2xx responses. For most 2xx response the default confirmation message should be fine.

bighappyface’s picture

Issue summary: View changes
FileSize
6.16 KB

@jrockowitz thanks for the feedback! Please see the updated patch. NVM - I forgot to refresh the page before posting this comment/patch.

bighappyface’s picture

Status: Needs review » Reviewed & tested by the community

+1 for #4

@jrockowitz I applied the patch locally and tested in context for my needs. Works great! What do you need to commit this enhancement?

bighappyface’s picture

bighappyface’s picture

+++ b/src/Plugin/WebformHandler/RemotePostWebformHandler.php
@@ -1036,18 +1041,7 @@ class RemotePostWebformHandler extends WebformHandlerBase {
-      $this->messenger()->addError(\Drupal::service('renderer')->renderPlain($build_message));

@jrockowitz are we good losing the `addError` here with the relocation to `displayCustomResponseMessage` where it becomes `addMessage`?

Everything comes up as a status message. Perhaps we can check for that in `displayCustomResponseMessage` to switch between status/error?

jrockowitz’s picture

Status: Reviewed & tested by the community » Needs work

2xx response should be status messages and everything else considered an error message.

bighappyface’s picture

Status: Needs work » Needs review
FileSize
13.39 KB
3.84 KB

@jrockowitz I have taken a stab at handling the status/error message states. Please see the attached patch and interdiff.

jrockowitz’s picture

I think this is the right solution. Attached is a minor improvement.

bighappyface’s picture

Status: Needs review » Reviewed & tested by the community

@jrockowitz #11 LGTM. I tested it out manually, and everything is good to go.

May we merge this?

Thank you so much for your help.

jrockowitz’s picture

Status: Reviewed & tested by the community » Fixed
bighappyface’s picture

bighappyface’s picture

#11 Patch Re-reroll

jrockowitz’s picture

@bighappyface The patch seemed fine to me so I committed it.

bighappyface’s picture

@jrockowitz sorry to bug - you committed a patch for this issue? I only see the commit for #3135006: Webform Handler Token String Conversion Notice in the commit log on 8.x-5.x

  • bighappyface authored 25b7bb9 on 8.x-5.x
    Issue #3134549 by bighappyface, jrockowitz: Success Messages For Remote...
jrockowitz’s picture

@bighappyface You are right I somehow messed up the commit. I just committed your patch. Thanks.

bighappyface’s picture

Status: Fixed » Closed (fixed)

Automatically closed - issue fixed for 2 weeks with no activity.