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

[BO - Export] Remplacer la pièce jointe de l'export par un lien téléchargeable en corps de mail #3114

Merged
merged 3 commits into from
Oct 3, 2024

Conversation

sfinx13
Copy link
Collaborator

@sfinx13 sfinx13 commented Oct 1, 2024

Ticket

#3111

Description

Contrainte de poids des fichiers pour les pièces jointes : Remplacer la pièce jointe par un lien de téléchargement.
image

Changements apportés

  • Ajout d'une migration afin de rendre la colonne signalement optionnel
  • Ajout d'un nouveau type de document
  • Mise à jour handler
  • Mise à jour template

Pré-requis

make worker-stop
make worker-start

Tests

  • Lancer un export csv et vérifier que l'export est téléchargeable depuis un lien
  • Lancer un export xls et vérifier que l'export est téléchargeable depuis un lien

Copy link

sonarcloud bot commented Oct 1, 2024

Quality Gate Failed Quality Gate failed

Failed conditions
23.7% Duplication on New Code (required ≤ 3%)

See analysis details on SonarCloud


public function up(Schema $schema): void
{
$this->addSql('ALTER TABLE file CHANGE signalement_id signalement_id INT DEFAULT NULL');
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

J'ai du enregistrer l'export dans la table file pour le rendre téléchargeable.
Faudra envisager un nettoyage par la suite
Aussi se reposer la question d'une table file agnostique de toutes données métiers

Copy link
Collaborator

Choose a reason for hiding this comment

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

t'as créé un ticket de nettoyage pour plus tard ?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Done #3127

Comment on lines +46 to +51
if (DocumentType::EXPORT === $documentType) {
return (new BinaryFileResponse($file))->setContentDisposition(
ResponseHeaderBag::DISPOSITION_INLINE,
$file->getFilename()
);
}
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Pour éviter la régression sur le nom de fichier d'export

@@ -24,6 +24,7 @@ enum DocumentType: String
case AUTRE_PROCEDURE = 'AUTRE_PROCEDURE';
case PHOTO_SITUATION = 'PHOTO_SITUATION';
case PHOTO_VISITE = 'PHOTO_VISITE';
case EXPORT = 'EXPORT';
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

La purge pourra s'appuyer sur ce champ

Comment on lines +54 to +55
$uuid = Uuid::v4();
$filename = 'export-histologe-'.$listExportMessage->getUserId().'-'.$datetimeStr.'-'.$uuid.'.'.$format;
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Rendre le nom de fichier moins prévisible

Copy link
Collaborator

@hmeneuvrier hmeneuvrier left a comment

Choose a reason for hiding this comment

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

Relecture et tests 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 lecture et tests

@emilschn emilschn merged commit 8cb7d67 into main Oct 3, 2024
2 of 3 checks passed
sfinx13 added a commit that referenced this pull request Oct 3, 2024
@sfinx13 sfinx13 deleted the hotfix/3111-remove-attachement-mail-export branch October 9, 2024 16:15
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.

3 participants