refactor: switch organization screen to new Redux api #752
Conversation
…api" This reverts commit fdabf02.
|
|
||
| export const ACTIVITY_GET_EVENTS_RECEIVED = createPaginationActionSet( | ||
| 'ACTIVITY_GET_EVENTS_RECEIVED' | ||
| ); | ||
| export const ACTIVITY_GET_STARRED_REPOS_FOR_USER = createPaginationActionSet( | ||
| 'ACTIVITY_GET_STARRED_REPOS_FOR_USER' | ||
| ); | ||
| export const ORGS_GET_MEMBERS = createPaginationActionSet('ORGS_GET_MEMBERS'); | ||
|
|
||
| export const ORGS_GET_BY_ID = createActionSet('ORGS_GET_BY_ID'); |
There was a problem hiding this comment.
ORGS_GET_BY_ID uses createActionSet instead of createPaginationActionSet, as we're retrieving a single entity.
| return this.call(`orgs/${orgId}`, params).then(response => ({ | ||
| response, | ||
| schema: Schemas.ORG, | ||
| })); |
There was a problem hiding this comment.
so this time, no nextPageUrl, and our schema is an object, not an array of objects
| } = state; | ||
|
|
||
| const orgId = ownProps.navigation.state.params.organization.login; | ||
| const org = orgs[orgId] || ownProps.navigation.state.params.organization; |
There was a problem hiding this comment.
Something important to have in mind:
This screen is accessed from a User profile, or from the Search results.
In both cases, ownProps.navigation.state.params.organization contains an incomplete Organization object (left), which is not enough to render the screen, as we need the number of repositories, etc, which is only present after calling the organization endpoint to get the complete object (right):
| initial object | after fetching information |
|---|---|
 |
 |
This is why, with or without this PR, if you navigate to a user profile, turn off internet, and then click on one of his organizations, you will see a broken incomplete UI. Usually the GitHub API call is so quick that it goes unnoticed.
Normally, org would only be read from state.orgs, which would already contain the incomplete organization object. Since the User profile & Search screens doesn't use the new API yet, hence, doesn't populate state.orgs with the incomplete object, I'm retrieving it from the navigation state.
Reading from navigation will be removed in a future PR, once the new Redux API is used everywhere. Only the organization id will be passed in the navigation object, and the Redux API will go get the object for us.
If we use this same approach for all our screens (receiving only IDs, no objects, and letting the redux API fetch things), GitPoint will be able to handle all Github.com urls with deep linking 🎉
There was a problem hiding this comment.
Yeah this is something I really had to stress with a bit building out the navigation flows for the first time. And it's tricky. This applies to orgs, users, repos, etc....(essentially every element that is accessed through a previous list screen).
Appreciate the breakdown and will keep it mind. This is a great and simple way to rely on the complete object but if it's not available fallback to the short form version
There was a problem hiding this comment.
I wouldn't have done that differently as a first approach, but working with this new Redux API & having in mind deep linking, it's now obvious that every screen should be "standalone" and be able to fetch it's data with simple ids.
A pull screen for example would require a repository identifier & the pull number, just like this Pull Request url on GitHub web:
https://github.com/**gitpoint/git-point**/pull/**752**
nahmean?
| this.props.fetchOrganizationMembers(organization.login); | ||
| if (!org.name) { | ||
| getOrgById(orgId); | ||
| } |
There was a problem hiding this comment.
This if does a little caching, to avoid refetching from the REST api if we already have the complete organization object (see comment below).
This is needed for now as single object retrieval is not cached in the new Redux API, and will be automatically handled soon.
| dispatch(getOrgMembersLoading(false)); | ||
| dispatch(getOrgMembersError(error)); | ||
| }); | ||
| }; |
There was a problem hiding this comment.
Sheeeeeesh all this boilerplate gone :) :) :)
| } = state; | ||
|
|
||
| const orgId = ownProps.navigation.state.params.organization.login; | ||
| const org = orgs[orgId] || ownProps.navigation.state.params.organization; |
There was a problem hiding this comment.
Yeah this is something I really had to stress with a bit building out the navigation flows for the first time. And it's tricky. This applies to orgs, users, repos, etc....(essentially every element that is accessed through a previous list screen).
Appreciate the breakdown and will keep it mind. This is a great and simple way to rely on the complete object but if it's not available fallback to the short form version
Description
Switched the Organization screen to the new API, the members list is now paged 🎉 (check the Facebook org for example)
Now we're already familiar with the paginated stuff. The implementation in this PR is similar to the previous ones.
This PR introduces "single entity" retrieval using the new Redux API, in order to fetch the organization information. This process is simpler than the pagination, as we're not dealing with nextUrl, or anything like that.
PS: Sorry for the 3 first unrelated commits, I managed to mess my fork 😓