-
Notifications
You must be signed in to change notification settings - Fork 40
Guard provider submission success callbacks #1363
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
Changes from 2 commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -119,6 +119,19 @@ public function get_form_selectlist() { | |
| * @param array $field_data_array Field data array. | ||
| */ | ||
| public function on_success( $entry, $form_id, $field_data_array ) { | ||
| $status = is_object( $entry ) && isset( $entry->status ) ? $entry->status : ( is_array( $entry ) && isset( $entry['status'] ) ? $entry['status'] : null ); | ||
| if ( null !== $status && 'active' !== $status ) { | ||
| return; | ||
|
Comment on lines
+126
to
+128
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
On installations using Forminator versions whose AGENTS.md reference: AGENTS.md:L428-L432 Useful? React with 👍 / 👎.
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Fixed in a23f680. Official WordPress.org Forminator SVN tags confirm the status property is absent through 1.44.3 and present by 1.48.3. Statusless legacy entries now pass only when is_spam is false and draft_id is empty; explicit non-active statuses remain rejected. Regression coverage now includes legacy success, legacy spam/draft, and current active/failure shapes. |
||
| } | ||
|
|
||
| if ( null === $status ) { | ||
| $is_spam = is_object( $entry ) && isset( $entry->is_spam ) ? $entry->is_spam : ( is_array( $entry ) && isset( $entry['is_spam'] ) ? $entry['is_spam'] : false ); | ||
| $draft_id = is_object( $entry ) && isset( $entry->draft_id ) ? $entry->draft_id : ( is_array( $entry ) && isset( $entry['draft_id'] ) ? $entry['draft_id'] : null ); | ||
| if ( $is_spam || ! empty( $draft_id ) ) { | ||
| return; | ||
| } | ||
| } | ||
|
|
||
| if ( ! $this->should_process_submission() ) { | ||
| return; | ||
| } | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,34 @@ | ||
| describe( 'WS Form success integration', () => { | ||
| let eventName; | ||
| let handler; | ||
| let formSubmission; | ||
|
|
||
| beforeEach( () => { | ||
| jest.resetModules(); | ||
| formSubmission = jest.fn(); | ||
| window.PUM = { integrations: { formSubmission } }; | ||
| window.jQuery = jest.fn( () => ( { | ||
| on: jest.fn( ( registeredEvent, callback ) => { | ||
| eventName = registeredEvent; | ||
| handler = callback; | ||
| } ), | ||
| } ) ); | ||
|
|
||
| require( '../../assets/js/src/integration/wsforms' ); | ||
| } ); | ||
|
|
||
| test( 'observes submit success and never registers save success', () => { | ||
| expect( eventName ).toBe( 'wsf-submit-success' ); | ||
| expect( eventName ).not.toContain( 'wsf-save-success' ); | ||
|
|
||
| const formElement = document.createElement( 'form' ); | ||
| handler( {}, {}, 7, 'instance-1', formElement, {} ); | ||
|
|
||
| expect( formSubmission ).toHaveBeenCalledTimes( 1 ); | ||
| expect( formSubmission ).toHaveBeenCalledWith( expect.anything(), { | ||
| formProvider: 'wsforms', | ||
| formId: 7, | ||
| formInstanceId: 'instance-1', | ||
| } ); | ||
| } ); | ||
| } ); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
When another plugin or a Forminator signature change invokes this hook with
nullor a scalar entry, both status checks fall through to the legacy path, where spam defaults to false and the draft ID to null, so the callback dispatches a submission and may run actions or record a conversion for a value that cannot represent a successful entry. Continue accepting statusless legacy objects, but return unless$entryis an object or supported array before applying the legacy fallback.AGENTS.md reference: AGENTS.md:L428-L432
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fixed in 773a94f. The callback now rejects every non-object/non-array entry before applying legacy status fallback, with null and scalar regression cases. Focused PHP tests and PHPCS remain clean.