Skip to content

Commit

Permalink
Fix: Issue where a SQL-related server error is thrown when loading ne…
Browse files Browse the repository at this point in the history
…w user creation form

# Fixing the form for new user creation (#864)

## Changes to the database schema

n/a

## Changes to the model and controller classes

* ``protected/models/User.php``
* ``protected/controller/UserController.php``

The code handling the form was not handling Yii model scenarios causing issue with the password field.
Correct handling requires the controller to specify upon instanciaiton of a new model, a scenario that matches the set of attributes that can be altered on that model for the given scenario (the validation method call should also take that scenario as input that was already the case) . The model needs a change too to indicate in which scenario it is safe to alter 
the password attribute.

Additional change to the controller involves wrapping a buggy email sending feature inside a try/catch to have the error logged instead of crashing the form submission and leaking stack trace to the user.
 
>Note: Fixing the problem with email sending is not in the scope of this PR

A further change to the controller is to add a test to detect whether the code is running on a ``dev`` and ``CI`` environment, in which case we set the captcha session variable to whatever is passed in the form so automated test can pass captcha verification while avoiding to mess up with the backend validation code.

>Note: Fixing the big problem with the captcha is not in the scope of this PR.

Finally, a minor issue (revealed when writing functional test) was fixed to ensure the generated image content is in PNG format.

## Changes to the view template files

* ``protected/views/user/_form.php``

The root cause of the problem raised in #864 lies in this file. The feature for linking author record and user record only make sense for a user that exists, so it shouldn't be active on  the new user creation form. I've fixed the code so the worklflow is not invoked for new user creation


## Changes to the style

n/a

## Changes to the tests

Installed Codeception for the main codebase, so that we can start reducing dependency on legacy testing libraries that prevent us from migrating PHP versions (in order to improve peformances and mitigate the security vulnerabilities uncovered by container scanning and SAST tools):

* Installed Codeception (see ``ops/configuration/php-conf/composer.json.dist`` and ``composer.lock``)
* Configured Codeception for acceptance tests (see ``tests/acceptance.suite.yml``)
* Configured Codeception for functional tests (see ``tests/functional.suite.yml``)
* Implemented the steps of the feature file scenario in ``tests/_support/AcceptanceTester.php``
* Added Codeception acceptance tests run command to ``tests/acceptance_runner``

Created and implemented two acceptance test scenarios to test the new user creation process:

* Added a feature file for new user creation: ``tests/acceptance/NewUser.feature``
* Added required generic steps in ``tests/_support/AcceptanceTester.php``
* Added required steps specific to ``asa:Curator`` user story in ``tests/_support/CuratorSteps.php``

Change to functional tests:

Replaced coverage-incomplete functional test for captcha (removed ``protected/tests/functional/CaptchaImageTest.php``)
with new Codeception functional tests (in ``tests/functional/CaptchaCest.php``) that cover more and relevant behaviours of our captchas usage:

* the tests cover both captcha usage, the one on contact form and the one on user form.
* we test that the generated captcha is different between successive call
* no need to test for the image size specific to each form, as captcha image presentation
should be the same across usage. We are not harmonising sizes in this task, it will be done later when we fix the big captcha bug
* we test the captcha is image/png mime which revealed a minor bug in User form controller (discrepancy between file format and generated format) which we fix here (see controller changes above)

That functional test refactoring is made more necessary now that acceptance tests scenario for user creation form is bypassing captcha using server variables and environment detection as we need to make sure captcha mechanism has test coverage.


The new functional tests called for the creation of a helper function in ``tests/_support/Helper/Functional.php`` to help with comparing two images.

## Changes to the provisioning

n/a
  • Loading branch information
rija authored Dec 14, 2021
2 parents 109d5ef + 982c7e5 commit 0c0c6ce
Show file tree
Hide file tree
Showing 27 changed files with 1,386 additions and 63 deletions.
10 changes: 10 additions & 0 deletions codeception.yml
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
paths:
tests: tests
output: tests/_output
data: tests/_data
support: tests/_support
envs: tests/_envs
actor_suffix: Tester
extensions:
enabled:
- Codeception\Extension\RunFailed
Loading

0 comments on commit 0c0c6ce

Please sign in to comment.