Cache clearing is currently broken #15
Conversation
….x-1.x Conflicts: config/install/captcha.settings.yml
| @@ -144,8 +144,7 @@ function captcha_element_process(array $element, FormStateInterface $form_state, | |||
| // This needs to be done even if the CAPTCHA will be ommitted later: | |||
| // other untrusted users should not get a cached page when | |||
| // the current untrusted user can skip the current CAPTCHA. | |||
| global $conf; | |||
| $conf['cache'] = FALSE; | |||
| \Drupal::service('page_cache_kill_switch')->trigger(); | |||
There was a problem hiding this comment.
hass has a good point. Let's try and move this logic to the separate plugins (test, image and math), all of those should include this I think.
There was a problem hiding this comment.
Also, I think the comment there is confusing and incorrect (I don't think some anon users can be trusted and others not?
We also need a max-age 0 in the render array structure, so that it's not getting cached inside a block for example. We probably want to have an explicit tests for a user login block with the math or image plugin, visiting it multiple times should a) never get cached, and b) result in a different output.
… max-age with value 0
|
Implemented some feedback, test is failing currently because of different behavior of captcha and image_captcha. |
|
The test still fails because image_captcha doesn't behave as expected. Might be a real problem with image_captcha. |
|
Alright, the latest version. Tests are working, caching too, the .travis.yml also works. Feedback is welcome, nonetheless 😄 . Edit: The change in image_captcha/src/Controller/CaptchaImageGeneratorController.php was a separate issue which @Berdir found and patched, I added a test for it (drupalGet on the same image path gave an error about db_query being an unknown function). |
Changed a comment to indicate which class exactly it was testing
|
|
||
| $this->adminUser = $this->drupalCreateUser(array('administer blocks')); | ||
| $this->drupalLogin($this->adminUser); | ||
| $this->drupalPlaceBlock('user_login_block', array('id' => 'login')); |
There was a problem hiding this comment.
drupalPlaceBlock() uses the API, there is no need to create a user for this.
Used a more specific string for the xpath query.
… cacheClearFix Conflicts: src/Tests/CaptchaCacheTestCase.php
|
Alright, should also pass the travis test now. That's what I get for using the online editor 😛 |
|
|
||
| public static $modules = ['block', 'image_captcha']; | ||
|
|
||
| function setUp() { |
There was a problem hiding this comment.
Missing docblock for the module list and setUp().
Cache clearing is currently broken
The test isn't sufficient to see this. But it's easy to check when you add some lines to the current test:
CaptchaAdminTestCase.php
Insert after Line 254
This pull request adds the test above, fixes the caching by using the appropriate service method. The other pull request was based on a wrong 8.x-1.x branch on my side, sorry about that.