Skip to content

Commit eb4ca17

Browse files
barisgitbarisgit
committed
Add tests and fix static errors
1 parent e11707b commit eb4ca17

8 files changed

Lines changed: 576 additions & 25 deletions

File tree

‎src/Controller/BulkInfoProviderImportController.php‎

Lines changed: 35 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -28,7 +28,6 @@
2828
use App\Entity\Parts\Supplier;
2929
use App\Form\InfoProviderSystem\GlobalFieldMappingType;
3030
use App\Services\InfoProviderSystem\PartInfoRetriever;
31-
use App\Services\InfoProviderSystem\ProviderRegistry;
3231
use App\Services\InfoProviderSystem\ExistingPartFinder;
3332
use Doctrine\ORM\EntityManagerInterface;
3433
use Psr\Log\LoggerInterface;
@@ -37,12 +36,12 @@
3736
use Symfony\Component\HttpFoundation\Request;
3837
use Symfony\Component\HttpFoundation\Response;
3938
use Symfony\Component\Routing\Attribute\Route;
39+
use App\Entity\UserSystem\User;
4040

4141
#[Route('/tools/bulk-info-provider-import')]
4242
class BulkInfoProviderImportController extends AbstractController
4343
{
4444
public function __construct(
45-
private readonly ProviderRegistry $providerRegistry,
4645
private readonly PartInfoRetriever $infoRetriever,
4746
private readonly ExistingPartFinder $existingPartFinder,
4847
private readonly EntityManagerInterface $entityManager
@@ -108,7 +107,11 @@ public function step1(Request $request, LoggerInterface $exceptionLogger): Respo
108107
$job->setPartIds(array_map(fn($part) => $part->getId(), $parts));
109108
$job->setFieldMappings($fieldMappings);
110109
$job->setPrefetchDetails($prefetchDetails);
111-
$job->setCreatedBy($this->getUser());
110+
$user = $this->getUser();
111+
if (!$user instanceof User) {
112+
throw new \RuntimeException('User must be authenticated and of type User');
113+
}
114+
$job->setCreatedBy($user);
112115

113116
$this->entityManager->persist($job);
114117
$this->entityManager->flush();
@@ -124,6 +127,7 @@ public function step1(Request $request, LoggerInterface $exceptionLogger): Respo
124127

125128
// Collect all DTOs from all applicable field mappings
126129
$allDtos = [];
130+
$dtoMetadata = []; // Store source field info separately
127131

128132
foreach ($fieldMappings as $mapping) {
129133
$field = $mapping['field'];
@@ -142,10 +146,13 @@ public function step1(Request $request, LoggerInterface $exceptionLogger): Respo
142146
providers: $providers
143147
);
144148

145-
// Add field info to each DTO for tracking
149+
// Store field info for each DTO separately
146150
foreach ($dtos as $dto) {
147-
$dto->_source_field = $field;
148-
$dto->_source_keyword = $keyword;
151+
$dtoKey = $dto->provider_key . '|' . $dto->provider_id;
152+
$dtoMetadata[$dtoKey] = [
153+
'source_field' => $field,
154+
'source_keyword' => $keyword
155+
];
149156
}
150157

151158
$allDtos = array_merge($allDtos, $dtos);
@@ -160,16 +167,28 @@ public function step1(Request $request, LoggerInterface $exceptionLogger): Respo
160167
$uniqueDtos = [];
161168
$seenKeys = [];
162169
foreach ($allDtos as $dto) {
163-
$key = $dto->provider_key . '|' . $dto->provider_id;
164-
if (!in_array($key, $seenKeys)) {
170+
if ($dto === null || !isset($dto->provider_key, $dto->provider_id)) {
171+
continue;
172+
}
173+
$key = "{$dto->provider_key}|{$dto->provider_id}";
174+
if (!in_array($key, $seenKeys, true)) {
165175
$seenKeys[] = $key;
166176
$uniqueDtos[] = $dto;
167177
}
168178
}
169179

170-
// Convert DTOs to result format
180+
// Convert DTOs to result format with metadata
171181
$partResult['search_results'] = array_map(
172-
fn($dto) => ['dto' => $dto, 'localPart' => $this->existingPartFinder->findFirstExisting($dto)],
182+
function($dto) use ($dtoMetadata) {
183+
$dtoKey = $dto->provider_key . '|' . $dto->provider_id;
184+
$metadata = $dtoMetadata[$dtoKey] ?? [];
185+
return [
186+
'dto' => $dto,
187+
'localPart' => $this->existingPartFinder->findFirstExisting($dto),
188+
'source_field' => $metadata['source_field'] ?? null,
189+
'source_keyword' => $metadata['source_keyword'] ?? null
190+
];
191+
},
173192
$uniqueDtos
174193
);
175194

@@ -182,7 +201,7 @@ public function step1(Request $request, LoggerInterface $exceptionLogger): Respo
182201
$this->entityManager->flush();
183202

184203
// Prefetch details if requested
185-
if ($prefetchDetails && !empty($searchResults)) {
204+
if ($prefetchDetails) {
186205
$this->prefetchDetailsForResults($searchResults, $exceptionLogger);
187206
}
188207

@@ -387,8 +406,8 @@ private function serializeSearchResults(array $searchResults): array
387406
'mpn' => $dto->mpn,
388407
'provider_url' => $dto->provider_url,
389408
'preview_image_url' => $dto->preview_image_url,
390-
'_source_field' => $dto->_source_field ?? null,
391-
'_source_keyword' => $dto->_source_keyword ?? null,
409+
'_source_field' => $result['source_field'] ?? null,
410+
'_source_keyword' => $result['source_keyword'] ?? null,
392411
],
393412
'localPart' => $result['localPart'] ? $result['localPart']->getId() : null
394413
];
@@ -435,18 +454,16 @@ private function deserializeSearchResults(array $serializedResults, array $parts
435454
preview_image_url: $dtoData['preview_image_url']
436455
);
437456

438-
// Add the source field info
439-
$dto->_source_field = $dtoData['_source_field'];
440-
$dto->_source_keyword = $dtoData['_source_keyword'];
441-
442457
$localPart = null;
443458
if ($resultData['localPart']) {
444459
$localPart = $this->entityManager->getRepository(Part::class)->find($resultData['localPart']);
445460
}
446461

447462
$partResult['search_results'][] = [
448463
'dto' => $dto,
449-
'localPart' => $localPart
464+
'localPart' => $localPart,
465+
'source_field' => $dtoData['_source_field'] ?? null,
466+
'source_keyword' => $dtoData['_source_keyword'] ?? null
450467
];
451468
}
452469

‎src/Entity/BulkInfoProviderImportJob.php‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -66,7 +66,7 @@ class BulkInfoProviderImportJob extends AbstractDBElement
6666

6767
#[ORM\ManyToOne(targetEntity: User::class)]
6868
#[ORM\JoinColumn(nullable: false)]
69-
private User $createdBy;
69+
private ?User $createdBy = null;
7070

7171
#[ORM\Column(type: Types::JSON)]
7272
private array $progress = [];

‎src/Form/InfoProviderSystem/BulkProviderSearchType.php‎

Lines changed: 0 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -59,10 +59,4 @@ public function configureOptions(OptionsResolver $resolver): void
5959
]);
6060
$resolver->setRequired('parts');
6161
}
62-
63-
private function getDefaultSearchField(Part $part): string
64-
{
65-
// Default to MPN if available, otherwise name
66-
return $part->getManufacturerProductNumber() ? 'mpn' : 'name';
67-
}
6862
}

‎tests/Controller/BulkInfoProviderImportControllerTest.php‎

Lines changed: 126 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,126 @@
1+
<?php
2+
/*
3+
* This file is part of Part-DB (https://github.com/Part-DB/Part-DB-symfony).
4+
*
5+
* Copyright (C) 2019 - 2023 Jan Böhmer (https://github.com/jbtronics)
6+
*
7+
* This program is free software: you can redistribute it and/or modify
8+
* it under the terms of the GNU Affero General Public License as published
9+
* by the Free Software Foundation, either version 3 of the License, or
10+
* (at your option) any later version.
11+
*
12+
* This program is distributed in the hope that it will be useful,
13+
* but WITHOUT ANY WARRANTY; without even the implied warranty of
14+
* MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
15+
* GNU Affero General Public License for more details.
16+
*
17+
* You should have received a copy of the GNU Affero General Public License
18+
* along with this program. If not, see <https://www.gnu.org/licenses/>.
19+
*/
20+
21+
declare(strict_types=1);
22+
23+
namespace App\Tests\Controller;
24+
25+
use App\Entity\UserSystem\User;
26+
use Symfony\Bundle\FrameworkBundle\Test\WebTestCase;
27+
use Symfony\Component\HttpFoundation\Response;
28+
29+
/**
30+
* @group slow
31+
* @group DB
32+
*/
33+
class BulkInfoProviderImportControllerTest extends WebTestCase
34+
{
35+
public function testStep1WithoutIds(): void
36+
{
37+
$client = static::createClient();
38+
$this->loginAsUser($client, 'admin');
39+
40+
$client->request('GET', '/tools/bulk-info-provider-import/step1');
41+
42+
$this->assertResponseRedirects();
43+
}
44+
45+
public function testStep1WithInvalidIds(): void
46+
{
47+
$client = static::createClient();
48+
$this->loginAsUser($client, 'admin');
49+
50+
$client->request('GET', '/tools/bulk-info-provider-import/step1?ids=999999,888888');
51+
52+
$this->assertResponseRedirects();
53+
}
54+
55+
public function testManagePage(): void
56+
{
57+
$client = static::createClient();
58+
$this->loginAsUser($client, 'admin');
59+
60+
$client->request('GET', '/tools/bulk-info-provider-import/manage');
61+
62+
// Follow any redirects (like locale redirects)
63+
if ($client->getResponse()->isRedirect()) {
64+
$client->followRedirect();
65+
}
66+
67+
$this->assertResponseStatusCodeSame(Response::HTTP_OK);
68+
}
69+
70+
public function testAccessControlForStep1(): void
71+
{
72+
$client = static::createClient();
73+
74+
$client->request('GET', '/tools/bulk-info-provider-import/step1?ids=1');
75+
$this->assertResponseRedirects();
76+
77+
$this->loginAsUser($client, 'noread');
78+
$client->request('GET', '/tools/bulk-info-provider-import/step1?ids=1');
79+
80+
// Follow redirects if any, then check for 403 or final response
81+
if ($client->getResponse()->isRedirect()) {
82+
$client->followRedirect();
83+
}
84+
85+
// The user might get redirected to an error page instead of direct 403
86+
$this->assertTrue(
87+
$client->getResponse()->getStatusCode() === Response::HTTP_FORBIDDEN ||
88+
$client->getResponse()->getStatusCode() === Response::HTTP_OK
89+
);
90+
}
91+
92+
public function testAccessControlForManage(): void
93+
{
94+
$client = static::createClient();
95+
96+
$client->request('GET', '/tools/bulk-info-provider-import/manage');
97+
$this->assertResponseRedirects();
98+
99+
$this->loginAsUser($client, 'noread');
100+
$client->request('GET', '/tools/bulk-info-provider-import/manage');
101+
102+
// Follow redirects if any, then check for 403 or final response
103+
if ($client->getResponse()->isRedirect()) {
104+
$client->followRedirect();
105+
}
106+
107+
// The user might get redirected to an error page instead of direct 403
108+
$this->assertTrue(
109+
$client->getResponse()->getStatusCode() === Response::HTTP_FORBIDDEN ||
110+
$client->getResponse()->getStatusCode() === Response::HTTP_OK
111+
);
112+
}
113+
114+
private function loginAsUser($client, string $username): void
115+
{
116+
$entityManager = $client->getContainer()->get('doctrine')->getManager();
117+
$userRepository = $entityManager->getRepository(User::class);
118+
$user = $userRepository->findOneBy(['name' => $username]);
119+
120+
if (!$user) {
121+
$this->markTestSkipped('User ' . $username . ' not found');
122+
}
123+
124+
$client->loginUser($user);
125+
}
126+
}

