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-30781)[API] feat: add allowed actions to collective offers api #14151

Merged
merged 2 commits into from
Sep 19, 2024
Merged

(PC-30781)[API] feat: add allowed actions to collective offers api #14151

merged 2 commits into from
Sep 19, 2024

Conversation

jcicurel-pass
Copy link
Contributor

@jcicurel-pass jcicurel-pass commented Sep 16, 2024 •
edited
Loading

But de la pull request

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

Ajout des actions autorisées en fonction du displayedStatus dans les api collective offers

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

@jcicurel-pass jcicurel-pass force-pushed the PC-30781-eac-back-renvoyer-au-front-les-regles-de-gestion branch 2 times, most recently from 1e2df26 to 9db6ae0 Compare September 16, 2024 13:37
@github-actions GitHub Actions
Copy link
Contributor

github-actions bot commented Sep 16, 2024 •
edited
Loading

Visit the preview URL for this PR (updated for commit 2822470):

https://pc-pro-testing--pr14151-pc-30781-eac-back-re-tw59ukbz.web.app

(expires Sat, 21 Sep 2024 08:44:35 GMT)

🔥 via Firebase Hosting GitHub Action 🌎

Sign: 032d233ee67e1c50d6af12e29c936c7076770eb1

@jcicurel-pass jcicurel-pass force-pushed the PC-30781-eac-back-renvoyer-au-front-les-regles-de-gestion branch from 9db6ae0 to 4b6259e Compare September 16, 2024 13:58
@jcicurel-pass jcicurel-pass marked this pull request as ready for review September 16, 2024 13:58
mageoffray
mageoffray approved these changes Sep 17, 2024
View reviewed changes
api/src/pcapi/core/educational/api/offer.py
@@ -19,8 +19,6 @@
from pcapi.core.educational.api import adage as educational_api_adage
import pcapi.core.educational.api.national_program as national_program_api
from pcapi.core.educational.exceptions import AdageException
from pcapi.core.educational.models import CollectiveOffer
Copy link
Contributor

Choose a reason for hiding this comment

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

💚

Copy link
Contributor

Choose a reason for hiding this comment

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

De façon générale on préfère les imports de modules pour ensuite faire my_module.my_method(...)

api/src/pcapi/routes/serialization/collective_offers_serialize.py Outdated
@@ -16,6 +16,7 @@
from pcapi.core.educational.models import CollectiveBooking
from pcapi.core.educational.models import CollectiveBookingStatus
from pcapi.core.educational.models import CollectiveOffer
from pcapi.core.educational.models import CollectiveOfferAllowedAction
Copy link
Contributor

Choose a reason for hiding this comment

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

Dans l'idée du premier commentaire, si tu veux faire un petit commit de nettoyage en transformant
from pcapi.core.educational import models as educational_models
Et remplacer CollectiveBooking (et les autres) par educational_models.CollectiveBooking. Petit à petit on aura des jolis imports :).

anoukhello
anoukhello reviewed Sep 17, 2024
View reviewed changes
api/src/pcapi/core/educational/models.py Outdated
CollectiveOfferAllowedAction.CAN_EDIT_DATES,
CollectiveOfferAllowedAction.CAN_EDIT_DISCOUNT,
CollectiveOfferAllowedAction.CAN_DUPLICATE,
CollectiveOfferAllowedAction.CAN_CANCEL,
Copy link
Contributor

Choose a reason for hiding this comment

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

En repassant sur ce tableau je me rends compte que celui-ci n'est pas tout à fait aligné, j'ai mis en rouge les cases qui changent par rapport à ce sur quoi tu t'es basé.
Par exemple ici, on ne doit pas pouvoir annuler une offre publiée (ce sera une amélioration future) et plus bas on ne veut pas pouvoir archiver une offre terminée.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

ça marche je change ça

@jcicurel-pass jcicurel-pass force-pushed the PC-30781-eac-back-renvoyer-au-front-les-regles-de-gestion branch from a622dd1 to 07d02d7 Compare September 17, 2024 14:48
@jcicurel-pass jcicurel-pass force-pushed the PC-30781-eac-back-renvoyer-au-front-les-regles-de-gestion branch from 07d02d7 to 0db810d Compare September 19, 2024 08:03
@jcicurel-pass jcicurel-pass force-pushed the PC-30781-eac-back-renvoyer-au-front-les-regles-de-gestion branch from 0db810d to 2822470 Compare September 19, 2024 08:34
@jcicurel-pass jcicurel-pass merged commit 385d5a2 into master Sep 19, 2024
26 checks passed
@jcicurel-pass jcicurel-pass deleted the PC-30781-eac-back-renvoyer-au-front-les-regles-de-gestion branch September 19, 2024 10:07
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@anoukhello anoukhello anoukhello approved these changes

@mageoffray mageoffray mageoffray 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.
3 participants
@jcicurel-pass @anoukhello @mageoffray