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

Sauvegarde des factures #940

Closed
wants to merge 2 commits into from
Closed

Sauvegarde des factures #940

wants to merge 2 commits into from

Conversation

kuraobi
Copy link
Contributor

@kuraobi kuraobi commented Apr 14, 2020

Closes #736 , #915

return $this->backOfficeLegacyBridge->afficherMessage('L\'écriture a été ' . ($action === self::ACTION_ADD ? 'ajoutée' : 'modifiée'), $this->urlGenerator->generate('admin_accounting_invoices'));
}

if ($this->session instanceof Session) {

Choose a reason for hiding this comment

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

Pourquoi ?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Parce que SessionInterface n'a pas de flashbag, il n'y a que l'implémentation Session qui en a un 🤷‍♂️

@@ -18,6 +18,7 @@ public function registerBundles()
new AppBundle\AppBundle(),
new \JMS\SerializerBundle\JMSSerializerBundle(),
new Presta\SitemapBundle\PrestaSitemapBundle(),
new \Oneup\FlysystemBundle\OneupFlysystemBundle()
Copy link
Member

Choose a reason for hiding this comment

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

@kuraobi est-ce qu'il serait possible de supprimer cette dépendance et écrire le fichier à la main ?

Choose a reason for hiding this comment

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

@kuraobi est-ce qu'il serait possible de supprimer cette dépendance et écrire le fichier à la main ?

Pourquoi ?

@agallou
Copy link
Member

agallou commented Dec 7, 2022

Cette PR utilise une dépendance sur un bundle qu'on ne souhaite pas forcément ajouter (il faudrait plutôt utiliser directement utiliser flyssystem ou une lib pour s3 (comme async-aws/s3).
Suite à une réunion mensuelle du pôle outil on est plus partir sur le fait de faire ce changement en deux temps : séparer la migration symfony (dans le ticket #1238) de l’historisation/sauvegarde des factures (le #736) (où on aurait aussi une commande génération des PDF des factures actuelles.)
On ferme donc la PR, en conservant ce besoin en backlog et pouvant potentiellement repartir de celle-ci pour les prochains changement.

@agallou agallou closed this Dec 7, 2022
@Shine-neko
Copy link

Shine-neko commented Dec 8, 2022

@agallou c'est quoi le soucis d'utiliser le bundle avec ?

@agallou
Copy link
Member

agallou commented Dec 8, 2022

@Shine-neko c'est pour limiter le nombre de dépendances ajoutées, ça ajouterais 3 dépendances à des projets alors qu'on peux en ajouter une seule.

@Shine-neko
Copy link

@agallou

Est-ce vraiment un soucis ? Qu'est ce que cela changerait ?

Je trouve ça dommage de se privé d'une contribution juste pour ça. ( Déjà qu'il y en a pas beaucoup ...)

@agallou
Copy link
Member

agallou commented Dec 18, 2022

@Shine-neko il n'y a pas seulement cela, il y a aussi le fait de séparer en deux le changement : d'abord faire l'historisation et ensuite migrer sous symfony, et cela avec des tests fonctionnels, afin de limiter le risque su recette partie assez sensible.
L'idée n'est pas de se priver d'une contribution : les tickets créés pourront s'inspirer et/ou reprendre cette PR.
Au niveau des contributions, en année 2022 le pôle s'est structuré, il y a maintenant des points mensuels, plus de personnes qui contribuent et sur le projet web ce sont 60 PRs qui ont été mergés.

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.

Changements non historisés
4 participants