Skip to content
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

[QA - Sentry] setKey on Array / TypeError #2218

Merged
merged 4 commits into from
Feb 9, 2024
Merged

Conversation

numew
Copy link
Collaborator

@numew numew commented Feb 6, 2024

Ticket

#2211 #2212

Description

  • Correction de l'erreur "Error: Call to a member function setKey() on array" si l'upload d'une image ne se passe pas correctement
  • Correction de l'erreur "Argument Bump guzzlehttp/guzzle from 7.4.3 to 7.4.5 #1 ($filename) must be of type string, array given" lors de la soumission du formulaire

Tests

Je n'ai pas su reproduire les erreurs via une utilisation normale, voici comment on peux tester :

  • Tester que l'upload d'image ne provoque pas l'erreur "setKey" aprés avoir ajouté throw new FileException('DEBUG'); sous la ligne 59 de src/Service/UploadHandlerService.php
  • Tester que la soumission de l'ancien formulaire (contenant des document) ne provoque pas d'erreur en transformant le champ en array (ex signalement[files][photos][OIG3.jpg] => signalement[files][photos][OIG3.jpg][]

Copy link
Collaborator

@sfinx13 sfinx13 left a comment

Choose a reason for hiding this comment

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

Lecture et Test OK

Tester que l'upload d'image ne provoque pas l'erreur "setKey" aprés avoir ajouté throw new FileException('DEBUG'); sous la ligne 59 de src/Service/UploadHandlerService.php

Du coup, est ce que tu peux écrire un test qui simule l'exception FileException lors de l'upload et de vérifier que c'est bien un tableau ?

Tu peux copier ce test UploadHandlerServiceTest::testTemporaryFileUploaded() et ajouter que writeStream lancera une exception

        $this->filesystemOperator
            ->expects($this->once())
            ->method('writeStream')
            ->willThrowException(new FileException());

ça permettra de vérifier qu'on a bien un tableau avec une clé error

        $uploadHandler = $uploadHandlerService->toTempFolder($uploadFile);
        $this->assertIsArray($uploadHandler);
        $this->assertArrayHasKey('error', $uploadHandler);

Copy link

sonarcloud bot commented Feb 8, 2024

Quality Gate Passed Quality Gate passed

Issues
1 New issue

Measures
0 Security Hotspots
No data about Coverage
0.0% Duplication on New Code

See analysis details on SonarCloud

@numew
Copy link
Collaborator Author

numew commented Feb 8, 2024

Lecture et Test OK

Tester que l'upload d'image ne provoque pas l'erreur "setKey" aprés avoir ajouté throw new FileException('DEBUG'); sous la ligne 59 de src/Service/UploadHandlerService.php

Du coup, est ce que tu peux écrire un test qui simule l'exception FileException lors de l'upload et de vérifier que c'est bien un tableau ?

Tu peux copier ce test UploadHandlerServiceTest::testTemporaryFileUploaded() et ajouter que writeStream lancera une exception

        $this->filesystemOperator
            ->expects($this->once())
            ->method('writeStream')
            ->willThrowException(new FileException());

ça permettra de vérifier qu'on a bien un tableau avec une clé error

        $uploadHandler = $uploadHandlerService->toTempFolder($uploadFile);
        $this->assertIsArray($uploadHandler);
        $this->assertArrayHasKey('error', $uploadHandler);

C'est fait, merci pour l'aide

Copy link
Collaborator

@sfinx13 sfinx13 left a comment

Choose a reason for hiding this comment

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

Code + Test OK

Copy link
Collaborator

@emilschn emilschn left a comment

Choose a reason for hiding this comment

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

OK relecture et test !

@emilschn emilschn merged commit 0819969 into develop Feb 9, 2024
3 checks passed
@sfinx13 sfinx13 deleted the bugfix/2211-2212-sentry branch February 13, 2024 21:09
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants