Skip to content
This repository has been archived by the owner on Jan 30, 2020. It is now read-only.

Support Recaptcha APIv2 #139

Merged
merged 4 commits into from
Feb 23, 2017
Merged

Support Recaptcha APIv2 #139

merged 4 commits into from
Feb 23, 2017

Conversation

akrabat
Copy link
Contributor

@akrabat akrabat commented Feb 20, 2017

Update to use zend-capchta:2.7.0 which require zendservice-recaptcha:3.0.0

From zend-form's point of view, we no longer need a hidden field or the JS. As a result, renderHiddenInput() and renderJsEvents() are now just stubbed out and marked deprecated.

ZendService_ReCaptcha supports v2 of the API which renders differently.
@@ -35,7 +35,7 @@
"zendframework/zend-text": "^2.6",
"zendframework/zend-validator": "^2.6",
"zendframework/zend-view": "^2.6.2",
"zendframework/zendservice-recaptcha": "*",
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Wow, didn't know we still had the death-star-constraint O_O

@akrabat akrabat added this to the 2.10.0 milestone Feb 20, 2017
Recaptcha API v2 generates much simpler HTML than API v1 did. Hence we
can simplify the tests.
@froschdesign
Copy link
Member

LGTM

ReCaptcha automatically creates a hidden element called
`g-recaptcha-response` for its response. If this element is not called
that, then the required validation will fail unless we render a
hidden element.
Copy link
Member

@weierophinney weierophinney left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks fine to me. Only comment is that we may be able to remove a method that is now a no-op; I'm just not sure it it gets exercised from something else?

* Create the JS events used to bind the challenge and response values to the submitted form.
* No longer used with v2 of Recaptcha API
*
* @deprecated
*
* @param string $challengeId
* @param string $responseId
* @return string
*/
protected function renderJsEvents($challengeId, $responseId)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm wondering if we could just remove this entirely?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

My thought was that someone may have overridden & called the parent

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@akrabat Good point — as it's protected, likely should be kept. sigh

@akrabat akrabat merged commit 75cdda0 into zendframework:develop Feb 23, 2017
akrabat added a commit that referenced this pull request Feb 23, 2017
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants