Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 3 additions & 2 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -3,14 +3,15 @@
All notable changes to this project will be documented in this file.

## [Unreleased]

- Update docker build to publish to "os2display" org on docker hub. Update github workflow to latest actions.
- Set up separate image builds for itkdev and os2display
- Updated `EventDatabaseApiFeedType` query ensuring started
but not finished events are found.
- Refactored all feed related classes and services
- Minor update of composer packages
- Updated psalm to version 5.x
- Fixed feed data provider id issue [#151](https://github.com/os2display/display-api-service/pull/151)
- Set up separate image builds for itkdev and os2display
- Updated add user command to ask which tenants user belongs to

## [1.2.8] - 2023-05-25

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@
* file that was distributed with this source code.
*/

namespace App\Command\Users;
namespace App\Command\Tenant;

use App\Entity\Tenant;
use App\Repository\TenantRepository;
Expand Down
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
<?php

namespace App\Command\Users;
namespace App\Command\Tenant;

use App\Entity\Tenant;
use App\Repository\TenantRepository;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -9,8 +9,9 @@
* file that was distributed with this source code.
*/

namespace App\Command\Users;
namespace App\Command\User;

use App\Entity\Tenant;
use App\Entity\User;
use App\Entity\UserRoleTenant;
use App\Repository\TenantRepository;
Expand All @@ -22,8 +23,8 @@
use Symfony\Component\Console\Exception\RuntimeException;
use Symfony\Component\Console\Input\InputArgument;
use Symfony\Component\Console\Input\InputInterface;
use Symfony\Component\Console\Input\InputOption;
use Symfony\Component\Console\Output\OutputInterface;
use Symfony\Component\Console\Question\ChoiceQuestion;
use Symfony\Component\Console\Style\SymfonyStyle;
use Symfony\Component\PasswordHasher\Hasher\UserPasswordHasherInterface;
use Symfony\Component\Stopwatch\Stopwatch;
Expand Down Expand Up @@ -63,7 +64,7 @@ public function __construct(
private UserPasswordHasherInterface $passwordHasher,
private CommandInputValidator $validator,
private UserRepository $users,
private TenantRepository $tenants
private TenantRepository $tenantRepository
) {
parent::__construct();
}
Expand All @@ -80,7 +81,8 @@ protected function configure(): void
->addArgument('email', InputArgument::OPTIONAL, 'The email of the new user')
->addArgument('password', InputArgument::OPTIONAL, 'The plain password of the new user')
->addArgument('full-name', InputArgument::OPTIONAL, 'The full name of the new user')
->addOption('admin', null, InputOption::VALUE_NONE, 'If set, the user is created as an administrator')
->addArgument('role', InputArgument::OPTIONAL, 'The role of the user [editor|admin]')
->addArgument('tenant-keys', InputArgument::IS_ARRAY | InputArgument::OPTIONAL, 'The keys of the tenants the user should belong to (separate multiple keys with a space)')
Comment on lines +84 to +85
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would use options for this (rather than arguments) to make it easier to handle multiple values, e.g.

app:user:add … --role=admin --tenant=hat --tenant=briller

but that's a matter of opinion.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Generally my preference as well. This was originally adapted from https://github.com/symfony/demo/blob/main/src/Command/AddUserCommand.php

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

And that example uses an option, --admin, for (implicitly) setting the role. This is not really important, so let's leave it as it is for now.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Agreed 😄

Changed because

  1. there might be more than 2 roles at some point,
  2. none of the people who has had to use the command have figured out the --admin option. So tried to make it more obvious how to select role

;
}

