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

(PC-31422)[PRO] feat: default statut filter value #14042

Merged
merged 2 commits into from
Sep 19, 2024

Conversation

smokhtari-passculture
Copy link
Contributor

@smokhtari-passculture smokhtari-passculture commented Sep 9, 2024

But de la pull request

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

Le but de la PR est d'avoir certains statut coché par défaut pour les offres collectives.
Il faut aussi prendre en compte le FF : WIP_ENABLE_NEW_COLLECTIVE_OFFERS_AND_BOOKINGS_STRUCTURE qui split les offres en deux pages donc les statuts par défaut sont différents pour la page vitrine et la page réservation
Ne pas oublier de tester l'existant avec le FF pour les brouillons activés : WIP_ENABLE_COLLECTIVE_DRAFT_OFFERS

Le FF pour les brouillons sera activé et supprimé avant l'activation du FF qui split les offres vitrines et réservations donc pas besoin de rajouter des conditions pour afficher les brouillons ou non dans les valeurs par défauts du filtre statut

Vérifications

  • J'ai écrit les tests nécessaires

Copy link
Contributor

github-actions bot commented Sep 9, 2024

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

https://pc-pro-testing--pr14042-pc-31422-default-fil-u7fg7d3m.web.app

(expires Sat, 21 Sep 2024 10:03:39 GMT)

🔥 via Firebase Hosting GitHub Action 🌎

Sign: 032d233ee67e1c50d6af12e29c936c7076770eb1

@smokhtari-passculture smokhtari-passculture force-pushed the pc-31422-default-filter-eac-list branch 2 times, most recently from a02993b to 8bd55a4 Compare September 10, 2024 16:07
Copy link
Contributor

@gmeigniez-pass gmeigniez-pass left a comment

Choose a reason for hiding this comment

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

Très casse tête comme sujet 😨 J'ai quelques retours pour des détails pour ces histoires de FF. Mais bien joué pour les changement, ça fonctionne c'est juste des détails!

Copy link
Contributor

@gmeigniez-pass gmeigniez-pass left a comment

Choose a reason for hiding this comment

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

Nickel pour les changements, et sans le FF brouillon ça va grandement simplifier 👍

@smokhtari-passculture smokhtari-passculture force-pushed the pc-31422-default-filter-eac-list branch 2 times, most recently from 00def84 to 69f78ac Compare September 18, 2024 14:15
Comment on lines 98 to 107
[
CollectiveOfferDisplayedStatus.PENDING,
CollectiveOfferDisplayedStatus.REJECTED,
CollectiveOfferDisplayedStatus.ACTIVE,
CollectiveOfferDisplayedStatus.INACTIVE,
CollectiveOfferDisplayedStatus.PREBOOKED,
CollectiveOfferDisplayedStatus.BOOKED,
CollectiveOfferDisplayedStatus.EXPIRED,
CollectiveOfferDisplayedStatus.ENDED,
],
Copy link
Contributor

Choose a reason for hiding this comment

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

On pourrait peut-être définir cette liste dans une constante plutôt que la répéter 15 fois dans le fichier ?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

yes, merci pour le commentaire je ne sais pas ce qui m'a pris surtout que la constante existait déjà

Copy link
Contributor

Choose a reason for hiding this comment

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

Ahah tqt ça arrive quand on a affaire à un ticket complexe comme celui ci !!

Comment on lines 122 to 127
await waitFor(async () => {
await userEvent.selectOptions(
screen.getByDisplayValue(venueName),
ALL_VENUES
)
})
Copy link
Contributor

Choose a reason for hiding this comment

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

await userEvent.selectOptions(
  await screen.findByDisplayValue(venueName),
  ALL_VENUES
)

Est-ce que ça fonctionne comme ça ? Il me semble que les findBy... sont équivalents à await waitFor( ... screen.getBy...)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

je n'ai pas compris la question

Copy link
Contributor

Choose a reason for hiding this comment

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

Yes je reformule, est-ce que ça ça fonctionne

await userEvent.selectOptions(
  await screen.findByDisplayValue(venueName),
  ALL_VENUES
)

à la place de :

await waitFor(async () => {
  await userEvent.selectOptions(
    screen.getByDisplayValue(venueName),
    ALL_VENUES
  )
})

?
Car il me semble que les deux sont équivalents, mais c'est vraiment un détail :)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

ah oui tu as raison et c'est plus propre

@smokhtari-passculture smokhtari-passculture merged commit b4bbbdc into master Sep 19, 2024
18 checks passed
@smokhtari-passculture smokhtari-passculture deleted the pc-31422-default-filter-eac-list branch September 19, 2024 12:29
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