several enhancements (#1808)

* show all fields in timesheet api docs
* fix action dropdown links #1806
* apply same validation rules for passwords and for api-tokens #1753
* added "duplicate team" action #1136
This commit is contained in:
Kevin Papst
2020-07-10 03:57:15 +02:00
committed by GitHub
parent ea39d5c74d
commit 19e4ebf88c
22 changed files with 196 additions and 15 deletions

View File

@@ -0,0 +1,12 @@
App\Entity\Tag:
exclusion_policy: All
custom_accessor_order: [id, name, color]
properties:
id:
include: true
name:
include: true
color:
include: true
timesheets:
exclude: true

View File

@@ -11,3 +11,6 @@ App\Entity\User:
plainPassword:
- NotBlank: { groups: [Registration, PasswordUpdate] }
- Length: { min: 8, max: 60, groups: [Registration, PasswordUpdate] }
plainApiToken:
- NotBlank: { groups: [ApiTokenUpdate] }
- Length: { min: 8, max: 60, groups: [ApiTokenUpdate] }

View File

@@ -94,7 +94,7 @@ final class ConfigurationController extends BaseApiController
}
/**
* Returns the instance specific timesheet configuration
* Returns the timesheet configuration
*
* @SWG\Response(
* response=200,
@@ -116,6 +116,7 @@ final class ConfigurationController extends BaseApiController
->setActiveEntriesHardLimit($this->timesheetConfiguration->getActiveEntriesHardLimit())
->setActiveEntriesSoftLimit($this->timesheetConfiguration->getActiveEntriesSoftLimit())
->setIsAllowFutureTimes($this->timesheetConfiguration->isAllowFutureTimes())
->setIsAllowOverlapping($this->timesheetConfiguration->isAllowOverlappingRecords())
;
$view = new View($model, 200);

View File

@@ -20,21 +20,35 @@ final class TimesheetConfig
*/
private $trackingMode = 'default';
/**
* Default begin datetime in PHP format
*
* @var string
*/
private $defaultBeginTime = 'now';
/**
* How many running timesheets a user is allowed to have at the same time
*
* @var int
*/
private $activeEntriesHardLimit = 1;
/**
* How many running timesheets a user is allowed before a warning is shown
*
* @var int
*/
private $activeEntriesSoftLimit = 1;
/**
* Whether entries for future times are allowed
*
* @var bool
*/
private $isAllowFutureTimes = true;
/**
* Whether overlapping entries are allowed
*
* @var bool
*/
private $isAllowOverlapping = true;
/**
* @return string
@@ -98,4 +112,16 @@ final class TimesheetConfig
return $this;
}
public function isAllowOverlapping(): bool
{
return $this->isAllowOverlapping;
}
public function setIsAllowOverlapping(bool $isAllowOverlapping): TimesheetConfig
{
$this->isAllowOverlapping = $isAllowOverlapping;
return $this;
}
}

View File

@@ -16,22 +16,32 @@ use App\Constants;
class Version
{
/**
* Kimai Version, eg. "1.9"
*
* @var string
*/
protected $version = Constants::VERSION;
/**
* Candidate: either "prod" or "dev"
*
* @var string
*/
protected $candidate = Constants::STATUS;
/**
* Full version including status, eg: "1.9-prod"
*
* @var string
*/
protected $semver = Constants::VERSION . '-' . Constants::STATUS;
/**
* The version name
*
* @var string
*/
protected $name = Constants::NAME;
/**
* A full copyright notice
*
* @var string
*/
protected $copyright = Constants::SOFTWARE . ' - ' . Constants::VERSION . ' ' . Constants::STATUS . ' (' . Constants::NAME . ') by Kevin Papst and contributors.';

View File

