Support visibility for tags (#4086)

This commit is contained in:
Kevin Papst
2023-06-09 17:07:49 +02:00
committed by GitHub
parent 6e781b59e2
commit 2e2bf986a4
17 changed files with 246 additions and 161 deletions

View File

@@ -0,0 +1,36 @@
<?php
declare(strict_types=1);
namespace DoctrineMigrations;
use App\Doctrine\AbstractMigration;
use Doctrine\DBAL\Schema\Schema;
/**
* @version 2.0.26
*/
final class Version20230606125948 extends AbstractMigration
{
public function getDescription(): string
{
return 'Adds the visible column for tags';
}
public function up(Schema $schema): void
{
$tags = $schema->getTable('kimai2_tags');
$tags->addColumn('visible', 'boolean', ['notnull' => false, 'default' => true]);
}
public function down(Schema $schema): void
{
$tags = $schema->getTable('kimai2_tags');
$tags->dropColumn('visible');
}
public function isTransactional(): bool
{
return false;
}
}

View File

@@ -1381,26 +1381,6 @@ parameters:
count: 1
path: src/Controller/Security/SelfRegistrationController.php
-
message: "#^Cannot call method getData\\(\\) on Symfony\\\\Component\\\\Form\\\\FormInterface\\|null\\.$#"
count: 1
path: src/Controller/TagController.php
-
message: "#^Cannot call method handleRequest\\(\\) on Symfony\\\\Component\\\\Form\\\\FormInterface\\|null\\.$#"
count: 1
path: src/Controller/TagController.php
-
message: "#^Cannot call method isSubmitted\\(\\) on Symfony\\\\Component\\\\Form\\\\FormInterface\\|null\\.$#"
count: 1
path: src/Controller/TagController.php
-
message: "#^Cannot call method isValid\\(\\) on Symfony\\\\Component\\\\Form\\\\FormInterface\\|null\\.$#"
count: 1
path: src/Controller/TagController.php
-
message: "#^Cannot call method getEntities\\(\\) on mixed\\.$#"
count: 2
@@ -5416,26 +5396,6 @@ parameters:
count: 1
path: src/Repository/TagRepository.php
-
message: "#^Method App\\\\Repository\\\\TagRepository\\:\\:deleteTag\\(\\) has no return type specified\\.$#"
count: 1
path: src/Repository/TagRepository.php
-
message: "#^Method App\\\\Repository\\\\TagRepository\\:\\:findAllTagNames\\(\\) return type has no value type specified in iterable type array\\.$#"
count: 1
path: src/Repository/TagRepository.php
-
message: "#^Method App\\\\Repository\\\\TagRepository\\:\\:findTagsByName\\(\\) has parameter \\$tagNames with no value type specified in iterable type array\\.$#"
count: 1
path: src/Repository/TagRepository.php
-
message: "#^Method App\\\\Repository\\\\TagRepository\\:\\:saveTag\\(\\) has no return type specified\\.$#"
count: 1
path: src/Repository/TagRepository.php
-
message: "#^Cannot cast mixed to int\\.$#"
count: 1

View File

@@ -180,9 +180,10 @@ final class TimesheetController extends BaseApiController
$query->setPageSize((int) $size);
}
/** @var array<string> $tags */
$tags = $paramFetcher->get('tags');
if (\is_array($tags) && \count($tags) > 0) {
$tags = $this->tagRepository->findTagsByName($tags);
$tags = $this->tagRepository->findTagsByName($tags, true);
foreach ($tags as $tag) {
$query->addTag($tag);
}

View File

@@ -62,6 +62,7 @@ final class TagController extends AbstractController
$table->addColumn('name', ['class' => 'alwaysVisible']);
$table->addColumn('amount', ['class' => 'text-center w-min']);
$table->addColumn('visible', ['class' => 'd-none text-center w-min']);
$table->addColumn('actions', ['class' => 'actions']);
$page = new PageSetup('tags');
@@ -146,6 +147,8 @@ final class TagController extends AbstractController
public function multiDelete(TagRepository $repository, Request $request): Response
{
$form = $this->getMultiUpdateForm($repository);
if ($form !== null) {
$form->handleRequest($request);
if ($form->isSubmitted() && $form->isValid()) {
@@ -158,6 +161,47 @@ final class TagController extends AbstractController
$this->flashDeleteException($ex);
}
}
}
return $this->redirectToRoute('tags');
}
#[Route(path: '/multi-invisible', name: 'tags_multi_invisible', methods: ['POST'])]
#[IsGranted('manage_tag')]
public function multiInvisible(TagRepository $repository, Request $request): Response
{
return $this->multiUpdateVisible($repository, $request, false);
}
#[Route(path: '/multi-visible', name: 'tags_multi_visible', methods: ['POST'])]
#[IsGranted('manage_tag')]
public function multiVisible(TagRepository $repository, Request $request): Response
{
return $this->multiUpdateVisible($repository, $request, true);
}
private function multiUpdateVisible(TagRepository $repository, Request $request, bool $visible): Response
{
$form = $this->getMultiUpdateForm($repository);
if ($form !== null) {
$form->handleRequest($request);
if ($form->isSubmitted() && $form->isValid()) {
try {
/** @var MultiUpdateTableDTO $dto */
$dto = $form->getData();
/** @var Tag $tag */
foreach ($dto->getEntities() as $tag) {
$tag->setVisible($visible);
}
$repository->multiUpdate($dto->getEntities());
$this->flashSuccess('action.delete.success');
} catch (\Exception $ex) {
$this->flashDeleteException($ex);
}
}
}
return $this->redirectToRoute('tags');
}
@@ -165,6 +209,12 @@ final class TagController extends AbstractController
private function getMultiUpdateForm(TagRepository $repository): ?FormInterface
{
$dto = new MultiUpdateTableDTO();
if ($this->isGranted('manage_tag')) {
$dto->addAction('visible', $this->generateUrl('tags_multi_visible'));
$dto->addAction('invisible', $this->generateUrl('tags_multi_invisible'));
}
if ($this->isGranted('delete_tag')) {
$dto->addDelete($this->generateUrl('tags_multi_delete'));
}

View File

@@ -43,6 +43,11 @@ class Tag
#[Serializer\Expose]
#[Serializer\Groups(['Default'])]
private ?string $name = null;
#[ORM\Column(name: 'visible', type: 'boolean', nullable: false, options: ['default' => true])]
#[Assert\NotNull]
#[Serializer\Expose]
#[Serializer\Groups(['Default'])]
private bool $visible = true;
use ColorTrait;
@@ -74,6 +79,16 @@ class Tag
return $this->name;
}
public function isVisible(): bool
{
return $this->visible;
}
public function setVisible(bool $visible): void
{
$this->visible = $visible;
}
public function addTimesheet(Timesheet $timesheet): void
{
if ($this->timesheets->contains($timesheet)) {

View File

@@ -10,6 +10,7 @@
namespace App\Form;
use App\Entity\Tag;
use App\Form\Type\YesNoType;
use Symfony\Component\Form\AbstractType;
use Symfony\Component\Form\Extension\Core\Type\TextType;
use Symfony\Component\Form\FormBuilderInterface;
@@ -32,6 +33,10 @@ class TagEditForm extends AbstractType
'description' => 'The tag name (forbidden character: comma)',
],
])
->add('visible', YesNoType::class, [
'label' => 'visible',
'help' => 'help.visible',
])
;
$this->addColor($builder);
}

