added comment field to invoice (#3045)

src/Constants.php

@@ -17,11 +17,11 @@ class Constants
     /**
      * The current release version
      */
-    public const VERSION = '1.16.9';
+    public const VERSION = '1.16.10';
     /**
      * The current release: major * 10000 + minor * 100 + patch
      */
-    public const VERSION_ID = 11609;
+    public const VERSION_ID = 11610;
     /**
      * The current release status, either "stable" or "dev"
      */

src/Controller/InvoiceController.php

@@ -18,7 +18,7 @@ use App\Export\Spreadsheet\AnnotatedObjectExporter;
 use App\Export\Spreadsheet\Writer\BinaryFileResponseWriter;
 use App\Export\Spreadsheet\Writer\XlsxWriter;
 use App\Form\InvoiceDocumentUploadForm;
-use App\Form\InvoicePaymentDateForm;
+use App\Form\InvoiceEditForm;
 use App\Form\InvoiceTemplateForm;
 use App\Form\Toolbar\InvoiceArchiveForm;
 use App\Form\Toolbar\InvoiceToolbarForm;
@@ -224,18 +224,19 @@ final class InvoiceController extends AbstractController
             return $this->redirectToRoute('admin_invoice_list');
         }
 
-        $token = $csrfTokenManager->refreshToken('invoice.status');
-
         if ($status === Invoice::STATUS_PAID) {
-            $form = $this->createPaymentDateForm($invoice, $status, $token->getValue());
+            if (null === $invoice->getPaymentDate()) {
+                $invoice->setPaymentDate($this->getDateTimeFactory()->createDateTime());
+                $invoice->setIsPaid();
+            }
+
+            $form = $this->createInvoiceEditForm($invoice);
             $form->handleRequest($request);
 
-            if (!$form->isSubmitted() || !$form->isValid()) {
+            return $this->render('invoice/invoice_edit.html.twig', [
-                return $this->render('invoice/payment_date_edit.html.twig', [
+                'invoice' => $invoice,
-                    'invoice' => $invoice,
+                'form' => $form->createView()
-                    'form' => $form->createView()
+            ]);
-                ]);
-            }
         }
 
         try {
@@ -248,6 +249,33 @@ final class InvoiceController extends AbstractController
         return $this->redirectToRoute('admin_invoice_list');
     }
 
