do not allow to stop already stopped timesheets (#289)

src/Model/DashboardSection.php

@@ -11,8 +11,8 @@ namespace App\Model;
 
 class DashboardSection
 {
-    const TYPE_SIMPLE = 'simple';
-    const TYPE_CHART = 'chart';
+    public const TYPE_SIMPLE = 'simple';
+    public const TYPE_CHART = 'chart';
 
     /**
      * @var null|string
@@ -54,6 +54,7 @@ class DashboardSection
     public function setType(string $type)
     {
         $this->type = $type;
+
         return $this;
     }
 

src/Repository/ActivityRepository.php

@@ -15,6 +15,8 @@ use App\Entity\User;
 use App\Model\ActivityStatistic;
 use App\Repository\Query\ActivityQuery;
 use Doctrine\ORM\Query;
+use Doctrine\ORM\QueryBuilder;
+use Pagerfanta\Pagerfanta;
 
 /**
  * Class ActivityRepository
@@ -130,7 +132,7 @@ class ActivityRepository extends AbstractRepository
 
     /**
      * @param ActivityQuery $query
-     * @return \Doctrine\ORM\QueryBuilder|\Pagerfanta\Pagerfanta
+     * @return QueryBuilder|Pagerfanta|array
      */
     public function findByQuery(ActivityQuery $query)
     {

src/Repository/CustomerRepository.php

@@ -16,6 +16,8 @@ use App\Entity\Timesheet;
 use App\Model\CustomerStatistic;
 use App\Repository\Query\CustomerQuery;
 use Doctrine\ORM\Query;
+use Doctrine\ORM\QueryBuilder;
+use Pagerfanta\Pagerfanta;
 
 /**
  * Class CustomerRepository
@@ -97,7 +99,7 @@ class CustomerRepository extends AbstractRepository
 
     /**
      * @param CustomerQuery $query
-     * @return \Doctrine\ORM\QueryBuilder|\Pagerfanta\Pagerfanta
+     * @return QueryBuilder|Pagerfanta|array
      */
     public function findByQuery(CustomerQuery $query)
     {

src/Repository/InvoiceTemplateRepository.php

@@ -12,6 +12,8 @@ namespace App\Repository;
 use App\Entity\InvoiceTemplate;
 use App\Repository\Query\BaseQuery;
 use Doctrine\ORM\Query;
+use Doctrine\ORM\QueryBuilder;
+use Pagerfanta\Pagerfanta;
 
 /**
  * Class InvoiceTemplateRepository
@@ -40,7 +42,7 @@ class InvoiceTemplateRepository extends AbstractRepository
 
     /**
      * @param BaseQuery $query
-     * @return \Doctrine\ORM\QueryBuilder|\Pagerfanta\Pagerfanta
+     * @return QueryBuilder|Pagerfanta|array
      */
     public function findByQuery(BaseQuery $query)
     {

src/Repository/ProjectRepository.php

@@ -15,6 +15,8 @@ use App\Entity\Timesheet;
 use App\Model\ProjectStatistic;
 use App\Repository\Query\ProjectQuery;
 use Doctrine\ORM\Query;
+use Doctrine\ORM\QueryBuilder;
+use Pagerfanta\Pagerfanta;
 
 /**
  * Class ProjectRepository
@@ -90,7 +92,7 @@ class ProjectRepository extends AbstractRepository
 
     /**
      * @param ProjectQuery $query
-     * @return \Doctrine\ORM\QueryBuilder|\Pagerfanta\Pagerfanta
+     * @return QueryBuilder|Pagerfanta|array
      */
     public function findByQuery(ProjectQuery $query)
     {

src/Repository/Query/BaseQuery.php

@@ -20,6 +20,7 @@ class BaseQuery
     public const DEFAULT_PAGESIZE = 25;
     public const DEFAULT_PAGE = 1;
 
+    public const RESULT_TYPE_OBJECTS = 'Objects';
     public const RESULT_TYPE_PAGER = 'PagerFanta';
     public const RESULT_TYPE_QUERYBUILDER = 'QueryBuilder';
 
@@ -141,13 +142,18 @@ class BaseQuery
     /**
      * @param string $resultType
      * @return $this
+     * @throws \InvalidArgumentException
      */
-    public function setResultType($resultType)
+    public function setResultType(string $resultType)
     {
-        if (in_array($resultType, [self::RESULT_TYPE_PAGER, self::RESULT_TYPE_QUERYBUILDER])) {
-            $this->resultType = $resultType;
+        $allowed = [self::RESULT_TYPE_PAGER, self::RESULT_TYPE_QUERYBUILDER, self::RESULT_TYPE_OBJECTS];
+
+        if (!in_array($resultType, $allowed)) {
+            throw new \InvalidArgumentException('Unsupported query result type');
         }
 
+        $this->resultType = $resultType;
+
         return $this;
     }
 

src/Repository/RepositoryException.php

@@ -0,0 +1,14 @@
+<?php
+
+/*
+ * This file is part of the Kimai time-tracking app.
+ *
+ * For the full copyright and license information, please view the LICENSE
+ * file that was distributed with this source code.
+ */
+
+namespace App\Repository;
+
+class RepositoryException extends \Exception
+{
+}

src/Repository/RepositoryTrait.php

@@ -23,12 +23,14 @@ trait RepositoryTrait
     /**
      * @param QueryBuilder $qb
      * @param BaseQuery $query
-     * @return QueryBuilder|Pagerfanta
+     * @return QueryBuilder|Pagerfanta|array
      */
     protected function getBaseQueryResult(QueryBuilder $qb, BaseQuery $query)
     {
-        if (BaseQuery::RESULT_TYPE_PAGER == $query->getResultType()) {
+        if (BaseQuery::RESULT_TYPE_PAGER === $query->getResultType()) {
             return $this->getPager($qb->getQuery(), $query->getPage(), $query->getPageSize());
+        } elseif (BaseQuery::RESULT_TYPE_OBJECTS === $query->getResultType()) {
+            return $qb->getQuery()->execute();
         }
 
         return $qb;

src/Repository/TimesheetRepository.php

@@ -33,14 +33,20 @@ class TimesheetRepository extends AbstractRepository
     /**
      * @param Timesheet $entry
      * @return bool
+     * @throws RepositoryException
      * @throws \Doctrine\ORM\ORMException
      * @throws \Doctrine\ORM\OptimisticLockException
      */
     public function stopRecording(Timesheet $entry)
     {
+        if (null !== $entry->getEnd()) {
+            throw new RepositoryException('Timesheet entry already stopped');
+        }
+
         // seems to be necessary so Doctrine will recognize a changed timestamp
         $entry->setBegin(clone $entry->getBegin());
         $entry->setEnd(new DateTime());
+
         $entityManager = $this->getEntityManager();
         $entityManager->persist($entry);
         $entityManager->flush();
@@ -51,7 +57,7 @@ class TimesheetRepository extends AbstractRepository
     /**
      * @param User $user
      * @param Activity $activity
-     * @return bool
+     * @return Timesheet
      * @throws \Doctrine\ORM\ORMException
      * @throws \Doctrine\ORM\OptimisticLockException
      */
@@ -67,7 +73,7 @@ class TimesheetRepository extends AbstractRepository
         $entityManager->persist($entry);
         $entityManager->flush();
 
-        return true;
+        return $entry;
     }
 
     /**
@@ -279,7 +285,7 @@ class TimesheetRepository extends AbstractRepository
 
     /**
      * @param TimesheetQuery $query
-     * @return QueryBuilder|Pagerfanta
+     * @return QueryBuilder|Pagerfanta|array
      */
     public function findByQuery(TimesheetQuery $query)
     {

src/Repository/WidgetRepository.php

@@ -60,7 +60,7 @@ class WidgetRepository
         $begin = !empty($widget['begin']) ? new \DateTime($widget['begin']) : null;
         $end = !empty($widget['end']) ? new \DateTime($widget['end']) : null;
         $theUser = $widget['user'] ? $user : null;
-        $type = isset($widget['type']) ? $widget['type'] : Widget::TYPE_COUNTER;
+        $type = $widget['type'] ?? Widget::TYPE_COUNTER;
 
         $data = $this->repository->getStatistic($widget['query'], $begin, $end, $theUser);
 

tests/Controller/ControllerBaseTest.php

@@ -11,10 +11,7 @@ namespace App\Tests\Controller;
 
 use App\DataFixtures\UserFixtures;
 use App\Entity\User;
-use Doctrine\Bundle\FixturesBundle\Fixture;
-use Doctrine\Common\DataFixtures\Executor\ORMExecutor;
-use Doctrine\Common\DataFixtures\Loader;
-use Doctrine\ORM\EntityManager;
+use App\Tests\KernelTestTrait;
 use Symfony\Bundle\FrameworkBundle\Client;
 use Symfony\Bundle\FrameworkBundle\Test\WebTestCase;
 use Symfony\Component\HttpFoundation\RedirectResponse;
@@ -24,6 +21,8 @@ use Symfony\Component\HttpFoundation\RedirectResponse;
  */
 abstract class ControllerBaseTest extends WebTestCase
 {
+    use KernelTestTrait;
+
     public const DEFAULT_LANGUAGE = 'en';
 
     /**
@@ -213,52 +212,6 @@ abstract class ControllerBaseTest extends WebTestCase
         }
     }
 
-    /**
-     * @param EntityManager $em
-     * @param Fixture $fixture
-     */
-    protected function importFixture(EntityManager $em, Fixture $fixture)
-    {
-        $loader = new Loader();
-        $loader->addFixture($fixture);
-
-        $executor = new ORMExecutor($em, null);
-        $executor->execute($loader->getFixtures(), true);
-    }
-
-    /**
-     * @param EntityManager $em
-     * @param string $role
-     * @return User|null
-     */
-    protected function getUserByRole(EntityManager $em, string $role = User::ROLE_USER)
-    {
-        $name = null;
-
-        switch ($role) {
-            case User::ROLE_SUPER_ADMIN:
-                $name = UserFixtures::USERNAME_SUPER_ADMIN;
-                break;
-
-            case User::ROLE_ADMIN:
-                $name = UserFixtures::USERNAME_ADMIN;
-                break;
-
-            case User::ROLE_TEAMLEAD:
-                $name = UserFixtures::USERNAME_TEAMLEAD;
-                break;
-
-            case User::ROLE_USER:
-                $name = UserFixtures::USERNAME_USER;
-                break;
-
-            default:
-                return null;
-        }
-
-        return $em->getRepository(User::class)->findOneBy(['username' => $name]);
-    }
-
     /**
      * @param Client $client
      */

tests/KernelTestTrait.php

@@ -0,0 +1,69 @@
+<?php
+
+/*
+ * This file is part of the Kimai time-tracking app.
+ *
+ * For the full copyright and license information, please view the LICENSE
+ * file that was distributed with this source code.
+ */
+
+namespace App\Tests;
+
+use App\DataFixtures\UserFixtures;
+use App\Entity\User;
+use Doctrine\Bundle\FixturesBundle\Fixture;
+use Doctrine\Common\DataFixtures\Executor\ORMExecutor;
+use Doctrine\Common\DataFixtures\Loader;
+use Doctrine\ORM\EntityManager;
+
+/**
+ * A trait to be used in all tests that extend the KernelTestCase.
+ */
+trait KernelTestTrait
+{
+    /**
+     * @param EntityManager $em
+     * @param Fixture $fixture
+     */
+    protected function importFixture(EntityManager $em, Fixture $fixture)
+    {
+        $loader = new Loader();
+        $loader->addFixture($fixture);
+
+        $executor = new ORMExecutor($em, null);
+        $executor->execute($loader->getFixtures(), true);
+    }
+
+    /**
+     * @param EntityManager $em
+     * @param string $role
+     * @return User|null
+     */
+    protected function getUserByRole(EntityManager $em, string $role = User::ROLE_USER)
+    {
+        $name = null;
+
+        switch ($role) {
+            case User::ROLE_SUPER_ADMIN:
+                $name = UserFixtures::USERNAME_SUPER_ADMIN;
+                break;
+
+            case User::ROLE_ADMIN:
+                $name = UserFixtures::USERNAME_ADMIN;
+                break;
+
+            case User::ROLE_TEAMLEAD:
+                $name = UserFixtures::USERNAME_TEAMLEAD;
+                break;
+
+            case User::ROLE_USER:
+                $name = UserFixtures::USERNAME_USER;
+                break;
+
+            default:
+                return null;
+        }
+
+        return $em->getRepository(User::class)->findOneBy(['username' => $name]);
+    }
+}

tests/Repository/AbstractRepositoryTest.php

@@ -0,0 +1,58 @@
+<?php
+
+/*
+ * This file is part of the Kimai time-tracking app.
+ *
+ * For the full copyright and license information, please view the LICENSE
+ * file that was distributed with this source code.
+ */
+
+namespace App\Tests\Repository;
+
+use App\Tests\KernelTestTrait;
+use Doctrine\ORM\EntityManager;
+use Symfony\Bundle\FrameworkBundle\Test\KernelTestCase;
+
+/**
+ * A base test class for AbstractRepository implementations.
+ */
+abstract class AbstractRepositoryTest extends KernelTestCase
+{
+    use KernelTestTrait;
+
+    /**
+     * @var EntityManager
+     */
+    private $entityManager;
+
+    /**
+     * {@inheritdoc}
+     */
+    protected function setUp()
+    {
+        $kernel = self::bootKernel();
+
+        $this->entityManager = $kernel->getContainer()
+            ->get('doctrine')
+            ->getManager();
+    }
+
+    /**
+     * @return EntityManager
+     */
+    protected function getEntityManager()
+    {
+        return $this->entityManager;
+    }
+
+    /**
+     * {@inheritdoc}
+     */
+    protected function tearDown()
+    {
+        parent::tearDown();
+
+        $this->entityManager->close();
+        $this->entityManager = null; // avoid memory leaks
+    }
+}

tests/Repository/Query/BaseQueryTest.php

@@ -37,11 +37,18 @@ class BaseQueryTest extends TestCase
     {
         $this->assertEquals(BaseQuery::RESULT_TYPE_PAGER, $sut->getResultType());
 
-        $sut->setResultType('foo-bar');
-        $this->assertEquals(BaseQuery::RESULT_TYPE_PAGER, $sut->getResultType());
-
         $sut->setResultType(BaseQuery::RESULT_TYPE_QUERYBUILDER);
         $this->assertEquals(BaseQuery::RESULT_TYPE_QUERYBUILDER, $sut->getResultType());
+
+        $sut->setResultType(BaseQuery::RESULT_TYPE_OBJECTS);
+        $this->assertEquals(BaseQuery::RESULT_TYPE_OBJECTS, $sut->getResultType());
+
+        try {
+            $sut->setResultType('foo-bar');
+        } catch (\Exception $exception) {
+            $this->assertInstanceOf(\InvalidArgumentException::class, $exception);
+            $this->assertEquals('Unsupported query result type', $exception->getMessage());
+        }
     }
 
     protected function assertHiddenEntity(BaseQuery $sut)

tests/Repository/TimesheetRepositoryTest.php

@@ -0,0 +1,102 @@
+<?php
+
+/*
+ * This file is part of the Kimai time-tracking app.
+ *
+ * For the full copyright and license information, please view the LICENSE
+ * file that was distributed with this source code.
+ */
+
+namespace App\Tests\Repository;
+
+use App\Entity\Timesheet;
+use App\Entity\User;
+use App\Repository\Query\BaseQuery;
+use App\Repository\Query\TimesheetQuery;
+use App\Repository\RepositoryException;
+use App\Tests\DataFixtures\TimesheetFixtures;
+use Doctrine\ORM\QueryBuilder;
+use Pagerfanta\Pagerfanta;
+
+/**
+ * @covers \App\Repository\TimesheetRepository
+ */
+class TimesheetRepositoryTest extends AbstractRepositoryTest
+{
+    public function testResultTypeForQueryState()
+    {
+        $em = $this->getEntityManager();
+        $repository = $em->getRepository(Timesheet::class);
+
+        $query = new TimesheetQuery();
+
+        $result = $repository->findByQuery($query);
+        $this->assertInstanceOf(Pagerfanta::class, $result);
+
+        $query->setResultType(BaseQuery::RESULT_TYPE_QUERYBUILDER);
+        $result = $repository->findByQuery($query);
+        $this->assertInstanceOf(QueryBuilder::class, $result);
+
+        $query->setResultType(BaseQuery::RESULT_TYPE_PAGER);
+        $result = $repository->findByQuery($query);
+        $this->assertInstanceOf(Pagerfanta::class, $result);
+
+        $query->setResultType(BaseQuery::RESULT_TYPE_OBJECTS);
+        $result = $repository->findByQuery($query);
+        $this->assertInternalType('array', $result);
+    }
+
+    public function testStoppedEntriesCannotBeStoppedAgain()
+    {
+        $em = $this->getEntityManager();
+        $user = $this->getUserByRole($em, User::ROLE_USER);
+        $repository = $em->getRepository(Timesheet::class);
+
+        $fixtures = new TimesheetFixtures();
+        $fixtures->setUser($user);
+        $fixtures->setAmount(1);
+
+        $this->importFixture($em, $fixtures);
+
+        $query = new TimesheetQuery();
+        $query->setResultType(BaseQuery::RESULT_TYPE_OBJECTS);
+        $query->setUser($user);
+        $query->setState(TimesheetQuery::STATE_STOPPED);
+
+        $entities = $repository->findByQuery($query);
+
+        $this->assertCount(1, $entities);
+        $this->assertInstanceOf(Timesheet::class, $entities[0]);
+
+        $this->expectException(RepositoryException::class);
+        $this->expectExceptionMessage('Timesheet entry already stopped');
+
+        $repository->stopRecording($entities[0]);
+    }
+
+    public function testStartAndStop()
+    {
+        $em = $this->getEntityManager();
+        $user = $this->getUserByRole($em, User::ROLE_USER);
+        $repository = $em->getRepository(Timesheet::class);
+
+        $fixtures = new TimesheetFixtures();
+        $fixtures->setUser($user);
+        $fixtures->setAmount(1);
+        $this->importFixture($em, $fixtures);
+
+        $query = new TimesheetQuery();
+        $query->setResultType(BaseQuery::RESULT_TYPE_OBJECTS);
+        $entities = $repository->findByQuery($query);
+        $activity = $entities[0]->getActivity();
+
+        $user = $this->getUserByRole($em, User::ROLE_USER);
+        $timesheet = $repository->startRecording($user, $activity);
+        $this->assertInstanceOf(Timesheet::class, $timesheet);
+        $this->assertNull($timesheet->getEnd());
+
+        $result = $repository->stopRecording($timesheet);
+        $this->assertTrue($result);
+        $this->assertInstanceOf(\DateTime::class, $timesheet->getEnd());
+    }
+}