View File

@@ -23,6 +23,7 @@ final class TagToolbarForm extends AbstractType
public function buildForm(FormBuilderInterface $builder, array $options): void
{
$this->addVisibilityChoice($builder);
$this->addSearchTermInputField($builder);
$this->addPageSizeChoice($builder);
$this->addHiddenPagination($builder);

View File

@@ -59,6 +59,7 @@ final class TagsSelectType extends AbstractType
}
$newData = [];
/** @var array<string> $newNames */
$newNames = [];
foreach ($tagIds as $tag) {
if (!\in_array($tag, $foundIds, true)) {
@@ -68,8 +69,10 @@ final class TagsSelectType extends AbstractType
}
}
// in case someone is using tags like "1234" this can interfere with the ID
$tags = $this->tagRepository->findTagsByName($newNames);
// 1. in case someone is using tags like "1234" this can interfere with the ID
// 2. if we would load only visible tags, we would try to create new ones below
// and that would trigger the unique constraint
$tags = $this->tagRepository->findTagsByName($newNames, null);
$foundTagNames = [];
foreach ($tags as $tag) {
$newData[] = (string) $tag->getId();

View File

@@ -11,12 +11,15 @@ namespace App\Repository\Query;
class TagQuery extends BaseQuery
{
use VisibilityTrait;
public const TAG_ORDER_ALLOWED = ['name', 'amount'];
public function __construct()
{
$this->setDefaults([
'orderBy' => 'name',
'visibility' => VisibilityInterface::SHOW_VISIBLE,
]);
}
}

View File

@@ -14,8 +14,8 @@ use App\Repository\Paginator\QueryBuilderPaginator;
use App\Repository\Query\TagFormTypeQuery;
use App\Repository\Query\TagQuery;
use App\Utils\Pagination;
use Doctrine\DBAL\ParameterType;
use Doctrine\ORM\EntityRepository;
use Doctrine\ORM\Exception\ORMException;
use Doctrine\ORM\QueryBuilder;
/**
@@ -28,24 +28,14 @@ class TagRepository extends EntityRepository
*/
public const MAX_AMOUNT_SELECT = 500;
/**
* @param Tag $tag
* @throws ORMException
* @throws \Doctrine\ORM\OptimisticLockException
*/
public function saveTag(Tag $tag)
public function saveTag(Tag $tag): void
{
$entityManager = $this->getEntityManager();
$entityManager->persist($tag);
$entityManager->flush();
}
/**
* @param Tag $tag
* @throws ORMException
* @throws \Doctrine\ORM\OptimisticLockException
*/
public function deleteTag(Tag $tag)
public function deleteTag(Tag $tag): void
{
$entityManager = $this->getEntityManager();
$entityManager->remove($tag);
@@ -53,26 +43,33 @@ class TagRepository extends EntityRepository
}
/**
* @param array $tagNames
* @param array<string> $tagNames
* @return array<Tag>
*/
public function findTagsByName(array $tagNames): array
public function findTagsByName(array $tagNames, ?bool $visible = null): array
{
if ($visible === null) {
return $this->findBy(['name' => $tagNames]);
}
public function findTagByName(string $tagName): ?Tag
return $this->findBy(['name' => $tagNames, 'visible' => $visible]);
}
public function findTagByName(string $tagName, ?bool $visible = null): ?Tag
{
if ($visible === null) {
return $this->findOneBy(['name' => $tagName]);
}
return $this->findOneBy(['name' => $tagName, 'visible' => $visible]);
}
/**
* Find all tag names in an alphabetical order
* Find all visible tag names in alphabetical order.
*
* @param string $filter
* @return array
* @return array<string>
*/
public function findAllTagNames($filter = null): array
public function findAllTagNames(?string $filter = null): array
{
$qb = $this->createQueryBuilder('t');
@@ -80,10 +77,12 @@ class TagRepository extends EntityRepository
->select('t.name')
->addOrderBy('t.name', 'ASC');
$qb->andWhere($qb->expr()->eq('t.visible', ':visible'));
$qb->setParameter('visible', true, ParameterType::BOOLEAN);
if (null !== $filter) {
$qb
->andWhere('t.name LIKE :filter')
->setParameter('filter', '%' . $filter . '%');
$qb->andWhere('t.name LIKE :filter');
$qb->setParameter('filter', '%' . $filter . '%');
}
return array_column($qb->getQuery()->getScalarResult(), 'name');
@@ -123,7 +122,7 @@ class TagRepository extends EntityRepository
{
$qb = $this->createQueryBuilder('tag');
$qb->select('tag.id, tag.name, tag.color, SIZE(tag.timesheets) as amount');
$qb->select('tag.id, tag.name, tag.color, tag.visible, SIZE(tag.timesheets) as amount');
$orderBy = $query->getOrderBy();
$orderBy = match ($orderBy) {
@@ -131,6 +130,14 @@ class TagRepository extends EntityRepository
default => 'tag.' . $orderBy,
};
if ($query->isShowVisible()) {
$qb->andWhere($qb->expr()->eq('tag.visible', ':visible'));
$qb->setParameter('visible', true, ParameterType::BOOLEAN);
} elseif ($query->isShowHidden()) {
$qb->andWhere($qb->expr()->eq('tag.visible', ':visible'));
$qb->setParameter('visible', false, ParameterType::BOOLEAN);
}
$qb->addOrderBy($orderBy, $query->getOrder());
if ($query->hasSearchTerm()) {
@@ -159,6 +166,8 @@ class TagRepository extends EntityRepository
$qb = $this->createQueryBuilder('tag');
$qb->orderBy('tag.name', 'ASC');
$qb->andWhere($qb->expr()->eq('tag.visible', ':visible'));
$qb->setParameter('visible', true, ParameterType::BOOLEAN);
return $qb;
}
@@ -183,4 +192,25 @@ class TagRepository extends EntityRepository
throw $ex;
}
}
/**
* @param Tag[] $tags
* @throws \Exception
*/
public function multiUpdate(iterable $tags): void
{
$em = $this->getEntityManager();
$em->beginTransaction();
try {
foreach ($tags as $tag) {
$em->persist($tag);
}
$em->flush();
$em->commit();
} catch (\Exception $ex) {
$em->rollback();
throw $ex;
}
}
}