+    /**
+     * @Route(path="/edit/{id}", name="admin_invoice_edit", methods={"GET", "POST"})
+     * @Security("is_granted('access', invoice.getCustomer())")
+     * @Security("is_granted('create_invoice')")
+     */
+    public function editAction(Invoice $invoice, Request $request): Response
+    {
+        $form = $this->createInvoiceEditForm($invoice);
+        $form->handleRequest($request);
+
+        if ($form->isSubmitted() && $form->isValid()) {
+            try {
+                $this->invoiceRepository->saveInvoice($invoice);
+                $this->flashSuccess('action.update.success');
+            } catch (Exception $ex) {
+                $this->flashUpdateException($ex);
+            }
+
+            return $this->redirectToRoute('admin_invoice_list');
+        }
+
+        return $this->render('invoice/invoice_edit.html.twig', [
+            'invoice' => $invoice,
+            'form' => $form->createView()
+        ]);
+    }
+
     /**
      * @Route(path="/delete/{id}/{token}", name="admin_invoice_delete", methods={"GET"})
      * @Security("is_granted('access', invoice.getCustomer())")
@@ -605,7 +633,7 @@ final class InvoiceController extends AbstractController
 
     private function renderTemplateForm(InvoiceTemplate $template, Request $request): Response
     {
-        $editForm = $this->createEditForm($template);
+        $editForm = $this->createTemplateEditForm($template);
 
         $editForm->handleRequest($request);
 
@@ -653,7 +681,7 @@ final class InvoiceController extends AbstractController
         ]);
     }
 
-    private function createEditForm(InvoiceTemplate $template): FormInterface
+    private function createTemplateEditForm(InvoiceTemplate $template): FormInterface
     {
         if ($template->getId() === null) {
             $url = $this->generateUrl('admin_invoice_template_create');
@@ -667,16 +695,10 @@ final class InvoiceController extends AbstractController
         ]);
     }
 
-    private function createPaymentDateForm(Invoice $invoice, string $status, string $token): FormInterface
+    private function createInvoiceEditForm(Invoice $invoice): FormInterface
     {
-        if (null === $invoice->getPaymentDate()) {
+        return $this->createForm(InvoiceEditForm::class, $invoice, [
-            $invoice->setPaymentDate($this->getDateTimeFactory()->createDateTime());
+            'action' => $this->generateUrl('admin_invoice_edit', ['id' => $invoice->getId()]),
-        }
-
-        $url = $this->generateUrl('admin_invoice_status', ['id' => $invoice->getId(), 'status' => $status, 'token' => $token]);
-
-        return $this->createForm(InvoicePaymentDateForm::class, $invoice, [
-            'action' => $url,
             'method' => 'POST',
             'timezone' => $this->getDateTimeFactory()->getTimezone()->getName(),
         ]);

src/Controller/UserController.php

@@ -136,7 +136,9 @@ final class UserController extends AbstractController
             $user->setTimezone($firstUser->getTimezone());
 
             $editForm = $this->getCreateUserForm($user);
-            $editForm->get('create_more')->setData(true);
+            if ($editForm->has('create_more')) {
+                $editForm->get('create_more')->setData(true);
+            }
         }
 
         return $this->render('user/create.html.twig', [

src/Entity/Invoice.php

@@ -12,6 +12,7 @@ namespace App\Entity;
 use App\Export\Annotation as Exporter;
 use App\Invoice\InvoiceModel;
 use Doctrine\ORM\Mapping as ORM;
+use JMS\Serializer\Annotation as Serializer;
 use Symfony\Bridge\Doctrine\Validator\Constraints\UniqueEntity;
 use Symfony\Component\Validator\Constraints as Assert;
 
@@ -26,7 +27,7 @@ use Symfony\Component\Validator\Constraints as Assert;
  * @UniqueEntity("invoiceNumber")
  * @UniqueEntity("invoiceFilename")
  *
- * @Exporter\Order({"id", "createdAt", "invoiceNumber", "status", "customer", "subtotal", "total", "tax", "currency", "vat", "dueDays", "dueDate", "paymentDate", "user", "invoiceFilename"})
+ * @Exporter\Order({"id", "createdAt", "invoiceNumber", "status", "customer", "subtotal", "total", "tax", "currency", "vat", "dueDays", "dueDate", "paymentDate", "user", "invoiceFilename", "comment"})
  * @Exporter\Expose("customer", label="label.customer", exp="object.getCustomer() === null ? null : object.getCustomer().getName()")
  * @Exporter\Expose("customerNumber", label="label.number", exp="object.getCustomer() === null ? null : object.getCustomer().getNumber()")
  * @Exporter\Expose("dueDate", label="invoice.due_days", type="datetime", exp="object.getDueDate() === null ? null : object.getDueDate()")
@@ -62,6 +63,18 @@ class Invoice
      */
     private $invoiceNumber;
 
+    /**
+     * @var string
+     *
+     * @Serializer\Expose()
+     * @Serializer\Groups({"Customer_Entity"})
+     *
+     * @Exporter\Expose(label="label.comment")
+     *
+     * @ORM\Column(name="comment", type="text", nullable=true)
+     */
+    private $comment;
+
     /**
      * @var Customer|null
      *
@@ -177,7 +190,7 @@ class Invoice
     private $localized = false;
 
     /**
-     * @var \DateTime
+     * @var \DateTime|null
      *
      * @ORM\Column(name="payment_date", type="date", nullable=true)
      */
@@ -312,6 +325,20 @@ class Invoice
         return $this->status === self::STATUS_CANCELED;
     }
 
