(BSR) refactor(Settings): remove SettingsContext #7593
Conversation
xlecunff-pass
changed the title
xlecunff-pass
force-pushed
the
remove-settings-context-2
branch
2 times, most recently
from
58099f6 to
283323a
Compare
xlecunff-pass
force-pushed
the
remove-settings-context-2
branch
17 times, most recently
from
e754306 to
90d3d91
Compare
xlecunff-pass
force-pushed
the
remove-settings-context-2
branch
4 times, most recently
from
4ec4a68 to
7426ebb
Compare
xlecunff-pass
force-pushed
the
remove-settings-context-2
branch
from
7426ebb to
98c7a76
Compare
src/features/auth/pages/forgottenPassword/ForgottenPassword/ForgottenPassword.native.test.tsx
Outdated
Show resolved
Hide resolved
xlecunff-pass
force-pushed
the
remove-settings-context-2
branch
from
62c52ec to
7520abb
Compare
|
|
There was a problem hiding this comment.
je suis étonné que les settings soient nécessaires dans autant de tests
je pense que ça ne devrait pas etre un BSR, tu peux voir avec @lbeneston-pass pour créer un ticket
| [QueryKeys.SETTINGS], | ||
| () => api.getNativeV1Settings(), | ||
| { | ||
| staleTime: STALE_TIME_APP_SETTINGS, |
There was a problem hiding this comment.
avant on avait, ça, ce n'est plus nécessaire ?
enabled: !!netInfo.isConnected && !!netInfo.isInternetReachable,| expect(result.current.offers[0]?.objectID).toEqual(offersFixture[0].objectID) | ||
| expect(result.current.offers[1]?.objectID).toEqual(offersFixture[1].objectID) |
There was a problem hiding this comment.
je trouvais l'expect d'avant plus explicite
pourquoi a-t-on besoin de le changer ?
| mockSettings() | ||
|
|
||
| jest.mock('features/identityCheck/context/SubscriptionContextProvider') | ||
| jest.mock('ui/theme/customFocusOutline/customFocusOutline') | ||
| mockSettings() |
There was a problem hiding this comment.
je pense qu'une seule fois serait suffisante
| renderSetAddress() | ||
|
|
||
| const input = screen.getByPlaceholderText('Ex\u00a0: 34 avenue de l’Opéra') | ||
| fireEvent.changeText(input, QUERY_ADDRESS) | ||
|
|
||
| fireEvent.press(screen.getByText('Continuer')) | ||
|
|
||
| await screen.findByText('Recherche et sélectionne ton adresse') | ||
| await screen.findByText('Entre ton adresse') |
There was a problem hiding this comment.
comment ça se fait que ça passait avant ?
| () => api.getNativeV1Settings(), | ||
| { | ||
| staleTime: STALE_TIME_APP_SETTINGS, | ||
| initialData: defaultSettings, |
There was a problem hiding this comment.
avant, on n'avait pas de données par défaut
le defaultSettings actuellement utilisé est censé etre utilisé par des tests, avec des données invalides en prod
exemple: on a un localhost qui traine dedans
je ne suis pas contre avoir des données par défaut
je pense qu'il faudrait éviter d'utiliser cette fixture
| jest.mock('styled-components/native') | ||
| jest.mock('features/auth/context/SettingsContext') | ||
| jest.mock('libs/monitoring/services') |
There was a problem hiding this comment.
comment ça se fait qu'avant on n'en avait pas besoin ?
Comme React Query gère son cache qui est déjà accessible depuis n'importe quelle partie de l'application, il me semblait inutile d'y accéder via un contexte React