Adding fix for Role feature state check #2851
Conversation
@willdoran if you use the |
src/App/Validator/Role/Update.php
Outdated
| @@ -36,6 +38,15 @@ protected function getRules() | |||
| ]; | |||
| } | |||
|
|
|||
| public function checkRolesEnabled(\Kohana\Validation\Validation $validation, $permissions) | |||
There was a problem hiding this comment.
Couple of things:
- Don't you need to add this into
getRulesabove? - Should updates fail entirely? or should we be able to rename roles in the free tier?
There was a problem hiding this comment.
Just realised this is going back to the lumen branch. You need to change the interface to rely on Ushahidi\Core\Tool\ValidationEngine instead. It should work as written otherwise
There was a problem hiding this comment.
Should updates fail entirely? or should we be able to rename roles in the free tier?
nope no changes to roles at all allowed
Don't you need to add this into getRules above?
yep
There was a problem hiding this comment.
You need to change the interface to rely on Ushahidi\Core\Tool\ValidationEngine instead.
Can you link an example from the other validators, not sure I follow.
|
@rjmackay Yeah I think that will work, will add it. |
#ush022 - check that checkRolesEnabled returns what we expect
|
@rjmackay could you peak at this again when you have a second? |
| */ | ||
| class RoleUpdateTest extends TestCase | ||
| { | ||
|
|
There was a problem hiding this comment.
Ideally we should add a test that actually calls Update::check() with some data too.
Notes
I'm not certain how to test this in the integration tests since we can't alter the features via request and I don't immediately see a way to do it from the tests.
This pull request makes the following changes:
Test checklist:
With Roles feature enabled set to false
Attempt to create/update a new role via HTTP Post
You should receive a 422
I certify that I ran my checklist
Fixes #1617
Ping @ushahidi/platform