+    public function getStatus(): string
+    {
+        return $this->status;
+    }
+
+    public function setStatus(string $status): void
+    {
+        if (!\in_array($status, [self::STATUS_NEW, self::STATUS_PENDING, self::STATUS_PAID, self::STATUS_CANCELED])) {
+            throw new \InvalidArgumentException('Unknown invoice status');
+        }
+
+        $this->status = $status;
+    }
+
     public function setIsCanceled(): void
     {
         $this->status = self::STATUS_CANCELED;
@@ -362,4 +389,14 @@ class Invoice
 
         return $this;
     }
+
+    public function setComment(?string $comment): void
+    {
+        $this->comment = $comment;
+    }
+
+    public function getComment(): ?string
+    {
+        return $this->comment;
+    }
 }

src/EventSubscriber/Actions/InvoiceSubscriber.php

@@ -30,6 +30,11 @@ class InvoiceSubscriber extends AbstractActionsSubscriber
             return;
         }
 
+        $event->addAction('edit', ['url' => $this->path('admin_invoice_edit', ['id' => $invoice->getId()]), 'class' => 'modal-ajax-form']);
+        $event->addAction('download', ['url' => $this->path('admin_invoice_download', ['id' => $invoice->getId()]), 'target' => '_blank']);
+
+        $event->addDivider();
+
         if (!$invoice->isPending()) {
             $event->addAction('invoice.pending', ['url' => $this->path('admin_invoice_status', ['id' => $invoice->getId(), 'status' => 'pending', 'token' => $payload['token']])]);
         } else {
@@ -42,11 +47,8 @@ class InvoiceSubscriber extends AbstractActionsSubscriber
             $event->addAction($id, ['url' => $this->path('admin_invoice_status', ['id' => $invoice->getId(), 'status' => 'canceled', 'token' => $payload['token']]), 'title' => 'invoice.cancel', 'translation_domain' => 'actions']);
         }
 
-        $event->addDivider();
-
-        $event->addAction('download', ['url' => $this->path('admin_invoice_download', ['id' => $invoice->getId()]), 'target' => '_blank']);
-
         if ($this->isGranted('delete_invoice')) {
+            $event->addDivider();
             $event->addDelete($this->path('admin_invoice_delete', ['id' => $invoice->getId(), 'token' => $payload['token']]), false);
         }
     }

src/Form/InvoiceEditForm.php

@@ -12,10 +12,12 @@ namespace App\Form;
 use App\Entity\Invoice;
 use App\Form\Type\DatePickerType;
 use Symfony\Component\Form\AbstractType;
+use Symfony\Component\Form\Extension\Core\Type\ChoiceType;
+use Symfony\Component\Form\Extension\Core\Type\TextareaType;
 use Symfony\Component\Form\FormBuilderInterface;
 use Symfony\Component\OptionsResolver\OptionsResolver;
 
