Hi guys,

I have developed a small module for drupal 7 Image Captcha Refresh , I guess will be a good idea include this module into the CAPTCHA module as additional option, which will provide users use it from the box. If you are agree with this idea, I will prepare a patch which will include needed features. Also I can help with fixing bugs after that.

Thanks.

Support from Acquia helps fund testing for Drupal Acquia logo

Comments

DmitryDrozdik created an issue. See original summary.

svipsa’s picture

Hey, Cool idea. Let's do it!

afi13’s picture

This makes sense. I'd be happy to review and test any patches made for this functionality.

rsvelko’s picture

+1 for this feature.
The https://www.drupal.org/project/image_captcha_refresh module has already usage of 7435 and 0 open bugs. The code is small and seems easy to review and merge into captcha.

So +1 for this.

rsvelko’s picture

@DmitryDrozdik: when creating the new merge patch :
- maybe u will need to stop using form_alter and put the form api code into the normal captcha module's form generator function
- when its a separate module it maybe checks if captcha module is installed - now when u merge it - remove that check cause it will be the captcha module checking itself if enabled...

seorusus’s picture

I encourage it

alexander.ilivanov’s picture

+1 for that.

anpolimus’s picture

+1

ddrozdik’s picture

iVanilla’s picture

Hi.
Could you develop a version of Drupal 8?
Thanks.

AswathyAjish’s picture

Please develop a D8 version of this.

ddrozdik’s picture

ok. I am working on this.

RavindraSingh’s picture

Have you developed some dev version of this module in D8? If yes, I can contribute in that with you.

RavindraSingh’s picture

I had ported this module in Drupal 8. but as i had a discussion with @ddrozdik, He suggested to add the feature to captcha module. So here is the first working image refresh patch with Captcha 8.x-1.x branch.

This works good.

I am waiting for the captcha module maintainer if they approve this feature should be a CAPTCHA, after that I am happy to improve the below mentioned tasks -

<ul>
  <li>basePath setup in JS to call refresh route.</li>
  <li>Removing theme hook from .module file. better to use link in image_captcha_captcha for captcha_refresh #this is important if this works we can remove and other functions ported from the original module.</li>
  <li>Removing hook_element_info_alter</li>
  <li>Removing image_captcha_element_info_alter</li>
  <li>Add tests</li>
</ul>
RavindraSingh’s picture

Status: Active » Needs review
RavindraSingh’s picture

For #5, We are not using form alter in new patch. I am trying to fix this from image_captcha module only.

Status: Needs review » Needs work

The last submitted patch, 14: image_captcha_refresh.patch, failed testing.

naveenvalecha’s picture