Expand Down Expand Up @@ -109,7 +111,12 @@ protected function initialize(InputInterface $input, OutputInterface $output): v
*/
protected function interact(InputInterface $input, OutputInterface $output): void
{
if (null !== $input->getArgument('email') && null !== $input->getArgument('password') && null !== $input->getArgument('full-name')) {
if (null !== $input->getArgument('email') &&
null !== $input->getArgument('password') &&
null !== $input->getArgument('full-name') &&
null !== $input->getArgument('role') &&
null !== $input->getArgument('tenant-keys')
) {
return;
}

Expand Down Expand Up @@ -149,6 +156,39 @@ protected function interact(InputInterface $input, OutputInterface $output): voi
$fullName = $this->io->ask('Full Name', null, [$this->validator, 'validateFullName']);
$input->setArgument('full-name', $fullName);
}

$helper = $this->getHelper('question');

// Ask for the role if it's not defined
$role = $input->getArgument('role');
if (null !== $role) {
$this->io->text(' > <info>Role</info>: '.$role);
} else {
$question = new ChoiceQuestion(
'Please select the user\'s role (defaults to editor)',
CommandInputValidator::ALLOWED_USER_ROLES,
0
);
$question->setErrorMessage('Role %s is invalid.');

$role = $helper->ask($input, $output, $question);
$output->writeln('You have just selected: '.$role);
}

// Ask for the tenant keys if it's not defined
$tenantKeys = $input->getArgument('tenant-keys');
if (0 < \count($tenantKeys)) {
$this->io->text(' > <info>Tenant Keys</info>: '.$tenantKeys);
} else {
$question = new ChoiceQuestion(
'Please select the tenant(s) the user should belong to (to select multiple answer with a list. E.g: "key1, key3")',
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't understand what “(to select multiple answer with a list. E.g: "key1, key3")” means. Doesn't the helper help the user picking multiple values?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No. You have to answer with a list of comma separated values. There is no "UI".

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh, I thought the terminal presented a checkbox like user experience.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

https://symfony.com/doc/current/components/console/helpers/questionhelper.html#multiple-choices

use Symfony\Component\Console\Question\ChoiceQuestion;

// ...
public function execute(InputInterface $input, OutputInterface $output): int
{
    // ...
    $helper = $this->getHelper('question');
    $question = new ChoiceQuestion(
        'Please select your favorite colors (defaults to red and blue)',
        ['red', 'blue', 'yellow'],
        '0,1'
    );
    $question->setMultiselect(true);

    $colors = $helper->ask($input, $output, $question);
    $output->writeln('You have just selected: ' . implode(', ', $colors));

    return Command::SUCCESS;
}

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Notice implode(', ', $colors)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

But yes, before testing it I also expected a checkbox like feature

$this->getTenantsChoiceList(),
);
$question->setMultiselect(true);

$tenantKeys = $helper->ask($input, $output, $question);
$output->writeln('You have just selected: '.implode(', ', $tenantKeys));
}
}