-class InvoicePaymentDateForm extends AbstractType
+class InvoiceEditForm extends AbstractType
 {
     /**
      * {@inheritdoc}
@@ -28,9 +30,23 @@ class InvoicePaymentDateForm extends AbstractType
         ];
 
         $builder
+            ->add('comment', TextareaType::class, [
+                'label' => 'label.description',
+                'required' => false,
+            ])
+            ->add('status', ChoiceType::class, [
+                'choices' => [
+                    'status.new' => Invoice::STATUS_NEW,
+                    'status.pending' => Invoice::STATUS_PENDING,
+                    'status.paid' => Invoice::STATUS_PAID,
+                    'status.canceled' => Invoice::STATUS_CANCELED,
+                ],
+                'label' => 'label.status',
+                'required' => true,
+            ])
             ->add('paymentDate', DatePickerType::class, array_merge($dateTimeOptions, [
                 'label' => 'invoice.payment_date',
-                'required' => true,
+                'required' => false,
             ]));
     }
 

src/Migrations/Version20211230163612.php

@@ -0,0 +1,38 @@
+<?php
+
+declare(strict_types=1);
+
+/*
+ * 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 DoctrineMigrations;
+
+use App\Doctrine\AbstractMigration;
+use Doctrine\DBAL\Schema\Schema;
+
+/**
+ * @version 1.17
+ */
+final class Version20211230163612 extends AbstractMigration
+{
+    public function getDescription(): string
+    {
+        return 'Adds the comment column to the invoices table.';
+    }
+
+    public function up(Schema $schema): void
+    {
+        $invoices = $schema->getTable('kimai2_invoices');
+        $invoices->addColumn('comment', 'text', ['notnull' => false]);
+    }
+
+    public function down(Schema $schema): void
+    {
+        $invoices = $schema->getTable('kimai2_invoices');
+        $invoices->dropColumn('comment');
+    }
+}

templates/invoice/index.html.twig

@@ -108,7 +108,6 @@
                                     {{ widgets.label_customer(model.customer) }}
                                 </td>
                                 <td class="w-min text-center">
-                                    {{ widgets.action_button('show', {'url': '#invoice_preview_details_' ~ model.customer.id, 'title': 'timesheet.all'|trans, 'class': 'btn btn-sm hidden-xs hidden-sm'}, 'link') }}
                                     {{ widgets.action_button('print', {'url': '#', 'onclick': 'return singleInvoice(this)', 'title': 'button.preview'|trans, 'target': '_blank', 'class': 'btn btn-sm', 'attr': {'data-customer': model.customer.id, 'data-template': model.template.id, 'data-href': path('invoice_preview', {'customer': model.customer.id, 'token': csrf_token('invoice.preview')})}}) }}
                                     {{ widgets.action_button('save', {'url': '#', 'onclick': 'return singleInvoice(this)', 'title': 'action.save'|trans, 'class': 'btn btn-sm', 'attr': {'data-customer': model.customer.id, 'data-template': model.template.id, 'data-href': path('invoice_create', {'customer': model.customer.id, 'template': model.template.id, 'token': csrf_token('invoice.create')})}}, 'success') }}
                                 </td>

templates/invoice/listing.html.twig

@@ -9,6 +9,7 @@
     'date': {'class': 'alwaysVisible'},
     'user': {'class': 'hidden-xs hidden-sm text-nowrap hidden', 'orderBy': false},
     'customer': {'class': 'hidden-xs hidden-sm text-nowrap', 'orderBy': false},
+    'comment': {'class': 'hidden-xs hidden-sm', 'title': 'label.description'|trans},
     'invoice_number': {'class': 'hidden-xs hidden-sm w-min', 'title': 'invoice.number'|trans, 'orderBy': false},
     'due_date': {'class': 'hidden-xs w-min', 'title': 'invoice.due_days'|trans, 'orderBy': false},
     'payment_date': {'class': 'hidden-xs hidden w-min', 'title': 'invoice.payment_date'|trans, 'orderBy': false},
@@ -36,10 +37,11 @@
     {% else %}
         {{ tables.datatable_header(tableName, columns, query, {}) }}
         {% for entry in entries %}
-            <tr class="alternative-link open-edit{% if entry.canceled %} warning text-muted{% endif %}" data-href="{{ path('admin_invoice_download', {'id': entry.id}) }}">
+            <tr class="modal-ajax-form open-edit{% if entry.canceled %} warning text-muted{% endif %}" data-href="{{ path('admin_invoice_edit', {'id': entry.id}) }}">
                 <td class="{{ tables.data_table_column_class(tableName, columns, 'date') }}">{{ entry.createdAt|date_short }}</td>
                 <td class="{{ tables.data_table_column_class(tableName, columns, 'user') }}">{{ widgets.user_avatar(entry.user) }} {{ widgets.username(entry.user) }}</td>
                 <td class="{{ tables.data_table_column_class(tableName, columns, 'customer') }}">{{ widgets.label_customer(entry.customer) }}</td>
+                <td class="{{ tables.data_table_column_class(tableName, columns, 'comment') }}">{{ entry.comment }}</td>
                 <td class="{{ tables.data_table_column_class(tableName, columns, 'invoice_number') }}">{{ widgets.label(entry.invoiceNumber, 'default') }}</td>
                 <td class="{{ tables.data_table_column_class(tableName, columns, 'due_date') }}">{{ macros.invoice_due_date(entry) }}</td>
                 <td class="{{ tables.data_table_column_class(tableName, columns, 'payment_date') }}">

tests/Controller/InvoiceControllerTest.php

@@ -352,20 +352,20 @@ class InvoiceControllerTest extends ControllerBaseTest
         $this->assertHasValidationError(
             $client,
             '/invoice/change-status/' . $id . '/paid/' . $token->getValue(),
-            'form[name=invoice_payment_date_form]',
+            'form[name=invoice_edit_form]',
             [
-                'invoice_payment_date_form' => [
+                'invoice_edit_form' => [
                     'paymentDate' => 'invalid'
                 ]
             ],
-            ['#invoice_payment_date_form_paymentDate']
+            ['#invoice_edit_form_paymentDate']
         );
 
         $this->assertTrue($client->getResponse()->isSuccessful());
 
-        $form = $client->getCrawler()->filter('form[name=invoice_payment_date_form]')->form();
+        $form = $client->getCrawler()->filter('form[name=invoice_edit_form]')->form();
         $client->submit($form, [
-            'invoice_payment_date_form' => [
+            'invoice_edit_form' => [
                 'paymentDate' => (new \DateTime())->format('Y-m-d')
             ]
         ]);

tests/Controller/ProfileControllerTest.php

@@ -49,12 +49,10 @@ class ProfileControllerTest extends ControllerBaseTest
         $client = $this->getClientForAuthenticatedUser(User::ROLE_USER);
 
         $dates = [
-            new \DateTime('-10 days'),
+            new \DateTime('2018-06-13'),
-            new \DateTime('-1 year'),
+            new \DateTime('2021-10-20'),
         ];
 
-        $em = $this->getEntityManager();
-
         foreach ($dates as $start) {
             $fixture = new TimesheetFixtures();
             $fixture->setAmount(10);
@@ -125,7 +123,6 @@ class ProfileControllerTest extends ControllerBaseTest
         $client = $this->getClientForAuthenticatedUser(User::ROLE_USER);
         $this->request($client, '/profile/' . UserFixtures::USERNAME_USER . '/edit');
 
-        $em = $this->getEntityManager();
         /** @var User $user */
         $user = $this->getUserByRole(User::ROLE_USER);
 
@@ -150,7 +147,6 @@ class ProfileControllerTest extends ControllerBaseTest
 
         $this->assertHasFlashSuccess($client);
 
-        $em = $this->getEntityManager();
         $user = $this->getUserByRole(User::ROLE_USER);
 
         $this->assertEquals(UserFixtures::USERNAME_USER, $user->getUsername());
@@ -181,7 +177,6 @@ class ProfileControllerTest extends ControllerBaseTest
 
         $this->assertHasFlashSuccess($client);
 
-        $em = $this->getEntityManager();
         $user = $this->getUserByRole(User::ROLE_USER);
 
         $this->assertEquals(UserFixtures::USERNAME_USER, $user->getUsername());
@@ -196,7 +191,6 @@ class ProfileControllerTest extends ControllerBaseTest
         $client = $this->getClientForAuthenticatedUser(User::ROLE_USER);
         $this->request($client, '/profile/' . UserFixtures::USERNAME_USER . '/password');
 
-        $em = $this->getEntityManager();
         /** @var User $user */
         $user = $this->getUserByRole(User::ROLE_USER);
 
@@ -223,7 +217,6 @@ class ProfileControllerTest extends ControllerBaseTest
 
         $this->assertHasFlashSuccess($client);
 
-        $em = $this->getEntityManager();
         $user = $this->getUserByRole(User::ROLE_USER);
 
         $this->assertFalse($passwordEncoder->getEncoder($user)->isPasswordValid($user->getPassword(), UserFixtures::DEFAULT_PASSWORD, $user->getSalt()));
@@ -253,7 +246,6 @@ class ProfileControllerTest extends ControllerBaseTest
         $client = $this->getClientForAuthenticatedUser(User::ROLE_USER);
         $this->request($client, '/profile/' . UserFixtures::USERNAME_USER . '/api-token');
 
-        $em = $this->getEntityManager();
         /** @var User $user */
         $user = $this->getUserByRole(User::ROLE_USER);
         /** @var EncoderFactoryInterface $passwordEncoder */
@@ -279,7 +271,6 @@ class ProfileControllerTest extends ControllerBaseTest
 
         $this->assertHasFlashSuccess($client);
 
-        $em = $this->getEntityManager();
         $user = $this->getUserByRole(User::ROLE_USER);
 
         $this->assertFalse($passwordEncoder->getEncoder($user)->isPasswordValid($user->getApiToken(), UserFixtures::DEFAULT_API_TOKEN, $user->getSalt()));
@@ -316,7 +307,6 @@ class ProfileControllerTest extends ControllerBaseTest
         $client = $this->getClientForAuthenticatedUser(User::ROLE_SUPER_ADMIN);
         $this->request($client, '/profile/' . UserFixtures::USERNAME_USER . '/roles');
 
-        $em = $this->getEntityManager();
         /** @var User $user */
         $user = $this->getUserByRole(User::ROLE_USER);
 
@@ -336,7 +326,6 @@ class ProfileControllerTest extends ControllerBaseTest
 
         $this->assertHasFlashSuccess($client);
 
-        $em = $this->getEntityManager();
         $user = $this->getUserByRole(User::ROLE_USER);
 
         $this->assertEquals(['ROLE_TEAMLEAD', 'ROLE_SUPER_ADMIN', 'ROLE_USER'], $user->getRoles());

tests/Entity/InvoiceTest.php

@@ -54,6 +54,16 @@ class InvoiceTest extends TestCase
         self::assertFalse($sut->isPaid());
         self::assertFalse($sut->isOverdue());
         self::assertNull($sut->getPaymentDate());
+        self::assertNull($sut->getComment());
+    }
+
+    public function testSetInvalidStatus()
+    {
+        $this->expectException(\InvalidArgumentException::class);
+        $this->expectExceptionMessage('Unknown invoice status');
+
+        $sut = new Invoice();
+        $sut->setStatus('foo');
     }
 
     public function testSetterAndGetter()
@@ -65,17 +75,35 @@ class InvoiceTest extends TestCase
         self::assertFalse($sut->isNew());
         self::assertTrue($sut->isPending());
         self::assertFalse($sut->isPaid());
+        self::assertFalse($sut->isCanceled());
+        self::assertEquals(Invoice::STATUS_PENDING, $sut->getStatus());
 
         $sut->setIsPaid();
         self::assertFalse($sut->isNew());
         self::assertFalse($sut->isPending());
         self::assertTrue($sut->isPaid());
+        self::assertFalse($sut->isCanceled());
+        self::assertEquals(Invoice::STATUS_PAID, $sut->getStatus());
+
+        $sut->setStatus(Invoice::STATUS_PENDING);
+        self::assertTrue($sut->isPending());
+        self::assertEquals(Invoice::STATUS_PENDING, $sut->getStatus());
+
+        $sut->setIsCanceled();
+        self::assertFalse($sut->isNew());
+        self::assertFalse($sut->isPending());
+        self::assertFalse($sut->isPaid());
+        self::assertFalse($sut->isOverdue());
+        self::assertTrue($sut->isCanceled());
+        self::assertEquals(Invoice::STATUS_CANCELED, $sut->getStatus());
 
         $sut->setIsNew();
         self::assertTrue($sut->isNew());
         self::assertFalse($sut->isPending());
         self::assertFalse($sut->isPaid());
         self::assertFalse($sut->isOverdue());
+        self::assertFalse($sut->isCanceled());
+        self::assertEquals(Invoice::STATUS_NEW, $sut->getStatus());
 
         $paymentDate = new \DateTime();
         $sut->setPaymentDate($paymentDate);
@@ -102,6 +130,9 @@ class InvoiceTest extends TestCase
         self::assertEquals(348.99, $sut->getTotal());
         self::assertNotNull($sut->getUser());
         self::assertEquals(19, $sut->getVat());
+
+        $sut->setComment('foo bar');
+        self::assertEquals('foo bar', $sut->getComment());
     }
 
     protected function getInvoiceModel(\DateTime $created): InvoiceModel