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

fix: use the same relay env between Home screen and useMutation #8636

Merged
merged 6 commits into from
May 9, 2023

Conversation

dimatretyak
Copy link
Contributor

@dimatretyak dimatretyak commented May 5, 2023

This PR resolves CX-3586

Related PR: #8442

Description

The problem in the CX-3586 task was that we used different relay env on the Home screen and useMutation (this hook uses env, which is passed here).

We have already faced such a problem here and here + some related PR from George

Demo

Artists

Android iOS
Artists.mp4
artists.mp4

Artworks

Android iOS
Artworks.mp4
artworks.mp4

PR Checklist

  • I have tested my changes on iOS and Android.
  • I hid my changes behind a feature flag, or they don't need one.
  • I have included screenshots or videos, or I have not changed the UI.
  • I have added tests, or my changes don't require any.
  • I added an app state migration, or my changes do not require one.
  • I have documented any follow-up work that this PR will require, or it does not require any.
  • I have added a changelog entry below, or my changes do not require one.

To the reviewers 👀

  • I would like at least one of the reviewers to run this PR on the simulator or device.
Changelog updates

Changelog updates

Cross-platform user-facing changes

iOS user-facing changes

Android user-facing changes

Dev changes

  • use the same relay env between Home screen and useMutation - dimatretyak

Need help with something? Have a look at our docs, or get in touch with us.

@dimatretyak dimatretyak self-assigned this May 5, 2023
@dimatretyak dimatretyak marked this pull request as ready for review May 5, 2023 17:33
gkartalis
gkartalis previously approved these changes May 5, 2023
Copy link
Member

@gkartalis gkartalis left a comment

Choose a reason for hiding this comment

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

Oof 😅 thanks Dima! Will create a ticket for this on MOPLAT to address all of these issues in all the places that we have the default environment!

cc @damassi

@dimatretyak
Copy link
Contributor Author

Will create a ticket for this on MOPLAT to address all of these issues in all the places that we have the default environment!

@gkartalis thank you 🙏

Comment on lines 23 to 27
let mockEnvironment: ReturnType<typeof createMockEnvironment>

beforeEach(() => {
mockEnvironment = createMockEnvironment()
})
Copy link
Member

Choose a reason for hiding this comment

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

Here we could also use 'getRelayMockEnvironment' theoretically, no need to refactor though

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I'm doing it right now. I noticed getRelayMockEnvironment at the last moment

Copy link
Contributor Author

Choose a reason for hiding this comment

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

When using getRelayMockEnvironment I get an error. I'll try to understand the reason

image

Copy link
Contributor Author

@dimatretyak dimatretyak May 5, 2023

Choose a reason for hiding this comment

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

This will work if we manually pass mocked env to the Home query renderer component: 1ee7eb8

olerichter00
olerichter00 previously approved these changes May 8, 2023
Copy link
Contributor

@olerichter00 olerichter00 left a comment

Choose a reason for hiding this comment

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

Thanks a lot for the fix @dimatretyak 🌟

@dimatretyak dimatretyak dismissed stale reviews from olerichter00 and gkartalis via ac0bab7 May 9, 2023 11:03
@dimatretyak dimatretyak added Squash On Green A label to indicate that Peril should squash-merge this PR when all statuses are green and removed Squash On Green A label to indicate that Peril should squash-merge this PR when all statuses are green labels May 9, 2023
@dimatretyak dimatretyak force-pushed the dimatretyak/fix/home-screen-refetch-data branch from ac0bab7 to adbde24 Compare May 9, 2023 11:16
@dimatretyak dimatretyak added the Squash On Green A label to indicate that Peril should squash-merge this PR when all statuses are green label May 9, 2023
@dimatretyak dimatretyak merged commit c868677 into main May 9, 2023
@dimatretyak dimatretyak deleted the dimatretyak/fix/home-screen-refetch-data branch May 9, 2023 13:07
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Jira Synced Squash On Green A label to indicate that Peril should squash-merge this PR when all statuses are green
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants