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.

Sign up for GitHub

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

Jump to bottom

(PC-33051)[API] feat: Generate invoices with correct wording for OA. #15108

Merged
merged 1 commit into from
Nov 22, 2024
Merged

(PC-33051)[API] feat: Generate invoices with correct wording for OA. #15108

merged 1 commit into from
Nov 22, 2024

Conversation

abouabdallaoui-pass
Copy link
Contributor

But de la pull request

Ticket Jira (ou description si BSR) : https://passculture.atlassian.net/browse/PC-33051

Vérifications

  • J'ai écrit les tests nécessaires
  • J'ai mis à jour le fichier des plans de tests du portail pro si nécessaire
  • J'ai mis à jour la liste des routes et des titres de pages du portail pro si j'en ai rajouté/modifié ou supprimé une.
  • J'ai relu attentivement les migrations, en particulier pour éviter les locks, et je préviens les équipes Shérif et Data
  • J'ai ajouté des screenshots pour d'éventuels changements graphiques

@abouabdallaoui-pass abouabdallaoui-pass marked this pull request as draft November 19, 2024 15:27
@abouabdallaoui-pass abouabdallaoui-pass marked this pull request as ready for review November 22, 2024 08:35
vroullier-pass
vroullier-pass approved these changes Nov 22, 2024
View reviewed changes
api/tests/core/finance/test_api.py Outdated
@@ -2522,7 +2525,7 @@ def _store_invoice_pdf(invoice_storage_id, invoice_html) -> None:
assert reimbursement_by_venue_row["Dont offres collectives (TTC)"] == "0,00 €"
assert reimbursement_by_venue_row["Dont offres individuelles (TTC)"] == "308,40 €"
assert reimbursement_by_venue_row["Incidents (TTC)"] == "10,10 €"
assert reimbursement_by_venue_row["Lieux"] == venue.name
assert reimbursement_by_venue_row["structures" if with_oa else "lieux"] == venue.name
Copy link
Contributor

Choose a reason for hiding this comment

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

ça devrait logiquement être Structures ou Lieux non ? (enfin avec une majuscule)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Comme j'avais la possibilité d'avoir structure et Structure, il aurait fallu que je fasse 2 variables différentes dans ff_wording, et je trouvais que c'était peut être inutile.
Mais comme je rajoute un capitalise un peu partout, c'est peut être plus simple.

Copy link
Contributor

Choose a reason for hiding this comment

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

Ah my bad, je pensais que le capitalize s'appliquerait sur les valeurs dans le test, mais ça n'arrive quand dans le rendu de la facture c'est ça ?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

C'est ça, oui. Mais je m'en suis sorti sans. Les modifications arrivent.

api/tests/files/invoice/rendered_nc_debit_note.html Outdated
@@ -194,7 +194,7 @@
</svg>
<h1>Note de débit</h1>
<div class="customerInfo">
<p><b>Structure :</b> Association de coiffeurs - 8533184</p>
<p><b style="text-transform: capitalize;">structure :</b> Association de coiffeurs - 8533184</p>
Copy link
Contributor

Choose a reason for hiding this comment

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

Quand on aura supprimé le FF OA, il faudra se débarrasser du capitalize, du coup

api/src/pcapi/core/finance/api.py Outdated
@@ -88,6 +88,7 @@
from . import repository
from . import utils
from . import validation
from ...models.feature import FeatureToggle
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
from ...models.feature import FeatureToggle
from pcapi.models.feature import FeatureToggle

On aime pas avoir trop de points :p

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Bad Pycharm !

prouzet-pass
prouzet-pass reviewed Nov 22, 2024
View reviewed changes
Copy link
Contributor

@prouzet-pass prouzet-pass left a comment

Choose a reason for hiding this comment

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

Comme cela concerne les documents finance, je laisse @vroullier-pass et @ataib-pass relire et approuver, je laisse juste un commentaire.

api/src/pcapi/core/finance/api.py Outdated
@@ -88,6 +88,7 @@
from . import repository
from . import utils
from . import validation
from ...models.feature import FeatureToggle
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
from ...models.feature import FeatureToggle
from pcapi.models.feature import FeatureToggle

vroullier-pass
vroullier-pass approved these changes Nov 22, 2024
View reviewed changes
Copy link
Contributor

@vroullier-pass vroullier-pass left a comment

Choose a reason for hiding this comment

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

Nice

@abouabdallaoui-pass abouabdallaoui-pass merged commit d73a9ad into master Nov 22, 2024
24 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@prouzet-pass prouzet-pass prouzet-pass left review comments

@ataib-pass ataib-pass ataib-pass approved these changes

@vroullier-pass vroullier-pass vroullier-pass approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@abouabdallaoui-pass @vroullier-pass @ataib-pass @prouzet-pass