That's the nice feature. +1 to be included into captcha.
Here are some more nits other than mentioned in #14

  1. +++ b/image_captcha/image_captcha.libraries.yml
    @@ -10,3 +11,8 @@ base:
    +image-captcha-refresh:
    

    This library is using the jquery so make it dependent on the core/jquery

  2. +++ b/image_captcha/image_captcha.libraries.yml
    @@ -10,3 +11,8 @@ base:
    \ No newline at end of file
    

    Add new line at EOF

  3. +++ b/image_captcha/image_captcha.module
    @@ -260,3 +260,53 @@ function image_captcha_captcha($op, $captcha_type = '', $captcha_sid = NULL) {
    +use Drupal\Core\Url;
    

    Move this use statement at top ofthe file.

  4. +++ b/image_captcha/image_captcha.module
    @@ -260,3 +260,53 @@ function image_captcha_captcha($op, $captcha_type = '', $captcha_sid = NULL) {
    +      $uri = \Drupal::l(t('Get new captcha!'), ¶
    

    Remove the deprecated function.

  5. +++ b/image_captcha/image_captcha.module
    @@ -260,3 +260,53 @@ function image_captcha_captcha($op, $captcha_type = '', $captcha_sid = NULL) {
    +      ¶
    

    Extra spacing.

  6. +++ b/image_captcha/image_captcha.module
    @@ -260,3 +260,53 @@ function image_captcha_captcha($op, $captcha_type = '', $captcha_sid = NULL) {
    \ No newline at end of file
    

    Add new line at EOF

  7. +++ b/image_captcha/image_captcha.routing.yml
    @@ -18,3 +18,10 @@ image_captcha.generator:
    \ No newline at end of file
    

    Add new line at EOF

  8. +++ b/image_captcha/js/image_captcha_refresh.js
    @@ -0,0 +1,45 @@
    +        var url = baseUrl + '/' + $(this).attr('href') + '?' + date.getTime();
    

    We need a security token here to prevent the CSRF

  9. +++ b/image_captcha/src/Controller/CaptchaImageRefresh.php
    @@ -0,0 +1,69 @@
    + *
    + * ¶
    

    Remove extra lines

  10. +++ b/image_captcha/src/Controller/CaptchaImageRefresh.php
    @@ -0,0 +1,69 @@
    +    //put your code here
    

    Make the comment more explanatory.

  11. +++ b/image_captcha/src/Controller/CaptchaImageRefresh.php
    @@ -0,0 +1,69 @@
    +      $connection = Database::getConnection();
    

    Inject the database connection into controller.

  12. +++ b/image_captcha/src/Controller/CaptchaImageRefresh.php
    @@ -0,0 +1,69 @@
    +        'url' => \Drupal::url('image_captcha.generator', ['session_id' => $captcha_sid, 'timestamp' => REQUEST_TIME]),
    

    use the Url component class instead.

RavindraSingh’s picture

Thank you @naveenvalecha, As mentioned in my last comment I will improve these issue after only if CAPTCHA module maintainer agrees on adding the feature into Drupal 8.

RavindraSingh’s picture

+++ b/image_captcha/js/image_captcha_refresh.js
@@ -0,0 +1,45 @@
+        var url = baseUrl + '/' + $(this).attr('href') + '?' + date.getTime();
We need a security token here to prevent the CSRF

Will pass the baseURL from Drupal setting.

naveenvalecha’s picture

Component: Code » Image Captcha (image_captcha)

I will improve these issue after only if CAPTCHA module maintainer agrees on adding the feature into Drupal 8.

As a co-maintainer of the module. +1 from my end. pinged to wundo & elachlan about the same. Let's improve it and get this dang in.

wundo’s picture

I don't have time to work on this right now, but I'd love to have this committed to both D7 and D8 branches.
Assuming we have a good test coverage for this feature, +1.

Also, I'd make this part of the API itself, not only Image Captcha. I know some challenges implement their on "refresh" (e.g. Recaptcha), but it would be great to have this feature as challenge agnostic as possible (and allow individual challenges to suppress the standard behaviour)

wundo’s picture

ddrozdik’s picture

Issue tags: +feature request

I agree that make sense to do this as part of API. I will try to find some time to do this.
Regarding D7 branch, my module is used on many sites and migration this functionality into Captcha module will make lot's of regressions.

elachlan’s picture

I like the idea of it. I think instead of a button we should use some sort of an icon. It avoids need for translation.

bpresles’s picture

Status: Needs work » Needs review
FileSize
7.61 KB

Find attached an updated version of the patch

Status: Needs review » Needs work

The last submitted patch, 26: image_captcha_refresh-2608540-26.patch, failed testing. View results

bpresles’s picture

I didn't manage to run the unit test on my dev environment (for an unknown reason, there is no "captcha" group listed by phpunit (while all my other contrib modules with tests are listed). So I try posting a possible fix and we'll see if the test pass.

sakural’s picture

Title: Add Image CAPTCHA Refresh to the module » Add Image CAPTCHA Refresh to the module patch
FileSize
7.49 KB

I find these patch i can't use,
it return error that i can't apply normally.
so i create one, matbe it can help someoneelse.
thanks.

gg24’s picture

Assigned: ddrozdik » Unassigned
Status: Needs work » Needs review
FileSize
10.39 KB

Hi,

I have re-rolled the patch for 8.x-1.x and I have resolved the syntax issues as well. Please review the patch.

Thanks!

Status: Needs review » Needs work

The last submitted patch, 30: image_captcha_refresh-2608540-30.patch, failed testing. View results
- codesniffer_fixes.patch Interdiff of automated coding standards fixes only.

gg24’s picture

Status: Needs work » Needs review
navneet0693’s picture

+++ b/image_captcha/js/image_captcha_refresh.js
@@ -0,0 +1,45 @@
\ No newline at end of file

+++ b/image_captcha/templates/image-captcha-refresh.html.twig
@@ -0,0 +1,19 @@
\ No newline at end of file

New lines :-)

Little of more adjustments will be required in image_captcha.libraries.yml for tests to pass. 'all' category doesn't exists

 css:
    all:
      image_captcha.css: {}

to

 css:
    theme:
      image_captcha.css: {}
navneet0693’s picture

Status: Needs review » Needs work
gg24’s picture

Made changes as per suggestion above. Please review the patch.

Thanks!

navneet0693’s picture

Status: Needs review » Needs work
  1. +++ b/image_captcha/src/Controller/CaptchaImageRefresh.php
    @@ -0,0 +1,65 @@
    +/*
    + * To change this license header, choose License Headers in Project Properties.
    

    Finding it little bit irrelevant to context of class

  2. +++ b/image_captcha/src/Controller/CaptchaImageRefresh.php
    @@ -0,0 +1,65 @@
    +    catch (Exception $e) {
    

    Exception class not declared above. or you can use \Exception

  3. +++ b/image_captcha/src/Controller/CaptchaImageRefresh.php
    @@ -0,0 +1,65 @@
    +        $result['message'] = t('Error has occurred. Please contact to site administrator.');
    

    Using StringTranslationTait, $this->t()

++ to all who worked upon this.

gg24’s picture

Status: Needs work » Needs review
FileSize
10.39 KB

Hi @navneet0693,

I have made the suggested changes. Please review the patch attached.

Thanks!

navneet0693’s picture

Status: Needs review » Reviewed & tested by the community

This should be all, good to go!

  • wundo committed 4a08f67 on 8.x-1.x authored by gg24
    Issue #2608540 by gg24, bpresles, RavindraSingh, sakural, ddrozdik,...
wundo’s picture

Status: Reviewed & tested by the community » Fixed
Ice-D’s picture

Status: Fixed » Needs work

Updated the module via composer, now I'm getting

ReflectionException: Class \Drupal\image_captcha\Controller\CaptchaImageRefresh does not exist in ReflectionMethod->__construct() (line 123 of core/lib/Drupal/Core/Entity/EntityResolverManager.php

There indeed is no Controller of that name. But it's mentioned in image_captcha.settings.yml. What happened?

GreenSkunk’s picture

The patch does contain the controller BUT I can not get the patch to cleanly apply.
I've added the patch via composer and directly and both fail against the current dev.

The patch does work to add the CaptchaImageRefresh controller

Is anyone else having this problem?

Ice-D’s picture

Yes, I couldn't apply the patch either. Looking at the commit in #39, the CaptchaImageRefresh controller and a bunch of other related files seem to be missing. It would be great if one of the maintainers could take a quick look.

elachlan’s picture

Priority: Normal » Major

This broke the tests.

  • elachlan committed 6aa25be on 8.x-1.x authored by gg24
    Issue #2608540 by gg24, bpresles, RavindraSingh, sakural, ddrozdik,...
elachlan’s picture

Status: Needs work » Fixed

Tests are now passing.

Status: Fixed » Closed (fixed)

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

Abhinaw’s picture

Hi All, hope doing good. DO we have image_captcha Refresh modules ?

Abhinaw’s picture

DAMIANC _’s picture

Issue tags: -