View File

@@ -405,6 +405,14 @@
{%- if not customer.visible %}bg-orange-lt{% endif -%}
{% endmacro %}
{#
To be used like this:
<tr class="{{ class_tag_row(tag) }}">
#}
{% macro class_tag_row(tag) %}
{%- if not tag.visible %}bg-orange-lt{% endif -%}
{% endmacro %}
{#
To be used like this:
<tr {{ customer_row_attr(customer, now) }}>

View File

@@ -4,25 +4,21 @@
{% set manageAllowed = is_granted('manage_tag') %}
{% block datatable_row %}
{% set type = '' %}
{% if entry.amount == 0 %}
{% set type = 'bg-yellow' %}
{% endif %}
{% block datatable_row_attr %}{% if manageAllowed %} class="modal-ajax-form open-edit {{ widgets.class_tag_row(entry) }}" data-href="{{ path('tags_edit', {'id': entry.id}) }}" {% endif %}{% endblock %}
<tr{% if manageAllowed %} class="modal-ajax-form open-edit" data-href="{{ path('tags_edit', {'id': entry.id}) }}"{% endif %}>
{% if dataTable.hasBatchForm() %}
<td class="text-nowrap">
{{ tables.datatable_multiupdate_row(entry.id) }}
</td>
{% endif %}
<td class="{{ tables.class(dataTable, 'name') }}">
{% block datatable_column_value %}
{% if column == 'name' %}
{{ widgets.label_name(entry.name, entry.color|colorize(entry.name)) }}
</td>
<td class="{{ tables.class(dataTable, 'amount') }}">{{ widgets.badge_counter(entry.amount, null, type) }}</td>
<td class="{{ tables.class(dataTable, 'actions') }}">
{% elseif column == 'amount' %}
{{ widgets.badge_counter(entry.amount, null, (entry.amount == 0 ? 'bg-yellow' : '')) }}
{% elseif column == 'visible' %}
{{ widgets.label_visible(entry.visible) }}
{% elseif column == 'actions' %}
{% set event = actions(app.user, 'tag', 'index', {'tag': entry}) %}
{{ widgets.table_actions(event.actions) }}
</td>
</tr>
{% elseif column == 'id' %}
{% if dataTable.hasBatchForm() %}
{{ tables.datatable_multiupdate_row(entry.id) }}
{% endif %}
{% endif %}
{% endblock %}

View File

@@ -313,6 +313,7 @@ abstract class APIControllerBaseTest extends ControllerBaseTest
'id' => 'int',
'name' => 'string',
'color' => '@string',
'visible' => 'bool',
];
// embedded meta data

View File

@@ -33,17 +33,20 @@ class TagControllerTest extends APIControllerBaseTest
return $this->importFixture($fixture);
}
public function testIsSecure()
public function testIsSecure(): void
{
$this->assertUrlIsSecured('/api/tags');
}
public function testGetCollection()
public function testGetCollection(): void
{
$client = $this->getClientForAuthenticatedUser(User::ROLE_USER);
$this->importTagFixtures();
$this->assertAccessIsGranted($client, '/api/tags');
$result = json_decode($client->getResponse()->getContent(), true);
$content = $client->getResponse()->getContent();
$this->assertNotFalse($content);
$result = json_decode($content, true);
$this->assertIsArray($result);
$this->assertNotEmpty($result);
@@ -51,20 +54,23 @@ class TagControllerTest extends APIControllerBaseTest
$this->assertEquals('Test', $result[9]);
}
public function testEmptyCollection()
public function testEmptyCollection(): void
{
$client = $this->getClientForAuthenticatedUser(User::ROLE_USER);
$this->importTagFixtures();
$query = ['name' => 'nothing'];
$this->assertAccessIsGranted($client, '/api/tags', 'GET', $query);
$result = json_decode($client->getResponse()->getContent(), true);
$content = $client->getResponse()->getContent();
$this->assertNotFalse($content);
$result = json_decode($content, true);
$this->assertIsArray($result);
$this->assertEmpty($result);
$this->assertEquals(0, \count($result));
}
public function testPostAction()
public function testPostAction(): void
{
$client = $this->getClientForAuthenticatedUser(User::ROLE_ADMIN);
$this->importTagFixtures();
@@ -75,14 +81,17 @@ class TagControllerTest extends APIControllerBaseTest
$this->request($client, '/api/tags', 'POST', [], json_encode($data));
$this->assertTrue($client->getResponse()->isSuccessful());
$result = json_decode($client->getResponse()->getContent(), true);
$content = $client->getResponse()->getContent();
$this->assertNotFalse($content);
$result = json_decode($content, true);
$this->assertIsArray($result);
self::assertApiResponseTypeStructure('TagEntity', $result);
$this->assertNotEmpty($result['id']);
self::assertEquals('#00ff00', $result['color']);
}
public function testPostActionWithValidationErrors()
public function testPostActionWithValidationErrors(): void
{
$client = $this->getClientForAuthenticatedUser(User::ROLE_ADMIN);
$this->importTagFixtures();
@@ -96,7 +105,7 @@ class TagControllerTest extends APIControllerBaseTest
$this->assertApiCallValidationError($response, ['name', 'color']);
}
public function testPostActionAsRegularUser()
public function testPostActionAsRegularUser(): void
{
$client = $this->getClientForAuthenticatedUser(User::ROLE_USER);
$this->importTagFixtures();
@@ -106,20 +115,26 @@ class TagControllerTest extends APIControllerBaseTest
$this->request($client, '/api/tags', 'POST', [], json_encode($data));
$this->assertTrue($client->getResponse()->isSuccessful());
$result = json_decode($client->getResponse()->getContent(), true);
$content = $client->getResponse()->getContent();
$this->assertNotFalse($content);
$result = json_decode($content, true);
$this->assertIsArray($result);
self::assertApiResponseTypeStructure('TagEntity', $result);
$this->assertNotEmpty($result['id']);
self::assertEquals('foo', $result['name']);
}
public function testPartOfEntries()
public function testPartOfEntries(): void
{
$client = $this->getClientForAuthenticatedUser(User::ROLE_USER);
$this->importTagFixtures();
$query = ['name' => 'in'];
$this->assertAccessIsGranted($client, '/api/tags', 'GET', $query);
$result = json_decode($client->getResponse()->getContent(), true);
$content = $client->getResponse()->getContent();
$this->assertNotFalse($content);
$result = json_decode($content, true);
$this->assertIsArray($result);
$this->assertNotEmpty($result);
@@ -130,7 +145,7 @@ class TagControllerTest extends APIControllerBaseTest
$this->assertEquals('Marketing', $result[2]);
}
public function testDeleteAction()
public function testDeleteAction(): void
{
$client = $this->getClientForAuthenticatedUser(User::ROLE_ADMIN);
$tags = $this->importTagFixtures();
@@ -142,12 +157,15 @@ class TagControllerTest extends APIControllerBaseTest
$this->assertEmpty($client->getResponse()->getContent());
$this->assertAccessIsGranted($client, '/api/tags');
$result = json_decode($client->getResponse()->getContent(), true);
$content = $client->getResponse()->getContent();
$this->assertNotFalse($content);
$result = json_decode($content, true);
$this->assertEquals(9, \count($result));
}
public function testDeleteActionWithUnknownTimesheet()
public function testDeleteActionWithUnknownTimesheet(): void
{
$client = $this->getClientForAuthenticatedUser(User::ROLE_ADMIN);
$this->assertNotFoundForDelete($client, '/api/tags/' . PHP_INT_MAX);

View File

@@ -1117,56 +1117,6 @@ parameters:
count: 3
path: API/StatusControllerTest.php
-
message: "#^Method App\\\\Tests\\\\API\\\\TagControllerTest\\:\\:testDeleteAction\\(\\) has no return type specified\\.$#"
count: 1
path: API/TagControllerTest.php
-
message: "#^Method App\\\\Tests\\\\API\\\\TagControllerTest\\:\\:testDeleteActionWithUnknownTimesheet\\(\\) has no return type specified\\.$#"
count: 1
path: API/TagControllerTest.php
-
message: "#^Method App\\\\Tests\\\\API\\\\TagControllerTest\\:\\:testEmptyCollection\\(\\) has no return type specified\\.$#"
count: 1
path: API/TagControllerTest.php
-
message: "#^Method App\\\\Tests\\\\API\\\\TagControllerTest\\:\\:testGetCollection\\(\\) has no return type specified\\.$#"
count: 1
path: API/TagControllerTest.php
-
message: "#^Method App\\\\Tests\\\\API\\\\TagControllerTest\\:\\:testIsSecure\\(\\) has no return type specified\\.$#"
count: 1
path: API/TagControllerTest.php
-
message: "#^Method App\\\\Tests\\\\API\\\\TagControllerTest\\:\\:testPartOfEntries\\(\\) has no return type specified\\.$#"
count: 1
path: API/TagControllerTest.php
-
message: "#^Method App\\\\Tests\\\\API\\\\TagControllerTest\\:\\:testPostAction\\(\\) has no return type specified\\.$#"
count: 1
path: API/TagControllerTest.php
-
message: "#^Method App\\\\Tests\\\\API\\\\TagControllerTest\\:\\:testPostActionAsRegularUser\\(\\) has no return type specified\\.$#"
count: 1
path: API/TagControllerTest.php
-
message: "#^Method App\\\\Tests\\\\API\\\\TagControllerTest\\:\\:testPostActionWithValidationErrors\\(\\) has no return type specified\\.$#"
count: 1
path: API/TagControllerTest.php
-
message: "#^Parameter \\#1 \\$json of function json_decode expects string, string\\|false given\\.$#"
count: 6
path: API/TagControllerTest.php
-
message: "#^Parameter \\#1 \\$value of function count expects array\\|Countable, mixed given\\.$#"
count: 1

View File

@@ -343,6 +343,10 @@
<source>help.visible</source>
<target>Falls die Einstellung aus ist, wird das Objekt nicht in Auswahlboxen und Listen angezeigt.</target>
</trans-unit>
<trans-unit id="t1r372x" resname="invisible">
<source>invisible</source>
<target>Unsichtbar</target>
</trans-unit>
<trans-unit id="CvlqjtY" approved="yes" resname="budget">
<source>budget</source>
<target>Budget</target>

View File

@@ -343,6 +343,10 @@
<source>help.visible</source>
<target>If the setting is off, the object is not displayed in dropdown-boxes and list-views.</target>
</trans-unit>
<trans-unit id="t1r372x" resname="invisible">
<source>invisible</source>
<target>Invisible</target>
</trans-unit>
<trans-unit id="CvlqjtY" resname="budget">
<source>budget</source>
<target>Budget</target>