-
Notifications
You must be signed in to change notification settings - Fork 0
Theme reader #20 #21
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Theme reader #20 #21
Changes from all commits
478659d
df6473f
fddbf34
7521fa2
7c5f5b2
66e2450
cfd5ceb
1f68d2d
027c068
6b3b35d
27169a0
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,34 @@ | ||
| <?php | ||
|
|
||
| namespace App\Imports\Themes; | ||
|
|
||
| use PhpOffice\PhpSpreadsheet\Worksheet\Worksheet; | ||
|
|
||
| class ThemeReader | ||
| { | ||
| public function __construct( | ||
| private readonly Worksheet $sheet, | ||
| ) { | ||
| } | ||
|
|
||
| public function extract(): array | ||
| { | ||
| $themes = []; | ||
| foreach ($this->sheet->getRowIterator() as $row) { | ||
| $rowIndex = $row->getRowIndex(); | ||
| $name = $this->sheet->getCell('A'.$rowIndex)->getValue(); | ||
| $external_id = $this->sheet->getCell('B'.$rowIndex)->getValue(); | ||
|
|
||
| if (null !== $external_id && null !== $name) { | ||
| if ($rowIndex > 2) { | ||
| $themes[] = [ | ||
| 'externalId' => $external_id, | ||
| 'name' => $name, | ||
| ]; | ||
| } | ||
| } | ||
| } | ||
|
|
||
| return $themes; | ||
| } | ||
| } | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -4,13 +4,18 @@ | |
|
|
||
| use App\Entity\Theme; | ||
| use App\Imports\Themes\ExtractService; | ||
| use App\Imports\Themes\ThemeReader; | ||
| use PhpOffice\PhpSpreadsheet\IOFactory; | ||
| use PhpOffice\PhpSpreadsheet\Worksheet\Worksheet; | ||
| use Symfony\Bundle\FrameworkBundle\Test\KernelTestCase; | ||
|
|
||
| class ExtractServiceTest extends KernelTestCase | ||
| { | ||
| private $entityManager; | ||
| private $themeRepository; | ||
| private $projectDir; | ||
| private ?Worksheet $sheet; | ||
| private $extract_service; | ||
|
|
||
| protected function setUp(): void | ||
| { | ||
|
|
@@ -20,6 +25,13 @@ protected function setUp(): void | |
| $this->entityManager = $container->get('doctrine.orm.entity_manager'); | ||
| $this->themeRepository = $this->entityManager->getRepository(Theme::class); | ||
| $this->projectDir = $container->getParameter('kernel.project_dir'); | ||
| $excel_file = $this->projectDir.'/tests/Imports/Themes/test-themes.xlsx'; | ||
| if (!file_exists($excel_file)) { | ||
| throw new \Exception('file does not exist'); | ||
| } | ||
| $spreadsheet = IOFactory::load($excel_file); | ||
| $this->sheet = $spreadsheet->getActiveSheet(); | ||
| $this->extract_service = new ExtractService($this->entityManager, $this->sheet); | ||
| } | ||
|
|
||
| protected function tearDown(): void | ||
|
|
@@ -34,12 +46,10 @@ protected function tearDown(): void | |
|
|
||
| public function testImportThemeSave(): void | ||
| { | ||
| $ExtractServices = new ExtractService($this->entityManager); | ||
| // TODO: change this to inject the spreadsheet so that we can test the integration more easily. | ||
| $excel_file = $this->projectDir.'/tests/Imports/Themes/test-themes.xlsx'; | ||
| $themes = $ExtractServices->GetThemesFromExcelFile($excel_file); | ||
| $preparedThemes = $ExtractServices->PrepareThemesForDatabase($themes); | ||
| $savedThemesCount = $ExtractServices->SaveThemesOnDatabase($preparedThemes); | ||
| $read_themes = new ThemeReader($this->sheet); | ||
|
Owner
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Tu ne dois pas melanger les tests. Normalment tu testerait theme reader seul, extract service seul et tu auras un test d'integration pour les deux. Ce que j'avais en tete etait plutot: https://github.com/unflores/di_example/blob/main/src/test/ThemeReaderTest.php#L22-L32 Typiquement ce dont je parle s'appelle un test unitaire, parce qu'il y a un seul unit(ou module) de code testé. Brèf, tu pourrais le changer plutard si tu as envie mais pour l'instant on peut avancer je pense... |
||
| $themes = $read_themes->extract(); | ||
| $preparedThemes = $this->extract_service->PrepareThemesForDatabase($themes); | ||
| $savedThemesCount = $this->extract_service->SaveThemesOnDatabase($preparedThemes); | ||
|
|
||
| $this->assertEquals( | ||
| $savedThemesCount, | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Maintenant tu peux tester extract meme sans ton fichier originel: https://github.com/unflores/di_example/blob/main/src/test/ThemeReaderTest.php#L22-L27