diff --git a/CHANGELOG.md b/CHANGELOG.md index 5c6b8547..d00a3f2e 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -8,6 +8,8 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ## [Unreleased] +* [PR-328](https://github.com/itk-dev/economics/pull/328) + Added selected and total hours to the worklog selection list. * [PR-324](https://github.com/itk-dev/economics/pull/324) Added game center with snake * [PR-303](https://github.com/itk-dev/economics/pull/303) diff --git a/assets/controllers/entry-select_controller.js b/assets/controllers/entry-select_controller.js index cc70b401..e86ada93 100644 --- a/assets/controllers/entry-select_controller.js +++ b/assets/controllers/entry-select_controller.js @@ -8,6 +8,7 @@ export default class extends Controller { "spinner", "result", "submitButton", + "selectedHours", ]; submitEndpoint = null; @@ -20,6 +21,7 @@ export default class extends Controller { connect() { this.submitEndpoint = this.element.dataset.submitEndpoint; + this.updateSelectedHours(); } toggleAll() { @@ -30,11 +32,33 @@ export default class extends Controller { }); this.selectAll = !this.selectAll; + + this.updateSelectedHours(); } checkboxClick(event) { const entryId = event.params.id; this.dirtyEntrys.add(entryId.toString()); + + this.updateSelectedHours(); + } + + updateSelectedHours() { + if (!this.hasSelectedHoursTarget) { + return; + } + + const seconds = this.checkboxTargets.reduce((accumulator, target) => { + if (!target.checked) { + return accumulator; + } + + return accumulator + Number(target.dataset.timeSpentSeconds); + }, 0); + + this.selectedHoursTarget.textContent = String( + Math.round((seconds / 3600) * 100) / 100, + ); } async submitFormRedirectWithIds(event) { diff --git a/assets/styles/app.css b/assets/styles/app.css index d6549ebe..f4342cb9 100644 --- a/assets/styles/app.css +++ b/assets/styles/app.css @@ -393,8 +393,12 @@ @apply flex gap-3 mb-3; } - .sticky-row { - @apply sticky bg-white text-black dark:bg-gray-900 dark:text-white border-0 bottom-0; + .sticky-actions { + @apply sticky bottom-0 flex flex-wrap items-center gap-5 px-6 py-4 bg-white text-black dark:bg-gray-900 dark:text-white border-t border-gray-300 dark:border-gray-600; + } + + .sticky-actions-sums { + @apply flex flex-wrap items-center gap-5 ml-auto; } .comma-list span:after { diff --git a/phpstan-baseline.neon b/phpstan-baseline.neon index d1208155..4b1e7e3c 100644 --- a/phpstan-baseline.neon +++ b/phpstan-baseline.neon @@ -2407,6 +2407,12 @@ parameters: count: 1 path: tests/Integration/Controller/InvoiceFullFlowTest.php + - + message: '#^Call to an undefined method object\:\:sumSelectableTimeSpentSecondsByFilterData\(\)\.$#' + identifier: method.notFound + count: 1 + path: tests/Integration/Controller/InvoiceFullFlowTest.php + - message: '#^Cannot call method getAmount\(\) on App\\Entity\\InvoiceEntry\|null\.$#' identifier: method.nonObject @@ -2797,10 +2803,16 @@ parameters: count: 1 path: tests/Integration/Repository/WorkerRepositoryTest.php + - + message: '#^Call to an undefined method object\:\:findBy\(\)\.$#' + identifier: method.notFound + count: 1 + path: tests/Integration/Repository/WorklogRepositoryTest.php + - message: '#^Call to an undefined method object\:\:findOneBy\(\)\.$#' identifier: method.notFound - count: 8 + count: 12 path: tests/Integration/Repository/WorklogRepositoryTest.php - @@ -2839,18 +2851,6 @@ parameters: count: 5 path: tests/Integration/Repository/WorklogRepositoryTest.php - - - message: '#^Property App\\Tests\\Integration\\Repository\\WorklogRepositoryTest\:\:\$entityManager \(Doctrine\\ORM\\EntityManagerInterface\) does not accept object\.$#' - identifier: assign.propertyType - count: 1 - path: tests/Integration/Repository/WorklogRepositoryTest.php - - - - message: '#^Property App\\Tests\\Integration\\Repository\\WorklogRepositoryTest\:\:\$entityManager is never read, only written\.$#' - identifier: property.onlyWritten - count: 1 - path: tests/Integration/Repository/WorklogRepositoryTest.php - - message: '#^Property App\\Tests\\Integration\\Repository\\WorklogRepositoryTest\:\:\$projectRepository \(App\\Repository\\ProjectRepository\) does not accept object\.$#' identifier: assign.propertyType diff --git a/src/Controller/InvoiceEntryWorklogController.php b/src/Controller/InvoiceEntryWorklogController.php index a3346185..778b1bbc 100644 --- a/src/Controller/InvoiceEntryWorklogController.php +++ b/src/Controller/InvoiceEntryWorklogController.php @@ -86,12 +86,14 @@ public function worklogs(Request $request, Invoice $invoice, InvoiceEntry $invoi $form->handleRequest($request); $worklogs = $worklogRepository->findByFilterData($project, $invoiceEntry, $filterData); + $totalTimeSpentSeconds = $worklogRepository->sumSelectableTimeSpentSecondsByFilterData($project, $invoiceEntry, $filterData); return $this->render('invoice_entry/worklogs.html.twig', [ 'form' => $form->createView(), 'invoice' => $invoice, 'invoiceEntry' => $invoiceEntry, 'worklogs' => $worklogs, + 'totalHours' => round($totalTimeSpentSeconds / 3600, 2), 'submitEndpoint' => $this->generateUrl('app_invoice_entry_select_worklogs', ['invoice' => $invoice->getId(), 'invoiceEntry' => $invoiceEntry->getId()]), ]); } diff --git a/src/Repository/WorklogRepository.php b/src/Repository/WorklogRepository.php index 7e933f4f..3666feea 100644 --- a/src/Repository/WorklogRepository.php +++ b/src/Repository/WorklogRepository.php @@ -10,6 +10,7 @@ use App\Enum\NonBillableVersionsEnum; use App\Model\Invoices\InvoiceEntryWorklogsFilterData; use Doctrine\Bundle\DoctrineBundle\Repository\ServiceEntityRepository; +use Doctrine\ORM\QueryBuilder; use Doctrine\ORM\Tools\Pagination\Paginator; use Doctrine\Persistence\ManagerRegistry; @@ -47,6 +48,40 @@ public function remove(Worklog $entity, bool $flush = false): void } public function findByFilterData(Project $project, InvoiceEntry $invoiceEntry, InvoiceEntryWorklogsFilterData $filterData): iterable + { + return $this->createFilterDataQueryBuilder($project, $invoiceEntry, $filterData) + ->getQuery() + ->execute(); + } + + /** + * Sum the time spent on the worklogs matching the given filter that can be + * added to the invoice entry. + * + * Already billed worklogs and worklogs held by another invoice entry are + * listed without a checkbox, so their time can never become part of the + * selection and must not be part of the total either. + */ + public function sumSelectableTimeSpentSecondsByFilterData(Project $project, InvoiceEntry $invoiceEntry, InvoiceEntryWorklogsFilterData $filterData): int + { + $qb = $this->createFilterDataQueryBuilder($project, $invoiceEntry, $filterData); + + $sum = $qb + ->select('SUM(worklog.timeSpentSeconds)') + ->andWhere('worklog.isBilled = FALSE OR worklog.isBilled is NULL') + ->andWhere($qb->expr()->orX( + $qb->expr()->isNull('worklog.invoiceEntry'), + $qb->expr()->eq('worklog.invoiceEntry', ':selectableInvoiceEntry') + )) + ->setParameter('selectableInvoiceEntry', $invoiceEntry) + ->getQuery() + ->getSingleScalarResult(); + + // SUM returns null when no worklogs match the filter. + return (int) $sum; + } + + private function createFilterDataQueryBuilder(Project $project, InvoiceEntry $invoiceEntry, InvoiceEntryWorklogsFilterData $filterData): QueryBuilder { $qb = $this->createQueryBuilder('worklog'); @@ -97,7 +132,7 @@ public function findByFilterData(Project $project, InvoiceEntry $invoiceEntry, I ))->setParameter('invoiceEntry', $invoiceEntry); } - return $qb->getQuery()->execute(); + return $qb; } public function updateProjectByIssue(Issue $issue, Project $project): int diff --git a/templates/invoice_entry/worklogs.html.twig b/templates/invoice_entry/worklogs.html.twig index d6d2da9a..f894bb7a 100644 --- a/templates/invoice_entry/worklogs.html.twig +++ b/templates/invoice_entry/worklogs.html.twig @@ -22,7 +22,7 @@
- +
- + {% endif %} - - -
@@ -55,6 +55,7 @@ {{ stimulus_target('entry-select', 'checkbox') }} {{ stimulus_action('entry-select', 'checkboxClick', 'click', {id: worklog.id}) }} data-id="{{ worklog.id }}" + data-time-spent-seconds="{{ worklog.timeSpentSeconds }}" value="{{ worklog.id }}" id="entry-{{ worklog.id }}" {{ owned_by_invoice_entry ? 'checked' : '' }} @@ -96,18 +97,21 @@ {% endfor %} {% if worklogs is empty %}
{{ 'invoices.list_no_records_found'|trans }}{{ 'invoices.list_no_records_found'|trans }}
- - - -
+ +
+ + + +
+ {{ 'worklog.selected_hours'|trans }}: 0 + {{ 'worklog.total_hours'|trans }}: {{ totalHours }} +
+
diff --git a/tests/Integration/Controller/InvoiceFullFlowTest.php b/tests/Integration/Controller/InvoiceFullFlowTest.php index e9a30f46..2aedce2b 100644 --- a/tests/Integration/Controller/InvoiceFullFlowTest.php +++ b/tests/Integration/Controller/InvoiceFullFlowTest.php @@ -10,6 +10,7 @@ use App\Enum\InvoiceEntryTypeEnum; use App\Enum\MaterialNumberEnum; use App\Model\Invoices\ConfirmData; +use App\Model\Invoices\InvoiceEntryWorklogsFilterData; use App\Repository\ClientRepository; use App\Repository\InvoiceEntryRepository; use App\Repository\InvoiceRepository; @@ -172,7 +173,32 @@ public function testFullInvoiceLifecycle(): void $this->assertSame(0.0, (float) $worklogEntry->getAmount()); // 6. Attach worklogs to the WORKLOG entry. + $worklogsCrawler = $client->request('GET', '/admin/invoices/'.$invoiceId.'/entries/'.$worklogEntryId.'/worklogs'); + $this->assertResponseIsSuccessful(); + $worklogRepository = static::getContainer()->get(WorklogRepository::class); + + // The sums bar reports the hours that can be selected in the filtered + // list, and each checkbox carries the time the Stimulus controller sums + // for the selection. The controller renders the page with the filter + // defaults, since an unsubmitted GET form leaves the filter untouched. + $expectedTotalHours = round($worklogRepository->sumSelectableTimeSpentSecondsByFilterData( + $project, + $worklogEntry, + new InvoiceEntryWorklogsFilterData() + ) / 3600, 2); + $this->assertGreaterThan(0, $expectedTotalHours); + + $sums = $worklogsCrawler->filter('.sticky-actions-sums'); + $this->assertCount(1, $sums); + $this->assertSame('0', trim($sums->filter('[data-entry-select-target="selectedHours"]')->text())); + $this->assertEqualsWithDelta( + $expectedTotalHours, + (float) $sums->filter('[data-total-hours]')->attr('data-total-hours'), + 0.001 + ); + $this->assertGreaterThan(0, $worklogsCrawler->filter('input[data-time-spent-seconds]')->count()); + $unbilled = $worklogRepository->findBy( ['project' => $project, 'isBilled' => false], ['id' => 'ASC'], diff --git a/tests/Integration/Repository/WorklogRepositoryTest.php b/tests/Integration/Repository/WorklogRepositoryTest.php index 8a32efaf..137a1f5c 100644 --- a/tests/Integration/Repository/WorklogRepositoryTest.php +++ b/tests/Integration/Repository/WorklogRepositoryTest.php @@ -2,18 +2,19 @@ namespace App\Tests\Integration\Repository; +use App\Entity\InvoiceEntry; +use App\Entity\Project; +use App\Entity\Version; use App\Entity\Worklog; use App\Model\Invoices\InvoiceEntryWorklogsFilterData; use App\Repository\InvoiceEntryRepository; use App\Repository\IssueRepository; use App\Repository\ProjectRepository; use App\Repository\WorklogRepository; -use Doctrine\ORM\EntityManagerInterface; use Symfony\Bundle\FrameworkBundle\Test\KernelTestCase; class WorklogRepositoryTest extends KernelTestCase { - private EntityManagerInterface $entityManager; private WorklogRepository $repository; private ProjectRepository $projectRepository; @@ -21,7 +22,6 @@ protected function setUp(): void { self::bootKernel(); $container = self::getContainer(); - $this->entityManager = $container->get(EntityManagerInterface::class); $this->repository = $container->get(WorklogRepository::class); $this->projectRepository = $container->get(ProjectRepository::class); } @@ -122,6 +122,160 @@ public function testFindByFilterDataOnlyAvailable(): void } } + public function testSumSelectableTimeSpentSecondsByFilterDataMatchesSelectableWorklogs(): void + { + $project = $this->projectRepository->findOneBy(['name' => 'project-0-0']); + $invoiceEntryRepo = self::getContainer()->get(InvoiceEntryRepository::class); + $invoiceEntry = $invoiceEntryRepo->findOneBy([], ['id' => 'ASC']); + $this->assertInstanceOf(Project::class, $project); + $this->assertInstanceOf(InvoiceEntry::class, $invoiceEntry); + + $filterData = new InvoiceEntryWorklogsFilterData(); + $filterData->onlyAvailable = false; + $filterData->worker = 'admin@test.local'; + + $expected = $this->sumSelectable($project, $invoiceEntry, $filterData); + + $this->assertGreaterThan(0, $expected); + $this->assertSame( + $expected, + $this->repository->sumSelectableTimeSpentSecondsByFilterData($project, $invoiceEntry, $filterData) + ); + } + + public function testSumSelectableTimeSpentSecondsByFilterDataWithVersionFilter(): void + { + $project = $this->projectRepository->findOneBy(['name' => 'project-0-0']); + $invoiceEntryRepo = self::getContainer()->get(InvoiceEntryRepository::class); + $invoiceEntry = $invoiceEntryRepo->findOneBy([], ['id' => 'ASC']); + $this->assertInstanceOf(Project::class, $project); + $this->assertInstanceOf(InvoiceEntry::class, $invoiceEntry); + + $version = $project->getVersions()->first(); + $this->assertInstanceOf(Version::class, $version); + + $filterData = new InvoiceEntryWorklogsFilterData(); + $filterData->onlyAvailable = false; + $filterData->version = $version; + + $expected = $this->sumSelectable($project, $invoiceEntry, $filterData); + + $this->assertGreaterThan(0, $expected); + $this->assertSame( + $expected, + $this->repository->sumSelectableTimeSpentSecondsByFilterData($project, $invoiceEntry, $filterData) + ); + } + + public function testSumSelectableTimeSpentSecondsByFilterDataExcludesBilledWorklogs(): void + { + $project = $this->projectRepository->findOneBy(['name' => 'project-0-0']); + $invoiceEntryRepo = self::getContainer()->get(InvoiceEntryRepository::class); + $invoiceEntry = $invoiceEntryRepo->findOneBy([], ['id' => 'ASC']); + $this->assertInstanceOf(Project::class, $project); + $this->assertInstanceOf(InvoiceEntry::class, $invoiceEntry); + + $filterData = new InvoiceEntryWorklogsFilterData(); + + $listed = 0; + $billed = 0; + foreach ($this->repository->findByFilterData($project, $invoiceEntry, $filterData) as $worklog) { + $this->assertInstanceOf(Worklog::class, $worklog); + $listed += (int) $worklog->getTimeSpentSeconds(); + + if ($worklog->isBilled()) { + $billed += (int) $worklog->getTimeSpentSeconds(); + } + } + + // The default filter lists billed worklogs, which the picker renders + // without a checkbox, so the total must not include them. + $this->assertGreaterThan(0, $billed); + $this->assertSame( + $listed - $billed, + $this->repository->sumSelectableTimeSpentSecondsByFilterData($project, $invoiceEntry, $filterData) + ); + } + + public function testSumSelectableTimeSpentSecondsByFilterDataExcludesWorklogsHeldByAnotherEntry(): void + { + $project = $this->projectRepository->findOneBy(['name' => 'project-0-0']); + $invoiceEntryRepo = self::getContainer()->get(InvoiceEntryRepository::class); + [$otherEntry, $invoiceEntry] = $invoiceEntryRepo->findBy([], ['id' => 'ASC'], 2); + $this->assertInstanceOf(Project::class, $project); + $this->assertInstanceOf(InvoiceEntry::class, $otherEntry); + $this->assertInstanceOf(InvoiceEntry::class, $invoiceEntry); + + $filterData = new InvoiceEntryWorklogsFilterData(); + $filterData->onlyAvailable = false; + + $listed = 0; + $notSelectable = 0; + $unbilledHeldByOther = 0; + foreach ($this->repository->findByFilterData($project, $invoiceEntry, $filterData) as $worklog) { + $this->assertInstanceOf(Worklog::class, $worklog); + $seconds = (int) $worklog->getTimeSpentSeconds(); + $listed += $seconds; + + $owner = $worklog->getInvoiceEntry(); + $heldByOther = null !== $owner && $owner->getId() !== $invoiceEntry->getId(); + + if ($worklog->isBilled() || $heldByOther) { + $notSelectable += $seconds; + } + + if (!$worklog->isBilled() && $owner?->getId() === $otherEntry->getId()) { + $unbilledHeldByOther += $seconds; + } + } + + // Guard the point of the test: without unbilled worklogs on another + // entry, the held-by-another-entry exclusion would pass untested. + $this->assertGreaterThan(0, $unbilledHeldByOther, 'Expected unbilled worklogs held by another invoice entry in fixtures.'); + $this->assertSame( + $listed - $notSelectable, + $this->repository->sumSelectableTimeSpentSecondsByFilterData($project, $invoiceEntry, $filterData) + ); + } + + public function testSumSelectableTimeSpentSecondsByFilterDataIsZeroWhenNothingMatches(): void + { + $project = $this->projectRepository->findOneBy(['name' => 'project-0-0']); + $invoiceEntryRepo = self::getContainer()->get(InvoiceEntryRepository::class); + $invoiceEntry = $invoiceEntryRepo->findOneBy([], ['id' => 'ASC']); + $this->assertInstanceOf(Project::class, $project); + $this->assertInstanceOf(InvoiceEntry::class, $invoiceEntry); + + $filterData = new InvoiceEntryWorklogsFilterData(); + $filterData->onlyAvailable = false; + $filterData->worker = 'no-such-worker@test.local'; + + $this->assertSame(0, $this->repository->sumSelectableTimeSpentSecondsByFilterData($project, $invoiceEntry, $filterData)); + } + + /** + * Sum the listed worklogs the picker offers a checkbox for, mirroring the + * disabled condition in invoice_entry/worklogs.html.twig. + */ + private function sumSelectable(Project $project, InvoiceEntry $invoiceEntry, InvoiceEntryWorklogsFilterData $filterData): int + { + $sum = 0; + + foreach ($this->repository->findByFilterData($project, $invoiceEntry, $filterData) as $worklog) { + $this->assertInstanceOf(Worklog::class, $worklog); + + $owner = $worklog->getInvoiceEntry(); + + if ($worklog->isBilled() || (null !== $owner && $owner->getId() !== $invoiceEntry->getId())) { + continue; + } + + $sum += (int) $worklog->getTimeSpentSeconds(); + } + + return $sum; + } + public function testFindWorklogsByWorkerAndDateRange(): void { $year = (new \DateTime())->format('Y'); diff --git a/translations/messages.da.yaml b/translations/messages.da.yaml index e921e756..a88a0a54 100644 --- a/translations/messages.da.yaml +++ b/translations/messages.da.yaml @@ -326,6 +326,8 @@ worklog: worker: "Arbejder" time_spent: "Timer" date: "Dato" + total_hours: "Timer i alt" + selected_hours: "Valgte timer" is_billed_true: "Ja" is_billed_false: "Nej" period_from: "Periode fra"