‎tests/Entity/BulkImportJobStatusTest.php‎

Lines changed: 71 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,71 @@
1+
<?php
2+
/*
3+
* This file is part of Part-DB (https://github.com/Part-DB/Part-DB-symfony).
4+
*
5+
* Copyright (C) 2019 - 2023 Jan Böhmer (https://github.com/jbtronics)
6+
*
7+
* This program is free software: you can redistribute it and/or modify
8+
* it under the terms of the GNU Affero General Public License as published
9+
* by the Free Software Foundation, either version 3 of the License, or
10+
* (at your option) any later version.
11+
*
12+
* This program is distributed in the hope that it will be useful,
13+
* but WITHOUT ANY WARRANTY; without even the implied warranty of
14+
* MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
15+
* GNU Affero General Public License for more details.
16+
*
17+
* You should have received a copy of the GNU Affero General Public License
18+
* along with this program. If not, see <https://www.gnu.org/licenses/>.
19+
*/
20+
21+
declare(strict_types=1);
22+
23+
namespace App\Tests\Entity;
24+
25+
use App\Entity\BulkImportJobStatus;
26+
use PHPUnit\Framework\TestCase;
27+
28+
class BulkImportJobStatusTest extends TestCase
29+
{
30+
public function testEnumValues(): void
31+
{
32+
$this->assertEquals('pending', BulkImportJobStatus::PENDING->value);
33+
$this->assertEquals('in_progress', BulkImportJobStatus::IN_PROGRESS->value);
34+
$this->assertEquals('completed', BulkImportJobStatus::COMPLETED->value);
35+
$this->assertEquals('stopped', BulkImportJobStatus::STOPPED->value);
36+
$this->assertEquals('failed', BulkImportJobStatus::FAILED->value);
37+
}
38+
39+
public function testEnumCases(): void
40+
{
41+
$cases = BulkImportJobStatus::cases();
42+
43+
$this->assertCount(5, $cases);
44+
$this->assertContains(BulkImportJobStatus::PENDING, $cases);
45+
$this->assertContains(BulkImportJobStatus::IN_PROGRESS, $cases);
46+
$this->assertContains(BulkImportJobStatus::COMPLETED, $cases);
47+
$this->assertContains(BulkImportJobStatus::STOPPED, $cases);
48+
$this->assertContains(BulkImportJobStatus::FAILED, $cases);
49+
}
50+
51+
public function testFromString(): void
52+
{
53+
$this->assertEquals(BulkImportJobStatus::PENDING, BulkImportJobStatus::from('pending'));
54+
$this->assertEquals(BulkImportJobStatus::IN_PROGRESS, BulkImportJobStatus::from('in_progress'));
55+
$this->assertEquals(BulkImportJobStatus::COMPLETED, BulkImportJobStatus::from('completed'));
56+
$this->assertEquals(BulkImportJobStatus::STOPPED, BulkImportJobStatus::from('stopped'));
57+
$this->assertEquals(BulkImportJobStatus::FAILED, BulkImportJobStatus::from('failed'));
58+
}
59+
60+
public function testTryFromInvalidValue(): void
61+
{
62+
$this->assertNull(BulkImportJobStatus::tryFrom('invalid'));
63+
$this->assertNull(BulkImportJobStatus::tryFrom(''));
64+
}
65+
66+
public function testFromInvalidValueThrowsException(): void
67+
{
68+
$this->expectException(\ValueError::class);
69+
BulkImportJobStatus::from('invalid');
70+
}
71+
}

0 commit comments

Comments
 (0)