diff --git a/src/BusProNet/Model/CrmAttributesResponse.php b/src/BusProNet/Model/CrmAttributesResponse.php
index cbb3c5e..f659ccf 100644
--- a/src/BusProNet/Model/CrmAttributesResponse.php
+++ b/src/BusProNet/Model/CrmAttributesResponse.php
@@ -87,12 +87,13 @@ class CrmAttributesResponse
{
$crmSelections = [];
- foreach ($this->getAttributeGroups() as $group) {
+ // a response the CRM sent no attributes in still has to flatten to an empty set
+ foreach ($this->getAttributeGroups() ?? [] as $group) {
/** @var CrmAttributeGroup $group */
if (false === isset($crmSelections[$group->getLabel()])) {
$crmSelections[$group->getLabel()] = [];
}
- foreach ($group->getAttributes() as $attribute) {
+ foreach ($group->getAttributes() ?? [] as $attribute) {
/** @var CrmAttribute $attribute */
if (false === $attribute->isSelected()) {
continue;
diff --git a/src/BusProNet/UserDataHandler.php b/src/BusProNet/UserDataHandler.php
index 810e0e2..a635f64 100644
--- a/src/BusProNet/UserDataHandler.php
+++ b/src/BusProNet/UserDataHandler.php
@@ -165,10 +165,11 @@ class UserDataHandler
/**
* Updates an existing user from BusPro data.
*
- * Roles and hotel codes are imported once on user creation only and are managed
- * manually afterwards, so they are intentionally left untouched here. The only
- * exception are the privilege-free pending markers, which keep tracking the
- * administrative roles claimed in the CRM.
+ * Administrative roles and hotel codes are imported once on user creation only and are
+ * managed manually afterwards, so they are intentionally left untouched here. The
+ * exceptions are the privilege-free pending markers, which keep tracking the
+ * administrative roles claimed in the CRM, and ROLE_TEAMER, which needs no approval
+ * and is granted to whoever the CRM reports as a teamer.
*
* @param string[] $claimedRoles pending markers as returned by collectPendingRoles()
*/
@@ -188,6 +189,8 @@ class UserDataHandler
$this->refreshPendingRoles($user, $claimedRoles);
if (true === $isTeamer) {
+ $this->grantTeamerRole($user);
+
$address = Address::fromApiResponse($profileResponse);
$communication = Communication::fromApiResponse($profileResponse);
if (null === $user->getTeamer()) {
@@ -221,15 +224,19 @@ class UserDataHandler
/**
* Blocks a user the CRM no longer grants anything in this application.
*
- * The granted roles are deliberately kept: they stay visible for review and are what
- * makes the user reappear in the administrative list, where a super admin can unblock
- * them. Only the privilege-free markers are dropped, as they no longer reflect the CRM.
+ * The granted roles are deliberately kept: they stay visible for review. Only the
+ * privilege-free markers are dropped, as they no longer reflect the CRM. What keeps the
+ * user reachable is the block itself, not the roles: a teamer stays in the teamer list,
+ * everyone else is listed by getAdministrativeUsers() whatever roles are left.
* Regaining a CRM role does not unblock the account, that is a manual decision.
*/
public function disableForRevokedCrmRoles(User $user): void
{
- // an existing block may be a disciplinary one and must never be overwritten
+ // an existing block may be a disciplinary one and must never be overwritten, but
+ // findLocalUser() may have refreshed the BusPro ids and nothing else flushes here
if (true === $user->isDisabled()) {
+ $this->entityManager->flush();
+
return;
}
@@ -249,6 +256,33 @@ class UserDataHandler
]);
}
+ /**
+ * Grants ROLE_TEAMER to a user the CRM reports as a teamer.
+ *
+ * Unlike the administrative roles this one needs no approval, so it may be granted on
+ * any login rather than on creation only: it carries no privileges beyond the teamer
+ * area, and a teamer without it would be left with a teamer record they cannot reach,
+ * or locked out entirely for holding no assignable role at all.
+ *
+ * It is never withdrawn here. Losing the CRM attribute while holding no other role
+ * blocks the account anyway, and a role handed out manually must survive a login.
+ */
+ private function grantTeamerRole(User $user): void
+ {
+ $grantedRoles = $user->getAssignedRoles();
+
+ if (true === in_array('ROLE_TEAMER', $grantedRoles, true)) {
+ return;
+ }
+
+ $user->setRoles([...$grantedRoles, 'ROLE_TEAMER', ...$user->getPendingRoles()]);
+
+ $this->logger->info('Grant teamer role', [
+ 'user_id' => $user->getId(),
+ 'user_email' => $user->getEmail(),
+ ]);
+ }
+
/**
* Keeps the pending markers in sync with the administrative roles claimed in the CRM.
* The markers grant no privileges, so tracking them on every login is safe: only a
diff --git a/src/Controller/Admin/Teamer/DisableUserController.php b/src/Controller/Admin/Teamer/DisableUserController.php
index aa9d331..1f39983 100644
--- a/src/Controller/Admin/Teamer/DisableUserController.php
+++ b/src/Controller/Admin/Teamer/DisableUserController.php
@@ -29,7 +29,7 @@ class DisableUserController extends AbstractController
$form = $this->createForm(DisableUserType::class, $user, ['hx_post' => $request->getUri()]);
$form->handleRequest($request);
if ($form->isSubmitted() && $form->isValid()) {
- $user->setDisabledAt(new \DateTimeImmutable());
+ $user->setDisabled(true);
$this->entityManager->flush();
@@ -56,11 +56,7 @@ class DisableUserController extends AbstractController
$user = $teamer->getUser();
if (true === $request->isMethod(Request::METHOD_POST)) {
- $user
- ->setDisabledAt(null)
- ->setDisabledReason(null)
- ->setDisabledReasonInternal(null)
- ;
+ $user->setDisabled(false);
$this->entityManager->flush();
diff --git a/src/Form/UserType.php b/src/Form/UserType.php
index c94c39e..9fbfd2f 100644
--- a/src/Form/UserType.php
+++ b/src/Form/UserType.php
@@ -33,8 +33,6 @@ class UserType extends AbstractType
'required' => false,
'help' => 'Setzt die Rolle Admin voraus.',
])
- // must stay ahead of the reason: properties are written in field order and
- // unblocking clears the reasons
->add('disabled', CheckboxType::class, [
'label' => 'Account gesperrt',
'required' => false,
@@ -49,8 +47,33 @@ class UserType extends AbstractType
'data-action' => 'textarea-autosize#resize',
],
])
+ ->add('disabledReasonInternal', TextareaType::class, [
+ 'label' => 'Begründung intern',
+ 'required' => false,
+ 'help' => 'Wird der Benutzer:in nicht angezeigt.',
+ 'attr' => [
+ 'data-controller' => 'textarea-autosize',
+ 'data-action' => 'textarea-autosize#resize',
+ ],
+ ])
;
+ // the reasons only ever describe a block, so an unblocked account carries none.
+ // Done after mapping instead of relying on the field order, as the submitted text
+ // would otherwise be written back over the reset done by setDisabled()
+ $builder->addEventListener(FormEvents::POST_SUBMIT, static function (FormEvent $event): void {
+ $user = $event->getData();
+
+ if (false === $user instanceof User || true === $user->isDisabled()) {
+ return;
+ }
+
+ $user
+ ->setDisabledReason(null)
+ ->setDisabledReasonInternal(null)
+ ;
+ });
+
// hotel codes already assigned to the user may predate the catalog, so they are
// added as choices to keep them selectable instead of failing
$builder->addEventListener(FormEvents::PRE_SET_DATA, function (FormEvent $event): void {
diff --git a/src/Repository/UserRepository.php b/src/Repository/UserRepository.php
index ba09843..016f8ac 100644
--- a/src/Repository/UserRepository.php
+++ b/src/Repository/UserRepository.php
@@ -25,6 +25,11 @@ class UserRepository extends ServiceEntityRepository
}
/**
+ * Users holding an administrative role or awaiting approval for one, plus blocked
+ * accounts without a teamer. A user demoted by the CRM may keep no role that would list
+ * them, and a block is only ever lifted from a list: teamers are unblocked from theirs,
+ * everyone else has none but this one.
+ *
* @return User[]
*/
public function getAdministrativeUsers(): array
@@ -41,6 +46,11 @@ class UserRepository extends ServiceEntityRepository
;
}
+ $qb->orWhere($qb->expr()->andX(
+ $qb->expr()->isNotNull('u.disabledAt'),
+ $qb->expr()->isNull('u.teamer'),
+ ));
+
return $qb
->orderBy('u.lastName', 'ASC')
->getQuery()
diff --git a/src/Security/BpnAuthenticator.php b/src/Security/BpnAuthenticator.php
index c31b331..985e92f 100644
--- a/src/Security/BpnAuthenticator.php
+++ b/src/Security/BpnAuthenticator.php
@@ -112,12 +112,16 @@ class BpnAuthenticator extends AbstractLoginFormAuthenticator implements Authent
): ?User {
// Fetch CRM attributes, early return in case of an API error
try {
- /** @var CrmAttributesResponse $crmAttributes */
$crmAttributes = $this->apiClient->getCrmAttributes($email, $password);
} catch (ApiClientException $e) {
return null;
}
+ // BusPro answers with a notification record instead of the data on its own errors
+ if (false === $crmAttributes instanceof CrmAttributesResponse) {
+ return null;
+ }
+
// Flatten selected CRM attributes
$crmSelections = $crmAttributes->toArray();
@@ -140,6 +144,20 @@ class BpnAuthenticator extends AbstractLoginFormAuthenticator implements Authent
// are no user of it: never create an account, block an existing one. Returning
// the blocked user lets the UserChecker explain why the login was refused.
if ([] === $claimedRoles) {
+ // A response without any attribute group carries no roles either, so it looks
+ // exactly like a revocation while it really means the CRM told us nothing:
+ // an empty payload, a changed schema, a misconfigured attribute id. Blocking
+ // on that would lock out every user logging in, so refuse this single login
+ // instead and leave the account alone.
+ if ([] === ($crmAttributes->getAttributeGroups() ?? [])) {
+ $this->logger->warning('Skip demotion: CRM attributes response carries no attribute groups', [
+ 'user_id' => $user?->getId(),
+ 'user_email' => $email,
+ ]);
+
+ return null;
+ }
+
if (null === $user) {
return null;
}
diff --git a/src/Service/Cron/FeedbackReminderService.php b/src/Service/Cron/FeedbackReminderService.php
index 7f35095..1e39bfe 100644
--- a/src/Service/Cron/FeedbackReminderService.php
+++ b/src/Service/Cron/FeedbackReminderService.php
@@ -59,7 +59,7 @@ class FeedbackReminderService
$hotelManagers = $this
->userRepository
- ->getUsersByRoleAndHotelCode('ROLE_HOTEL_MANAGER', $hotelBaseCodes)
+ ->getUsersByRoleAndHotelCode('ROLE_HOUSE_MANAGER', $hotelBaseCodes)
;
if (0 === count($hotelManagers)) {
diff --git a/templates/admin/system/user/_form.html.twig b/templates/admin/system/user/_form.html.twig
index f5a0af5..8554824 100644
--- a/templates/admin/system/user/_form.html.twig
+++ b/templates/admin/system/user/_form.html.twig
@@ -5,6 +5,7 @@
{{ form_row(form.hotelCodes) }}
{{ form_row(form.disabled) }}
{{ form_row(form.disabledReason) }}
+ {{ form_row(form.disabledReasonInternal) }}