Remove reference to UserSession to get the current user identifier an…

…d use PersonProvider::getCurrentPerson directly instead
digital-blueprint · Sep 11, 2024 · feb75da · feb75da
1 parent 3f959bf
commit feb75da
Show file tree

Hide file tree

Showing 5 changed files with 51 additions and 34 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -1,5 +1,10 @@
 # Changelog
 
+## v0.2.5
+
+* Remove reference to UserSession to get the current user identifier and use
+PersonProvider::getCurrentPerson directly instead
+
 ## v0.2.4
 
 * Fix compatibility with the latest CAMPUSonline release, resulting in authentication

diff --git a/composer.json b/composer.json
@@ -7,7 +7,7 @@
         "php": ">=8.1",
         "ext-json": "*",
         "api-platform/core": "^2.7.11 || ^3.2",
-        "dbp/relay-base-person-bundle": "^0.2.27",
+        "dbp/relay-base-person-bundle": "^0.2.33",
         "dbp/relay-core-bundle": "^0.1.148",
         "dbp/relay-mono-bundle": "^0.4.10",
         "guzzlehttp/guzzle": "^7.4",

diff --git a/composer.lock b/composer.lock
diff --git a/src/Service/TuitionFeeService.php b/src/Service/TuitionFeeService.php
@@ -5,7 +5,6 @@
 namespace Dbp\Relay\MonoConnectorCampusonlineBundle\Service;
 
 use Dbp\Relay\BasePersonBundle\API\PersonProviderInterface;
-use Dbp\Relay\CoreBundle\API\UserSessionInterface;
 use Dbp\Relay\CoreBundle\Exception\ApiError;
 use Dbp\Relay\CoreBundle\Rest\Options;
 use Dbp\Relay\MonoBundle\ApiPlatform\Payment;
@@ -28,7 +27,6 @@ class TuitionFeeService implements BackendServiceInterface, LoggerAwareInterface
 
     public const PERSON_TITLE_LOCAL_DATA_ATTRIBUTE = 'title';
 
-    private UserSessionInterface $userSession;
     private TranslatorInterface $translator;
     private LoggerInterface $auditLogger;
     private PersonProviderInterface $personProvider;
