(PC-32301) feat(XPCine): add next movie feature #7040
Conversation
xlecunff-pass
changed the title
xlecunff-pass
force-pushed
the
PC-32301
branch
from
b27c79e
to
864da86
Compare
Performance Comparison ReportSignificant Changes To Render Duration
Show details
Meaningless Changes To Render DurationShow entries
Show details
Changes To Render CountThere are no entries Added ScenariosThere are no entries Removed ScenariosThere are no entries |
xlecunff-pass
force-pushed
the
PC-32301
branch
11 times, most recently
from
2651d2c
to
de06d07
Compare
| setSelectedDate(nextScreeningDate) | ||
| scrollToMiddleElement(nextDateIndex) | ||
| }} | ||
| onPress={() => goToDate(nextScreeningDate)} |
There was a problem hiding this comment.
J'aime beaucoup ce goToDate(date) je trouve ça très clair à la lecture
| const useMoviesScreeningsList = (offerIds: number[]) => { | ||
| const { selectedDate } = useMovieCalendar() | ||
| const { data: offersWithStocks } = useOffersStocks({ offerIds }) | ||
|
|
||
| const moviesOffers: MovieOffer[] = useMemo(() => { | ||
| const filteredOffersWithStocks = filterOffersStocksByDate( | ||
| offersWithStocks?.offers || [], | ||
| selectedDate | ||
| ) | ||
| const nextScreeningOffers = getNextMoviesByDate(offersWithStocks?.offers || [], selectedDate) | ||
|
|
||
| return [...filteredOffersWithStocks, ...nextScreeningOffers] | ||
| }, [offersWithStocks?.offers, selectedDate]).filter( | ||
| (offer) => offer.offer.subcategoryId === SubcategoryIdEnum.SEANCE_CINE | ||
| ) | ||
|
|
||
| return { | ||
| moviesOffers, | ||
| } | ||
| } |
There was a problem hiding this comment.
En arrivant dans le fichier VenueCalendar, je m'attends à voir le composant et pas un hook en premier, du coup je l'aurais bien extrait dans son propre fichier ce hook 🙂
There was a problem hiding this comment.
Le tout dans un petit dossier hook comme on peut le retrouver pour d'autres features gtlPlaylist par exemple
| @@ -82,7 +82,9 @@ describe('useGetVenueByDay', () => { | |||
|
|
|||
| await act(async () => {}) | |||
|
|
|||
| expect(result.current.items).toHaveLength(1) | |||
| const filteredItems = result.current.items.filter((item) => !item.nextDate) | |||
There was a problem hiding this comment.
Je ne suis pas sûre de comprendre le test, tu récupères result du hook, mais ensuite dans le test tu filtres sur la donnée pour retirer les items dont tu ne veux pas et tu expect d'avoir bien 1 seule entrée dans ton tableau ce qui semble certain vu que tu as filtré juste avant 🤔 est ce qu'on ne veut pas vérifier que le hook fait ça et renvoie la donnée déjà filtrée ? Je ne comprends pas pourquoi ce filter doit être fait dans le test ?
There was a problem hiding this comment.
Oui on est pas censé avoir de logique dans les tests 🤔
| @@ -185,7 +187,9 @@ describe('useGetVenueByDay', () => { | |||
| rerender({ date: TOMORROW.toDate() }) | |||
| }) | |||
|
|
|||
| expect(result.current.items).toHaveLength(5) | |||
| const filteredItems = result.current.items.filter((item) => !item.nextDate) | |||
There was a problem hiding this comment.
Pareil, pas sûre de comprendre pourquoi c'est dans le test qu'on fait un filter
| return moviesOfferBuilder(offersWithStocks?.offers) | ||
| .withoutMoviesAfter15Days() | ||
| .sortedByDistance(location) | ||
| .withoutMoviesOnDay(date) | ||
| .withNextScreeningFromDate(date) | ||
| .build() |
There was a problem hiding this comment.
Je comprends pas bien ce morceau de code même si j'aime bien l'idée du builder.
J'ai du mal à faire la différence entre Movies et Screening.
Et je comprends pas ce que veut dire : je veux des offres withoutMoviesAfter15Days, je le traduis en "je veux des offres sans films après 15 jours", ou même "je veux des offres sans séances après 15 jours", mais je comprends toujours pas.
Sachant que je veux des offres "sans séances après 15 jours" et "sans séances le date" et "avec la prochaine séance à partir de date", ça m'embrouille.
En réflechissant, ce qu'on veut c'est des séances entre date + 1 jour et aujourd'hui + 15 jours c'est ça ? Tu penses de mettre une fonction dans ton builder où on peut choper les offres qui ont des séances dans un intervalle ?
There was a problem hiding this comment.
Merci pour le commentaire, je suis tout à fait d'accord il faudra choisir un naming plus approprié et indiquer le range de jours dans la méthode c'est pas tip top
Je vais merge en l'état et on pourra reprendre ce sujet dans un autre ticket
xlecunff-pass
force-pushed
the
PC-32301
branch
from
de06d07
to
bb714bb
Compare
|
|
Link to JIRA ticket: https://passculture.atlassian.net/browse/PC-32301
Checklist
I have:
Screenshots