improvements following review

This commit is contained in:
Julie Lenaerts 2021-10-01 16:32:05 +02:00
parent 461b96ea37
commit b3b54486b5
3 changed files with 124 additions and 169 deletions

View File

@ -2,6 +2,7 @@
namespace Chill\TaskBundle\Controller;
use Chill\MainBundle\Entity\Center;
use Chill\MainBundle\Entity\Scope;
use Chill\PersonBundle\Privacy\PrivacyEvent;
use Psr\Log\LoggerInterface;
@ -24,15 +25,16 @@ use Chill\PersonBundle\Security\Authorization\PersonVoter;
use Chill\PersonBundle\Repository\PersonRepository;
use Chill\TaskBundle\Event\TaskEvent;
use Symfony\Component\EventDispatcher\EventDispatcherInterface;
use Symfony\Component\Translation\TranslatorInterface;
use Chill\TaskBundle\Event\UI\UIEvent;
use Chill\MainBundle\Repository\CenterRepository;
use Chill\MainBundle\Repository\ScopeRepository;
use Chill\MainBundle\Repository\UserRepository;
use Chill\MainBundle\Timeline\TimelineBuilder;
use Chill\PersonBundle\Entity\AccompanyingPeriod;
use Chill\PersonBundle\Repository\AccompanyingPeriodRepository;
use Chill\PersonBundle\Security\Authorization\AccompanyingPeriodVoter;
use Doctrine\ORM\EntityManagerInterface;
use Symfony\Component\HttpFoundation\RequestStack;
use Doctrine\ORM\EntityRepository;
use Symfony\Contracts\Translation\TranslatorInterface as TranslationTranslatorInterface;
/**
@ -42,27 +44,23 @@ use Symfony\Contracts\Translation\TranslatorInterface as TranslationTranslatorIn
*/
class SingleTaskController extends AbstractController
{
protected EventDispatcherInterface $eventDispatcher;
/**
* @var EventDispatcherInterface
*/
protected $eventDispatcher;
protected TimeLineBuilder $timelineBuilder;
/**
*
* @var TimelineBuilder
*/
protected $timelineBuilder;
protected LoggerInterface $logger;
/**
* @var LoggerInterface
*/
protected $logger;
protected EntityRepository $personRepository;
/**
* @var RequestStack
*/
protected $request;
protected EntityRepository $courseRepository;
protected SingleTaskRepository $taskRepository;
protected EntityRepository $userRepository;
protected EntityRepository $centerRepository;
protected EntityRepository $scopeRepository;
/**
* SingleTaskController constructor.
@ -73,20 +71,25 @@ class SingleTaskController extends AbstractController
EventDispatcherInterface $eventDispatcher,
TimelineBuilder $timelineBuilder,
LoggerInterface $logger,
RequestStack $requestStack
EntityManagerInterface $em
) {
$this->eventDispatcher = $eventDispatcher;
$this->timelineBuilder = $timelineBuilder;
$this->logger = $logger;
$this->request = $requestStack->getCurrentRequest();
$this->personRepository = $em->getRepository(Person::class);
$this->courseRepository = $em->getRepository(AccompanyingPeriod::class);
$this->taskRepository = $em->getRepository(SingleTask::class);
$this->userRepository = $em->getRepository(User::class);
$this->centerRepository = $em->getRepository(Center::class);
$this->scopeRepository = $em->getRepository(Scope::class);
}
private function getEntityContext()
private function getEntityContext(Request $request)
{
if($this->request->query->has('person_id')){
if($request->query->has('person_id')){
return 'person';
} else if ($this->request->query->has('course_id')) {
} else if ($request->query->has('course_id')) {
return 'course';
} else {
return null;
@ -101,7 +104,8 @@ class SingleTaskController extends AbstractController
* )
*/
public function newAction(
TranslationTranslatorInterface $translator
TranslationTranslatorInterface $translator,
Request $request
) {
$task = (new SingleTask())
@ -109,46 +113,41 @@ class SingleTaskController extends AbstractController
->setType('task_default')
;
$entityType = $this->getEntityContext();
$entityType = $this->getEntityContext($request);
if ($entityType !== null) {
$entityId = $this->request->query->getInt("{$entityType}_id", 0); // sf4 check:
$entityId = $request->query->getInt("{$entityType}_id", 0); // sf4 check:
// prevent error: `Argument 2 passed to ::getInt() must be of the type int, null given`
if ($entityId === null) {
return new Response("You must provide a {$entityType}_id", Response::HTTP_BAD_REQUEST);
}
if($entityType === 'person')
{
$person = $this->getDoctrine()->getManager()
->getRepository(Person::class)
switch($entityType){
case 'person':
$person = $this->personRepository
->find($entityId);
if ($person === null) {
$this->createNotFoundException("Invalid person id");
}
if ($person === null) {
$this->createNotFoundException("Invalid person id");
}
$task->setPerson($person);
$task->setPerson($person);
break;
case 'course':
$course = $this->courseRepository
->find($entityId);
if($course === null) {
$this->createNotFoundException("Invalid accompanying course id");
}
$task->setCourse($course);
break;
default:
new Response("You must provide a {$entityType}_id", Response::HTTP_BAD_REQUEST);
break;
}
if($entityType === 'course')
{
$course = $this->getDoctrine()->getManager()
->getRepository(AccompanyingPeriod::class)
->find($entityId);
if($course === null) {
$this->createNotFoundException("Invalid accompanying course id");
}
$task->setCourse($course);
}
}
//TODO : resolve access rights
@ -157,8 +156,7 @@ class SingleTaskController extends AbstractController
// . 'allowed to create this task');
$form = $this->setCreateForm($task, new Role(TaskVoter::CREATE));
$form->handleRequest($this->request);
$form->handleRequest($request);
if ($form->isSubmitted()) {
if ($form->isValid()) {
@ -171,18 +169,17 @@ class SingleTaskController extends AbstractController
$this->addFlash('success', $translator->trans("The task is created"));
if($entityType === 'person')
{
return $this->redirectToRoute('chill_task_singletask_list', [
switch($entityType){
case 'person':
return $this->redirectToRoute('chill_task_singletask_list', [
'person_id' => $task->getPerson()->getId()
]);
}
if($entityType === 'course')
{
return $this->redirectToRoute('chill_task_singletask_courselist', [
]);
break;
case 'course':
return $this->redirectToRoute('chill_task_singletask_courselist', [
'course_id' => $task->getCourse()->getId()
]);
]);
break;
}
} else {
@ -190,7 +187,7 @@ class SingleTaskController extends AbstractController
}
}
switch($this->getEntityContext()){
switch($this->getEntityContext($request)){
case 'person':
return $this->render('@ChillTask/SingleTask/Person/new.html.twig', array(
'form' => $form->createView(),
@ -218,8 +215,8 @@ class SingleTaskController extends AbstractController
public function showAction($id)
{
$em = $this->getDoctrine()->getManager();
$task = $em->getRepository(SingleTask::class)->find($id);
// $em = $this->getDoctrine()->getManager();
$task = $this->taskRepository->find($id);
if (!$task) {
throw $this->createNotFoundException('Unable to find Task entity.');
@ -227,15 +224,7 @@ class SingleTaskController extends AbstractController
if ($task->getPerson() !== null) {
$personId = $task->getPerson()->getId();
if ($personId === null) {
return new Response("You must provide a person_id", Response::HTTP_BAD_REQUEST);
}
$person = $this->getDoctrine()->getManager()
->getRepository(Person::class)
->find($personId);
$person = $task->getPerson();
if ($person === null) {
throw $this->createNotFoundException("Invalid person id");
@ -250,22 +239,9 @@ class SingleTaskController extends AbstractController
}
if ($task->getCourse() !== null)
if ($task->getCourse() === null)
{
$courseId = $task->getCourse()->getId();
if ($courseId === null) {
return new Response("You must provide a course_id", Response::HTTP_BAD_REQUEST);
}
$course = $this->getDoctrine()->getManager()
->getRepository(AccompanyingPeriod::class)
->find($courseId);
if ($course === null)
{
throw $this->createNotFoundException("Invalid course id");
}
return new Response("You must provide a course_id", Response::HTTP_BAD_REQUEST);
}
@ -299,34 +275,23 @@ class SingleTaskController extends AbstractController
*/
public function editAction(
$id,
TranslationTranslatorInterface $translator
TranslationTranslatorInterface $translator,
Request $request
) {
$em = $this->getDoctrine()->getManager();
$task = $em->getRepository(SingleTask::class)->find($id);
if ($task->getContext() instanceof Person) {
$personId = $task->getPerson()->getId();
if ($personId === null) {
$person = $task->getPerson();
if ($person === null) {
return new Response("You must provide a person_id", Response::HTTP_BAD_REQUEST);
}
$person = $this->getDoctrine()->getManager()
->getRepository(Person::class)
->find($personId);
if ($person === null) {
throw $this->createNotFoundException("Invalid person id");
}
} else {
$courseId = $task->getCourse()->getId();
if ($courseId === null) {
return new Response("You must provide a course_id", Response::HTTP_BAD_REQUEST);
}
$course = $this->getDoctrine()->getManager()
->getRepository(AccompanyingPeriod::class)
->find($courseId);
$course = $task->getCourse();
if ($course === null) {
throw $this->createNotFoundException("Invalid accompanying period id");
@ -348,7 +313,7 @@ class SingleTaskController extends AbstractController
$form = $event->getForm();
$form->handleRequest($this->request);
$form->handleRequest($request);
if ($form->isSubmitted()) {
if ($form->isValid()) {
@ -370,12 +335,12 @@ class SingleTaskController extends AbstractController
return $this->redirectToRoute(
'chill_task_singletask_list',
$this->request->query->get('list_params', [])
$request->query->get('list_params', [])
);
} else {
return $this->redirectToRoute(
'chill_task_singletask_courselist',
$this->request->query->get('list_params', [])
$request->query->get('list_params', [])
);
}
} else {
@ -423,9 +388,7 @@ class SingleTaskController extends AbstractController
$id,
TranslationTranslatorInterface $translator
) {
$em = $this->getDoctrine()->getManager();
$task = $em->getRepository(SingleTask::class)->find($id);
$task = $this->taskRepository->find($id);
if (!$task) {
throw $this->createNotFoundException('Unable to find Task entity.');
@ -437,8 +400,7 @@ class SingleTaskController extends AbstractController
return new Response("You must provide a person_id", Response::HTTP_BAD_REQUEST);
}
$person = $this->getDoctrine()->getManager()
->getRepository(Person::class)
$person = $this->personRepository
->find($personId);
if ($person === null) {
@ -451,8 +413,7 @@ class SingleTaskController extends AbstractController
return new Response("You must provide a course_id", Response::HTTP_BAD_REQUEST);
}
$course = $this->getDoctrine()->getManager()
->getRepository(AccompanyingPeriod::class)
$course = $this->courseRepository
->find($courseId);
if($course === null){
@ -571,11 +532,12 @@ class SingleTaskController extends AbstractController
*/
public function listAction(
PaginatorFactory $paginatorFactory,
SingleTaskRepository $taskRepository,
PersonRepository $personRepository,
AccompanyingPeriodRepository $courseRepository,
CenterRepository $centerRepository,
FormFactoryInterface $formFactory
// SingleTaskRepository $taskRepository,
// PersonRepository $personRepository,
// AccompanyingPeriodRepository $courseRepository,
// CenterRepository $centerRepository,
FormFactoryInterface $formFactory,
Request $request
) {
/* @var $viewParams array The parameters for the view */
/* @var $params array The parameters for the query */
@ -589,10 +551,10 @@ class SingleTaskController extends AbstractController
$viewParams['accompanyingCourse'] = null;
$params['accompanyingCourse'] = null;
if (!empty($this->request->query->get('person_id', NULL))) {
if (!empty($request->query->get('person_id', NULL))) {
$personId = $this->request->query->getInt('person_id', 0);
$person = $personRepository->find($personId);
$personId = $request->query->getInt('person_id', 0);
$person = $this->personRepository->find($personId);
if ($person === null) {
throw $this->createNotFoundException("This person ' $personId ' does not exist.");
@ -603,10 +565,10 @@ class SingleTaskController extends AbstractController
$params['person'] = $person;
}
if (!empty($this->request->query->get('course_id', NULL))) {
if (!empty($request->query->get('course_id', NULL))) {
$courseId = $this->request->query->getInt('course_id', 0);
$course = $courseRepository->find($courseId);
$courseId = $request->query->getInt('course_id', 0);
$course = $this->courseRepository->find($courseId);
if ($course === null) {
throw $this->createNotFoundException("This accompanying course ' $courseId ' does not exist.");
@ -617,27 +579,27 @@ class SingleTaskController extends AbstractController
$params['accompanyingCourse'] = $course;
}
if (!empty($this->request->query->get('center_id', NULL))) {
$center = $centerRepository->find($this->request->query->getInt('center_id'));
if (!empty($request->query->get('center_id', NULL))) {
$center = $this->centerRepository->find($this->request->query->getInt('center_id'));
if ($center === null) {
throw $this->createNotFoundException('center not found');
}
$params['center'] = $center;
}
if(!empty($this->request->query->get('types', []))) {
if(!empty($request->query->get('types', []))) {
$types = $this->request->query->get('types', []);
if (count($types) > 0) {
$params['types'] = $types;
}
}
if (!empty($this->request->query->get('user_id', null))) {
if ($this->request->query->get('user_id') === '_unassigned') {
if (!empty($request->query->get('user_id', null))) {
if ($request->query->get('user_id') === '_unassigned') {
$params['unassigned'] = true;
} else {
$userId = $this->request->query->getInt('user_id', 0);
$userId = $request->query->getInt('user_id', 0);
$user = $this->getDoctrine()->getManager()
->getRepository(User::class)
->find($userId);
@ -651,12 +613,11 @@ class SingleTaskController extends AbstractController
}
}
if (!empty($this->request->query->get('scope_id'))) {
if (!empty($request->query->get('scope_id'))) {
$scopeId = $this->request->query->getInt('scope_id', 0);
$scopeId = $request->query->getInt('scope_id', 0);
$scope = $this->getDoctrine()->getManager()
->getRepository(Scope::class)
$scope = $this->scopeRepository
->find($scopeId);
if ($scope === null) {
@ -668,7 +629,7 @@ class SingleTaskController extends AbstractController
}
$possibleStatuses = \array_merge(SingleTaskRepository::DATE_STATUSES, [ 'closed' ]);
$statuses = $this->request->query->get('status', $possibleStatuses);
$statuses = $request->query->get('status', $possibleStatuses);
$diff = \array_diff($statuses, $possibleStatuses);
if (count($diff) > 0) {
@ -683,7 +644,7 @@ class SingleTaskController extends AbstractController
$tasks_count = 0;
foreach($statuses as $status) {
if($this->request->query->has('status')
if($request->query->has('status')
&& FALSE === \in_array($status, $statuses)) {
continue;
}
@ -696,14 +657,14 @@ class SingleTaskController extends AbstractController
$params['is_closed'] = true;
}
$count = $taskRepository
$count = $this->taskRepository
->countByParameters($params, $this->getUser())
;
$paginator = $paginatorFactory->create($count);
$viewParams['single_task_'.$status.'_count'] = $count;
$viewParams['single_task_'.$status.'_paginator'] = $paginator;
$viewParams['single_task_'.$status.'_tasks'] = $taskRepository
$viewParams['single_task_'.$status.'_tasks'] = $this->taskRepository
->findByParameters($params, $this->getUser(),
$singleStatus ? $paginator->getCurrentPage()->getFirstItemNumber() : 0,
$singleStatus ? $paginator->getItemsPerPage() : 10)
@ -729,7 +690,7 @@ class SingleTaskController extends AbstractController
'add_type' => true
]);
$form->handleRequest($this->request);
$form->handleRequest($request);
if (isset($person)) {
$event = new PrivacyEvent($person, array(
@ -744,10 +705,10 @@ class SingleTaskController extends AbstractController
}
protected function getPersonParam(EntityManagerInterface $em)
protected function getPersonParam(Request $request)
{
$person = $em->getRepository(Person::class)
->find($this->request->query->getInt('person_id'))
$person = $this->personRepository
->find($request->query->getInt('person_id'))
;
if (NULL === $person) {
@ -760,10 +721,10 @@ class SingleTaskController extends AbstractController
return $person;
}
protected function getUserParam(EntityManagerInterface $em)
protected function getUserParam(Request $request)
{
$user = $em->getRepository(User::class)
->find($this->request->query->getInt('user_id'))
$user = $this->userRepository
->find($request->query->getInt('user_id'))
;
if (NULL === $user) {
@ -799,17 +760,16 @@ class SingleTaskController extends AbstractController
*/
public function listCourseTasks(
AccompanyingPeriodRepository $courseRepository,
SingleTaskRepository $taskRepository,
FormFactoryInterface $formFactory,
TranslationTranslatorInterface $translator
TranslationTranslatorInterface $translator,
Request $request
): Response
{
if (!empty($this->request->query->get('course_id', NULL))) {
if (!empty($request->query->get('course_id', NULL))) {
$courseId = $this->request->query->getInt('course_id', 0);
$course = $courseRepository->find($courseId);
$courseId = $request->query->getInt('course_id', 0);
$course = $this->courseRepository->find($courseId);
if ($course === null) {
throw $this->createNotFoundException("This accompanying course ' $courseId ' does not exist.");
@ -817,13 +777,11 @@ class SingleTaskController extends AbstractController
}
$em = $this->getDoctrine()->getManager();
if($course === NULL) {
throw $this->createNotFoundException('Accompanying course not found');
}
$tasks = $taskRepository
$tasks = $this->taskRepository
->findBy(
array('course' => $course)
);

View File

@ -8,5 +8,5 @@ services:
$eventDispatcher: '@Symfony\Component\EventDispatcher\EventDispatcherInterface'
$timelineBuilder: "@chill_main.timeline_builder"
$logger: "@chill.main.logger"
$requestStack: '@Symfony\Component\HttpFoundation\RequestStack'
$em: "@doctrine.orm.default_entity_manager"
tags: ["controller.service_arguments"]

View File

@ -7,14 +7,11 @@ namespace Chill\Migrations\Task;
use Doctrine\DBAL\Schema\Schema;
use Doctrine\Migrations\AbstractMigration;
/**
* Auto-generated Migration: Please modify to your needs!
*/
final class Version20210909153533 extends AbstractMigration
{
public function getDescription(): string
{
return '';
return 'An accompanying period can be linked to a task. Key course_id is added to single task entity';
}
public function up(Schema $schema): void