@@ -42,12 +40,10 @@ class TuitionFeeService implements BackendServiceInterface, LoggerAwareInterface
 
     public function __construct(
         TranslatorInterface $translator,
-        UserSessionInterface $userSession,
         PersonProviderInterface $personProvider,
         ConfigurationService $config,
     ) {
         $this->translator = $translator;
-        $this->userSession = $userSession;
         $this->logger = new NullLogger();
         $this->auditLogger = new NullLogger();
         $this->personProvider = $personProvider;
@@ -113,19 +109,18 @@ public function updateData(PaymentPersistence $paymentPersistence): bool
             || $payment->getDataUpdatedAt() <= $updateExpiration
         ) {
             $this->auditLogger->debug('CO: Updating the payment data', $this->getLoggingContext($payment));
-            $userIdentifier = $this->userSession->getUserIdentifier();
-            if ($userIdentifier === null) {
-                throw new ApiError(Response::HTTP_UNAUTHORIZED, 'No user identifier!');
-            }
 
             $personProviderOptions = [];
-            $person = $this->personProvider->getPerson($userIdentifier,
+            $currentPerson = $this->personProvider->getCurrentPerson(
                 Options::requestLocalDataAttributes($personProviderOptions, [self::PERSON_TITLE_LOCAL_DATA_ATTRIBUTE]));
+            if ($currentPerson === null) {
+                throw new ApiError(Response::HTTP_FORBIDDEN, 'Forbidden');
+            }
 
-            $payment->setLocalIdentifier($person->getIdentifier());
-            $payment->setGivenName($person->getGivenName());
-            $payment->setFamilyName($person->getFamilyName());
-            $payment->setHonorificSuffix($person->getLocalDataValue(self::PERSON_TITLE_LOCAL_DATA_ATTRIBUTE));
+            $payment->setLocalIdentifier($currentPerson->getIdentifier());
+            $payment->setGivenName($currentPerson->getGivenName());
+            $payment->setFamilyName($currentPerson->getFamilyName());
+            $payment->setHonorificSuffix($currentPerson->getLocalDataValue(self::PERSON_TITLE_LOCAL_DATA_ATTRIBUTE));
 
             $api = $this->getApiByType($payment->getType(), $payment);
             $obfuscatedId = $payment->getLocalIdentifier();

diff --git a/tests/TuitionFeeServiceTest.php b/tests/TuitionFeeServiceTest.php
@@ -4,10 +4,8 @@
 
 namespace Dbp\Relay\MonoConnectorCampusonlineBundle\Tests;
 
-use Dbp\Relay\BasePersonBundle\Entity\Person;
 use Dbp\Relay\BasePersonBundle\Service\DummyPersonProvider;
 use Dbp\Relay\CoreBundle\Exception\ApiError;
-use Dbp\Relay\CoreBundle\TestUtils\TestUserSession;
 use Dbp\Relay\MonoBundle\ApiPlatform\Payment;
 use Dbp\Relay\MonoBundle\Persistence\PaymentPersistence;
 use Dbp\Relay\MonoBundle\Persistence\PaymentStatus;
@@ -25,14 +23,13 @@ class TuitionFeeServiceTest extends KernelTestCase
 {
     private TuitionFeeService $tuitionFeeService;
 
+    private ?DummyPersonProvider $personProvider = null;
+
     protected function setUp(): void
     {
-        $dummyPersonProvider = new DummyPersonProvider();
-        $person = new Person();
-        $person->setIdentifier('testuser');
-        $person->setGivenName('John');
-        $person->setFamilyName('Doe');
-        $dummyPersonProvider->setCurrentPerson($person);
+        $this->personProvider = new DummyPersonProvider();
+        $this->personProvider->addPerson('testuser', 'John', 'Doe', ['title' => 'Dr.']);
+        $this->personProvider->setCurrentPersonIdentifier('testuser');
         $config = new ConfigurationService();
         $config->setConfig([
             'payment_types' => [
@@ -44,8 +41,7 @@ protected function setUp(): void
             ],
         ]);
         $translator = self::getContainer()->get(TranslatorInterface::class);
-        $this->tuitionFeeService = new TuitionFeeService($translator,
-            new TestUserSession('testuser'), $dummyPersonProvider, $config);
+        $this->tuitionFeeService = new TuitionFeeService($translator, $this->personProvider, $config);
     }
 
     private function getAuthResponses(): array
@@ -141,7 +137,28 @@ public function testUpdateData(): void
         $this->assertSame($paymentPersistence->getCurrency(), 'EUR');
         $this->assertSame($paymentPersistence->getGivenName(), 'John');
         $this->assertSame($paymentPersistence->getFamilyName(), 'Doe');
-        $this->assertSame($paymentPersistence->getHonorificSuffix(), 'title');
+        $this->assertSame($paymentPersistence->getHonorificSuffix(), 'Dr.');
+    }
+
+    public function testUpdateDataNoCurrentPerson(): void
+    {
+        $this->mockResponses([
+            new Response(200, ['Content-Type' => 'application/json'], '{"amount":300,"semesterKey":"2022S"}'),
+        ]);
+
+        $paymentPersistence = new PaymentPersistence();
+        $paymentPersistence->setIdentifier('test_payment_persistence');
+        $paymentPersistence->setType('test_payment_type');
+        $paymentPersistence->setData('22S');
+
+        $this->personProvider->setCurrentPersonIdentifier(null);
+
+        try {
+            $this->tuitionFeeService->updateData($paymentPersistence);
+            $this->fail('Expected an ApiError');
+        } catch (ApiError $apiError) {
+            $this->assertSame($apiError->getStatusCode(), HttpResponse::HTTP_FORBIDDEN);
+        }
     }
 
     public function testUpdateDataAmountToSmall(): void