/**
Expand All @@ -163,10 +203,11 @@ protected function execute(InputInterface $input, OutputInterface $output): int
$email = $input->getArgument('email');
$plainPassword = $input->getArgument('password');
$fullName = $input->getArgument('full-name');
$isAdmin = $input->getOption('admin');
$role = $input->getArgument('role');
$tenantKeys = $input->getArgument('tenant-keys');

// make sure to validate the user data is correct
$this->validateUserData($email, $plainPassword, $fullName);
$this->validateUserData($email, $plainPassword, $fullName, $role, $tenantKeys);

// create the user and hash its password
$user = new User();
Expand All @@ -179,19 +220,18 @@ protected function execute(InputInterface $input, OutputInterface $output): int
$hashedPassword = $this->passwordHasher->hashPassword($user, $plainPassword);
$user->setPassword($hashedPassword);

// @TODO Make it possible to only select specific Tenants
$tenants = $this->tenants->findAll();
foreach ($tenants as $tenant) {
foreach ($tenantKeys as $tenantKey) {
$tenant = $this->tenantRepository->findOneBy(['tenantKey' => $tenantKey]);
$userRoleTenant = new UserRoleTenant();
$userRoleTenant->setTenant($tenant);
$userRoleTenant->setRoles([$isAdmin ? 'ROLE_ADMIN' : 'ROLE_EDITOR']);
$userRoleTenant->setRoles(['ROLE_'.strtoupper($role)]);
$user->addUserRoleTenant($userRoleTenant);
}

$this->entityManager->persist($user);
$this->entityManager->flush();

$this->io->success(sprintf('%s was successfully created: %s', $isAdmin ? 'Administrator user' : 'User', $user->getUserIdentifier()));
$this->io->success(sprintf('%s was successfully created: %s', ucfirst($role), $user->getUserIdentifier()));

$event = $stopwatch->stop('add-user-command');
if ($output->isVerbose()) {
Expand All @@ -201,7 +241,7 @@ protected function execute(InputInterface $input, OutputInterface $output): int
return Command::SUCCESS;
}

private function validateUserData($email, $plainPassword, $fullName): void
private function validateUserData($email, $plainPassword, $fullName, $role, $tenantKeys): void
{
// first check if a user with the same username already exists.
$existingUser = $this->users->findOneBy(['email' => $email]);
Expand All @@ -214,6 +254,19 @@ private function validateUserData($email, $plainPassword, $fullName): void
$this->validator->validatePassword($plainPassword);
$this->validator->validateEmail($email);
$this->validator->validateFullName($fullName);
$this->validator->validateRole($role);
$this->validator->validateTenantKeys($tenantKeys);
}

private function getTenantsChoiceList(): array
{
$tenants = [];
/** @var Tenant $tenant */
foreach ($this->tenantRepository->findBy([], ['tenantKey' => 'ASC']) as $tenant) {
$tenants[$tenant->getTenantKey()] = $tenant->getDescription();
}

return $tenants;
}

/**
Expand All @@ -226,17 +279,12 @@ private function getCommandHelp(): string
return <<<'HELP'
The <info>%command.name%</info> command creates new users and saves them in the database:

<info>php %command.full_name%</info> <comment>email password</comment>

By default the command creates regular users. To create administrator users,
add the <comment>--admin</comment> option:

<info>php %command.full_name%</info> email password <comment>--admin</comment>
<info>php %command.full_name%</info> <comment>email password fullname role tenants</comment>

If you omit any of the two required arguments, the command will ask you to
If you omit any of the required arguments, the command will ask you to
provide the missing values:

# command will ask you for the password
# command will ask you for the password etc.
<info>php %command.full_name%</info> <comment>email</comment>

# command will ask you for all arguments
Expand Down
41 changes: 41 additions & 0 deletions src/Utils/CommandInputValidator.php
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@

namespace App\Utils;

use App\Repository\TenantRepository;
use Symfony\Component\Console\Exception\InvalidArgumentException;

use function Symfony\Component\String\u;
Expand All @@ -23,6 +24,12 @@
*/
class CommandInputValidator
{
public const ALLOWED_USER_ROLES = ['editor', 'admin'];

public function __construct(
private TenantRepository $tenantRepository,
) {}

public function validateUsername(?string $username): string
{
if (empty($username)) {
Expand Down Expand Up @@ -83,4 +90,38 @@ public function validateFullName(?string $fullName): string

return $fullName;
}

public function validateRole(?string $role): string
{
if (empty($role)) {
throw new InvalidArgumentException('The role can not be empty.');
}

if (!in_array($role, self::ALLOWED_USER_ROLES)) {
throw new InvalidArgumentException('Unknown role: '.$role);
}

return $role;
}

public function validateTenantKeys(?array $tenantKeys): array
{
if (empty($tenantKeys)) {
throw new InvalidArgumentException('The user must belong to at least one tenant.');
}

$unknownKeys = [];
foreach ($tenantKeys as $tenantKey) {
$tenant = $this->tenantRepository->findOneBy(['tenantKey' => $tenantKey]);
if (null === $tenant) {
$unknownKeys[] = $tenantKey;
}
}

if (0 !== \count($unknownKeys)) {
throw new InvalidArgumentException(sprintf('Unknown tenant keys: %s.', implode(', ', $unknownKeys)));
}

return $tenantKeys;
}
}