application and school configuration #1736
Conversation
There was a problem hiding this comment.
Additional stuff:
-
Add configurations to the tests at https://github.com/ilios/ilios/blob/master/tests/CoreBundle/Entity/SchoolTest.php#L41
-
Needs an API version bump in https://github.com/ilios/ilios/blob/master/src/Ilios/WebBundle/Service/WebIndexFromJson.php#L14
| case self::DELETE: | ||
| // grant CREATE, EDIT and DELETE privileges | ||
| // if the user has the 'Developer' role | ||
| return $this->userHasRole($user, ['Developer']); |
There was a problem hiding this comment.
Should they also be in the school? Along with being a developer.
There was a problem hiding this comment.
this is correct as-is, following the same pattern as vocabularies. unless you want to enforce more narrow/school-scoped permissions on the frontend as well.
| throw $this->createAccessDeniedException('Unauthorized access!'); | ||
| } | ||
|
|
||
| $answer['application_configs'][] = $applicationConfig; |
There was a problem hiding this comment.
We don't underscore anywhere else. Also it makes ember-data sad and we have to write custom adapters. Can we use applicationConfigs here?
| }); | ||
|
|
||
| //If there are no matches return an empty array | ||
| $answer['application_configs'] = $result ? array_values($result) : []; |
There was a problem hiding this comment.
Can we use applicationConfigs here?
| $manager = $this->container->get('ilioscore.applicationconfig.manager'); | ||
| $manager->update($applicationConfig, true, false); | ||
|
|
||
| $answer['application_configs'] = [$applicationConfig]; |
There was a problem hiding this comment.
Can we use applicationConfigs here?
| * | ||
| * @return Response | ||
| */ | ||
| public function getAction($id) |
There was a problem hiding this comment.
This should use DTOs instead of entities. We're going that way with all of the old stuff and it would be good not to have to circle back and fix new ones as well.
There was a problem hiding this comment.
will be handled as follow-up item, see #1743.
| */ | ||
| public function getAction($id) | ||
| { | ||
| $schoolConfig = $this->getOr404($id); |
There was a problem hiding this comment.
We should use the DTO pattern here. It is better.
There was a problem hiding this comment.
will need to be addressed later, see #1743.
| throw $this->createAccessDeniedException('Unauthorized access!'); | ||
| } | ||
|
|
||
| $answer['school_configs'][] = $schoolConfig; |
There was a problem hiding this comment.
Can we use schoolConfigs here?
| }); | ||
|
|
||
| //If there are no matches return an empty array | ||
| $answer['school_configs'] = $result ? array_values($result) : []; |
There was a problem hiding this comment.
Can we use schoolConfigs here?
| $manager = $this->container->get('ilioscore.schoolconfig.manager'); | ||
| $manager->update($schoolConfig, true, false); | ||
|
|
||
| $answer['school_configs'] = [$schoolConfig]; |
There was a problem hiding this comment.
Can we use schoolConfigs here?
|
|
||
| $manager->update($schoolConfig, true, true); | ||
|
|
||
| $answer['school_config'] = $schoolConfig; |
There was a problem hiding this comment.
Can we use schoolConfigs here?
|
@jrjohnson ready to be reviewed again. |
fixes #1735