@@ -93,6 +93,7 @@ final class UserController extends BaseApiController
public function cgetAction(ParamFetcherInterface $paramFetcher): Response
{
$query = new UserQuery();
$query->setCurrentUser($this->getUser());
if (null !== ($visible = $paramFetcher->get('visible'))) {
$query->setVisibility($visible);

View File

@@ -376,7 +376,6 @@ class ProfileController extends AbstractController
UserApiTokenType::class,
$user,
[
'validation_groups' => ['apiTokenUpdate'],
'action' => $this->generateUrl('user_profile_api_token', ['username' => $user->getUsername()]),
'method' => 'POST'
]

View File

@@ -83,6 +83,42 @@ final class TeamController extends AbstractController
return $this->renderEditScreen(new Team(), $request);
}
/**
* @Route(path="/{id}/duplicate", name="team_duplicate", methods={"GET", "POST"})
* @Security("is_granted('edit', team) and is_granted('create_team')")
*/
public function duplicateTeam(Team $team, Request $request)
{
$newTeam = new Team();
$newTeam->setName($team->getName() . ' [COPY]');
$newTeam->setTeamLead($team->getTeamLead());
foreach ($team->getUsers() as $user) {
$newTeam->addUser($user);
}
foreach ($team->getCustomers() as $customer) {
$newTeam->addCustomer($customer);
}
foreach ($team->getProjects() as $project) {
$newTeam->addProject($project);
}
try {
// make sure that the teamlead is always part of the team, otherwise permission checks
// and filtering might not work as expected!
$team->addUser($team->getTeamLead());
$this->repository->saveTeam($newTeam);
$this->flashSuccess('action.update.success');
return $this->redirectToRoute('admin_team_edit', ['id' => $newTeam->getId()]);
} catch (\Exception $ex) {
$this->flashError('action.update.error', ['%reason%' => $ex->getMessage()]);
}
return $this->redirectToRoute('admin_team');
}
/**
* @Route(path="/{id}/edit", name="admin_team_edit", methods={"GET", "POST"})
* @Security("is_granted('edit', team)")

View File

@@ -70,6 +70,7 @@ final class UserController extends AbstractController
public function indexAction($page, Request $request): Response
{
$query = new UserQuery();
$query->setCurrentUser($this->getUser());
$query->setPage($page);
$form = $this->getToolbarForm($query);

View File

@@ -50,6 +50,11 @@ class TimesheetApiEditForm extends TimesheetEditForm
$resolver->setDefaults([
'csrf_protection' => false,
'allow_duration' => false,
// overwritten and changed to default "true",
// because the docs are cached without these fields otherwise
'include_user' => true,
'include_exported' => true,
'include_rate' => true,
]);
}
}

View File

@@ -41,6 +41,7 @@ class UserApiTokenType extends AbstractType
public function configureOptions(OptionsResolver $resolver)
{
$resolver->setDefaults([
'validation_groups' => ['ApiTokenUpdate'],
'data_class' => User::class,
'csrf_protection' => true,
'csrf_field_name' => '_token',

View File

@@ -30,4 +30,19 @@ trait VisibilityTrait
return $this;
}
public function isShowHidden(): bool
{
return $this->visibility === VisibilityInterface::SHOW_HIDDEN;
}
public function isShowVisible(): bool
{
return $this->visibility === VisibilityInterface::SHOW_VISIBLE;
}
public function isShowBoth(): bool
{
return $this->visibility === VisibilityInterface::SHOW_BOTH;
}
}

View File

@@ -32,7 +32,7 @@
{% set actions = actions|merge({'divider': null}) %}
{% endif %}
{% if is_granted('view_other_timesheet') %}
{% set actions = actions|merge({'timesheet': path('admin_timesheet', {'customer': activity.project ? activity.project.customer.id : null, 'project': activity.project ? activity.project.id : null, 'activity': activity.id})}) %}
{% set actions = actions|merge({'timesheet': path('admin_timesheet', {'customers[]': activity.project ? activity.project.customer.id : null, 'projects[]': activity.project ? activity.project.id : null, 'activities[]': activity.id})}) %}
{% endif %}
{% if is_granted('create_other_timesheet') %}
{% set actions = actions|merge({'create-timesheet': {'url': path('admin_timesheet_create', {'project': activity.project ? activity.project.id : null, 'activity': activity.id}), 'class': 'modal-ajax-form'}}) %}

View File

@@ -39,13 +39,13 @@
{% set actions = actions|merge({'divider': null}) %}
{% endif %}
{% if is_granted('view_project') or is_granted('view_teamlead_project') or is_granted('view_team_project') %}
{% set actions = actions|merge({'project': path('admin_project', {'customer': customer.id})}) %}
{% set actions = actions|merge({'project': path('admin_project', {'customers[]': customer.id})}) %}
{% endif %}
{% if is_granted('view_activity') %}
{% set actions = actions|merge({'activity': path('admin_activity', {'customer': customer.id})}) %}
{% set actions = actions|merge({'activity': path('admin_activity', {'customers[]': customer.id})}) %}
{% endif %}
{% if is_granted('view_other_timesheet') %}
{% set actions = actions|merge({'timesheet': path('admin_timesheet', {'customer': customer.id})}) %}
{% set actions = actions|merge({'timesheet': path('admin_timesheet', {'customers[]': customer.id})}) %}
{% endif %}
{% if customer.visible and is_granted('create_project') %}
{% set actions = actions|merge({'create-project': path('admin_project_create_with_customer', {'customer': customer.id})}) %}

View File

@@ -40,10 +40,10 @@
{% set actions = actions|merge({'divider': null}) %}
{% endif %}
{% if is_granted('view_activity') %}
{% set actions = actions|merge({'activity': path('admin_activity', {'customer': project.customer.id, 'project': project.id})}) %}
{% set actions = actions|merge({'activity': path('admin_activity', {'customers[]': project.customer.id, 'projects[]': project.id})}) %}
{% endif %}
{% if is_granted('view_other_timesheet') %}
{% set actions = actions|merge({'timesheet': path('admin_timesheet', {'customer': project.customer.id, 'project': project.id})}) %}
{% set actions = actions|merge({'timesheet': path('admin_timesheet', {'customers[]': project.customer.id, 'projects[]': project.id})}) %}
{% endif %}
{% if is_granted('create_activity') and project.visible and project.customer.visible %}
{% set actions = actions|merge({'create-activity': path('admin_activity_create_with_project', {'project': project.id})}) %}

View File

@@ -21,6 +21,9 @@
{% if is_granted('edit', team) %}
{% set class = '' %}
{% set actions = actions|merge({'edit': {'url': path('admin_team_edit', {'id': team.id}), 'class': class}}) %}
{% if is_granted('create_team') %}
{% set actions = actions|merge({'copy': {'url': path('team_duplicate', {'id': team.id})}}) %}
{% endif %}
{% endif %}
{% endif %}

View File

@@ -56,13 +56,13 @@ class ConfigurationControllerTest extends APIControllerBaseTest
$this->assertIsArray($result);
$this->assertNotEmpty($result);
$this->assertEquals(5, \count($result));
$this->assertEquals(6, \count($result));
$this->assertTimesheetStructure($result);
}
protected function assertTimesheetStructure(array $result)
{
$expectedKeys = ['activeEntriesHardLimit', 'activeEntriesSoftLimit', 'defaultBeginTime', 'isAllowFutureTimes', 'trackingMode'];
$expectedKeys = ['activeEntriesHardLimit', 'activeEntriesSoftLimit', 'defaultBeginTime', 'isAllowFutureTimes', 'isAllowOverlapping', 'trackingMode'];
$actual = array_keys($result);
sort($actual);
sort($expectedKeys);

View File

@@ -21,6 +21,7 @@ class TimesheetConfigTest extends TestCase
{
$sut = new TimesheetConfig();
$this->assertTrue($sut->isAllowFutureTimes());
$this->assertTrue($sut->isAllowOverlapping());
$this->assertEquals('now', $sut->getDefaultBeginTime());
$this->assertEquals('default', $sut->getTrackingMode());
$this->assertEquals(1, $sut->getActiveEntriesSoftLimit());
@@ -32,12 +33,14 @@ class TimesheetConfigTest extends TestCase
$sut = new TimesheetConfig();
$this->assertInstanceOf(TimesheetConfig::class, $sut->setIsAllowFutureTimes(false));
$this->assertInstanceOf(TimesheetConfig::class, $sut->setIsAllowOverlapping(false));
$this->assertInstanceOf(TimesheetConfig::class, $sut->setDefaultBeginTime('08:00'));
$this->assertInstanceOf(TimesheetConfig::class, $sut->setTrackingMode('punch'));
$this->assertInstanceOf(TimesheetConfig::class, $sut->setActiveEntriesSoftLimit(2));
$this->assertInstanceOf(TimesheetConfig::class, $sut->setActiveEntriesHardLimit(3));
$this->assertFalse($sut->isAllowFutureTimes());
$this->assertFalse($sut->isAllowOverlapping());
$this->assertEquals('08:00', $sut->getDefaultBeginTime());
$this->assertEquals('punch', $sut->getTrackingMode());
$this->assertEquals(2, $sut->getActiveEntriesSoftLimit());

View File

@@ -132,7 +132,7 @@ class TagControllerTest extends APIControllerBaseTest
protected function assertStructure(array $result, $full = true)
{
$expectedKeys = [
'id', 'name', 'color', 'timesheets'
'id', 'name', 'color'
];
if ($full) {

View File

@@ -243,6 +243,24 @@ class ProfileControllerTest extends ControllerBaseTest
$this->assertTrue($passwordEncoder->getEncoder($user)->isPasswordValid($user->getPassword(), 'test1234', $user->getSalt()));
}
public function testPasswordActionFailsIfPasswordLengthToShort()
{
$this->assertFormHasValidationError(
User::ROLE_USER,
'/profile/' . UserFixtures::USERNAME_USER . '/password',
'form[name=user_password]',
[
'user_password' => [
'plainPassword' => [
'first' => 'abcdef1',
'second' => 'abcdef1',
]
]
],
['#user_password_plainPassword_first']
);
}
public function testApiTokenAction()
{
$client = $this->getClientForAuthenticatedUser(User::ROLE_USER);
@@ -255,15 +273,15 @@ class ProfileControllerTest extends ControllerBaseTest
$passwordEncoder = static::$kernel->getContainer()->get('test.PasswordEncoder');
$this->assertTrue($passwordEncoder->getEncoder($user)->isPasswordValid($user->getApiToken(), UserFixtures::DEFAULT_API_TOKEN, $user->getSalt()));
$this->assertFalse($passwordEncoder->getEncoder($user)->isPasswordValid($user->getApiToken(), 'test123', $user->getSalt()));
$this->assertFalse($passwordEncoder->getEncoder($user)->isPasswordValid($user->getApiToken(), 'test1234', $user->getSalt()));
$this->assertEquals(UserFixtures::USERNAME_USER, $user->getUsername());
$form = $client->getCrawler()->filter('form[name=user_api_token]')->form();
$client->submit($form, [
'user_api_token' => [
'plainApiToken' => [
'first' => 'test123',
'second' => 'test123',
'first' => 'test1234',
'second' => 'test1234',
]
]
]);
@@ -278,7 +296,25 @@ class ProfileControllerTest extends ControllerBaseTest
$user = $this->getUserByRole(User::ROLE_USER);
$this->assertFalse($passwordEncoder->getEncoder($user)->isPasswordValid($user->getApiToken(), UserFixtures::DEFAULT_API_TOKEN, $user->getSalt()));
$this->assertTrue($passwordEncoder->getEncoder($user)->isPasswordValid($user->getApiToken(), 'test123', $user->getSalt()));
$this->assertTrue($passwordEncoder->getEncoder($user)->isPasswordValid($user->getApiToken(), 'test1234', $user->getSalt()));
}
public function testApiTokenActionFailsIfPasswordLengthToShort()
{
$this->assertFormHasValidationError(
User::ROLE_USER,
'/profile/' . UserFixtures::USERNAME_USER . '/api-token',
'form[name=user_api_token]',
[
'user_api_token' => [
'plainApiToken' => [
'first' => 'abcdef1',
'second' => 'abcdef1',
]
]
],
['#user_api_token_plainApiToken_first']
);
}
public function testRolesActionIsSecured()

View File

@@ -205,4 +205,15 @@ class TeamControllerTest extends ControllerBaseTest
$team = $em->getRepository(Team::class)->find(1);
self::assertEquals(1, \count($team->getProjects()));
}
public function testDuplicateAction()
{
$client = $this->getClientForAuthenticatedUser(User::ROLE_ADMIN);
$this->request($client, '/admin/teams/1/duplicate');
$this->assertIsRedirect($client, $this->createUrl('/admin/teams/2/edit'));
$client->followRedirect();
$node = $client->getCrawler()->filter('#team_edit_form_name');
self::assertEquals(1, $node->count());
self::assertEquals('Test team [COPY]', $node->attr('value'));
}
}

View File

@@ -25,18 +25,27 @@ class VisibilityQueryTest extends TestCase
$sut = new VisibilityQuery();
$this->assertEquals(VisibilityQuery::SHOW_VISIBLE, $sut->getVisibility());
self::assertTrue($sut->isShowVisible());
self::assertFalse($sut->isShowHidden());
self::assertFalse($sut->isShowBoth());
$sut->setVisibility('foo-bar');
$this->assertEquals(VisibilityQuery::SHOW_VISIBLE, $sut->getVisibility());
$sut->setVisibility('2');
$this->assertEquals(VisibilityQuery::SHOW_HIDDEN, $sut->getVisibility());
self::assertFalse($sut->isShowVisible());
self::assertTrue($sut->isShowHidden());
self::assertFalse($sut->isShowBoth());
$sut->setVisibility('0'); // keep the value that was previously set
$this->assertEquals(VisibilityQuery::SHOW_HIDDEN, $sut->getVisibility());
$sut->setVisibility(VisibilityQuery::SHOW_BOTH);
$this->assertEquals(VisibilityQuery::SHOW_BOTH, $sut->getVisibility());
self::assertFalse($sut->isShowVisible());
self::assertFalse($sut->isShowHidden());
self::assertTrue($sut->isShowBoth());
$sut->setVisibility(VisibilityQuery::SHOW_HIDDEN);
$this->assertEquals(VisibilityQuery::SHOW_HIDDEN, $sut->getVisibility());
@@ -50,18 +59,27 @@ class VisibilityQueryTest extends TestCase
$sut = new VisibilityTraitImpl();
$this->assertEquals(VisibilityInterface::SHOW_VISIBLE, $sut->getVisibility());
self::assertTrue($sut->isShowVisible());
self::assertFalse($sut->isShowHidden());
self::assertFalse($sut->isShowBoth());
$sut->setVisibility('foo-bar');
$this->assertEquals(VisibilityInterface::SHOW_VISIBLE, $sut->getVisibility());
$sut->setVisibility('2');
$this->assertEquals(VisibilityInterface::SHOW_HIDDEN, $sut->getVisibility());
self::assertFalse($sut->isShowVisible());
self::assertTrue($sut->isShowHidden());
self::assertFalse($sut->isShowBoth());
$sut->setVisibility('0'); // keep the value that was previously set
$this->assertEquals(VisibilityInterface::SHOW_HIDDEN, $sut->getVisibility());
$sut->setVisibility(VisibilityInterface::SHOW_BOTH);
$this->assertEquals(VisibilityInterface::SHOW_BOTH, $sut->getVisibility());
self::assertFalse($sut->isShowVisible());
self::assertFalse($sut->isShowHidden());
self::assertTrue($sut->isShowBoth());
$sut->setVisibility(VisibilityInterface::SHOW_HIDDEN);
$this->assertEquals(VisibilityInterface::SHOW_HIDDEN, $sut->getVisibility());