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-33637) feat(offer): add chronicles section #7556

Merged
merged 5 commits into from
Jan 27, 2025

Conversation

mmeissonnier-pass
Copy link
Contributor

@mmeissonnier-pass mmeissonnier-pass commented Jan 21, 2025

Link to JIRA ticket: https://passculture.atlassian.net/browse/PC-33637

Intégration de la section "L'avis du book club" dans la page offre. Les données proviennent du backend

  • Mise à jour des type de l'api

  • Mise à jour du hook useChronicles pour pouvoir passer une fonction de transformation directement dans le select du useQuery

  • Refactor des composants ChronicleCard et ChronicleCardList pour permettre plus de flexibilité côté natif et côté web.

  • Intégration du composant ChronicleCardList dans la page Offre et câblage avec la donnée

  • Remontée de certains composants depuis OfferBody vers OfferContentBase

  • Possibilité de gérer l'écart entre les tuiles via une prop

Flakiness

If I had to re-run tests in the CI due to flakiness, I add the incident on Notion

Checklist

I have:

  • Made sure my feature is working on web.
  • Made sure my feature is working on mobile (depending on relevance : real or virtual devices)
  • Written unit tests native (and web when implementation is different) for my feature.
  • Added a screenshot for UI tickets or deleted the screenshot section if no UI change
  • If my PR is a bugfix, I add the link of the "résolution de problème sur le bug" on Notion
  • I am aware of all the best practices and respected them.

Screenshots

Android

Kapture.2025-01-23.at.00.57.47.mp4

iOS

Kapture.2025-01-23.at.00.59.58.mp4

Web

Kapture.2025-01-23.at.11.59.09.mp4
Kapture.2025-01-23.at.12.00.07.mp4

Best Practices

Click to expand These rules apply to files that you make changes to. If you can't respect one of these rules, be sure to explain why with a comment. If you consider correcting the issue is too time consuming/complex: create a ticket. Link the ticket in the code.
  • In the production code: remove type assertions with as (type assertions are removed at compile-time, there is no runtime checking associated with a type assertion. There won’t be an exception or null generated if the type assertion is wrong). In certain cases as const is acceptable (for example when defining readonly arrays/objects). Using as in tests is tolerable.
  • Remove bypass type checking with any (when you want to accept anything because you will be blindly passing it through without interacting with it, you can use unknown). Using any in tests is tolerable.
  • Remove non-null assertion operators (just like other type assertions, this doesn’t change the runtime behavior of your code, so it’s important to only use ! when you know that the value can’t be null or undefined).
  • Remove all @ts-expect-error and @eslint-disable.
  • Remove all warnings, and errors that we are used to ignore (yarn test:lint, yarn test:types, yarn start:web...).
  • Use gap (ViewGap) instead of <Spacer.Column />, <Spacer.Row /> or <Spacer.Flex />.
  • Don't add new "alias hooks" (hooks created to group other hooks together). When adding new logic, this hook will progressively become more complex and harder to maintain.
  • Remove logic from components that should be dumb.

Test specific:

  • Avoid mocking internal parts of our code. Ideally, mock only external calls.
  • When you see a local variable that is over-written in every test, mock it.
  • Prefer user to fireEvent.
  • When mocking feature flags, use setFeatureFlags. If not possible, mention which one(s) you want to mock in a comment (example: jest.spyOn(useFeatureFlagAPI, 'useFeatureFlag').mockReturnValue(true) // WIP_NEW_OFFER_TILE in renderPassPlaylist.tsx )
  • In component tests, replace await act(async () => {}) and await waitFor(/* ... */) by await screen.findBySomething().
  • In hooks tests, use act by default and waitFor as a last resort.
  • Make a snapshot test for pages and modals ONLY.
  • Make a web specific snapshot when your web page/modal is specific to the web.
  • Make an a11y test for web pages.

Advice:

  • Use TDD
  • Use Storybook
  • Use pair programming/mobs

@mmeissonnier-pass mmeissonnier-pass force-pushed the feat/PC-33637-offer-chronicles-section branch 19 times, most recently from 1e2b046 to e4b5164 Compare January 23, 2025 11:41
@mmeissonnier-pass mmeissonnier-pass marked this pull request as ready for review January 23, 2025 11:56
@mmeissonnier-pass mmeissonnier-pass force-pushed the feat/PC-33637-offer-chronicles-section branch 2 times, most recently from 8834167 to 2c2fe27 Compare January 23, 2025 17:22
@mmeissonnier-pass mmeissonnier-pass force-pushed the feat/PC-33637-offer-chronicles-section branch from 2c2fe27 to e49078e Compare January 23, 2025 17:53
@mmeissonnier-pass mmeissonnier-pass force-pushed the feat/PC-33637-offer-chronicles-section branch 2 times, most recently from e702db2 to 7418716 Compare January 24, 2025 17:16
@mmeissonnier-pass mmeissonnier-pass force-pushed the feat/PC-33637-offer-chronicles-section branch 2 times, most recently from e391fc7 to fff3819 Compare January 27, 2025 11:24
@mmeissonnier-pass mmeissonnier-pass force-pushed the feat/PC-33637-offer-chronicles-section branch 3 times, most recently from b8825a1 to 7fbf2f5 Compare January 27, 2025 12:52
@mmeissonnier-pass mmeissonnier-pass force-pushed the feat/PC-33637-offer-chronicles-section branch 6 times, most recently from 103494d to 6759081 Compare January 27, 2025 15:40
@mmeissonnier-pass mmeissonnier-pass force-pushed the feat/PC-33637-offer-chronicles-section branch from 6759081 to 987325e Compare January 27, 2025 16:00
Copy link
Contributor

@clesausse-pass clesausse-pass left a comment

Choose a reason for hiding this comment

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

D'après les règles il vaut mieux privilégié l'act vide plus que le waitFor mais ok

@mmeissonnier-pass mmeissonnier-pass merged commit 2a8c335 into master Jan 27, 2025
53 checks passed
@mmeissonnier-pass mmeissonnier-pass deleted the feat/PC-33637-offer-chronicles-section branch January 27, 2025 18:03
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