Skip to content

feat: Make the starred repositories list paged #750

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

Merged
merged 5 commits into from
Mar 26, 2018
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions src/api/actions/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -3,3 +3,6 @@ import { createPaginationActionSet } from 'utils';
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'
);
Copy link
Member Author

Choose a reason for hiding this comment

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

First, we create our new actions. A "pagination action" in a object composed of the 3 usual sub-actions (pending, success, error) and an additional one: reset.

7 changes: 7 additions & 0 deletions src/api/client.js
Original file line number Diff line number Diff line change
Expand Up @@ -141,5 +141,12 @@ export class Client {
})
);
},
getStarredReposForUser: async (userId, params) => {
return this.call(`users/${userId}/starred`, params).then(response => ({
response,
nextPageUrl: this.getNextPageUrl(response),
schema: Schemas.REPO_ARRAY,
}));
},
Copy link
Member Author

Choose a reason for hiding this comment

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

Second, we implement our new API call: client.activity.getStarredReposForUser(userId)

The schema key tells which kind of objects the call to GitHub will be returning: An array of repositories.

PS: I'm following @oktokit/rest for the choice of namespace/method names.

Copy link
Member

Choose a reason for hiding this comment

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

Outside the scope of this PR, but curious

Where is nextPageUrl being used? Is it part of the pagination flow within proxy to define a nextPageUrl property so it knows how to continue moving past pages?

Copy link
Member Author

Choose a reason for hiding this comment

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

In our FlatList onEndReached, we call client.activity.getStarredReposForUser(login, { loadMore: true})

When the proxy sees this parameter, it overrides the GitHub URL that will be called with nextPageUrl: https://github.com/gitpoint/git-point/blob/master/src/api/proxies/create-dispatch-proxy.js#L54

};
}
3 changes: 3 additions & 0 deletions src/api/reducers/pagination.js
Original file line number Diff line number Diff line change
Expand Up @@ -78,4 +78,7 @@ const paginate = types => {
// Updates the pagination data for different actions.
export const pagination = combineReducers({
ACTIVITY_GET_EVENTS_RECEIVED: paginate(Actions.ACTIVITY_GET_EVENTS_RECEIVED),
ACTIVITY_GET_STARRED_REPOS_FOR_USER: paginate(
Actions.ACTIVITY_GET_STARRED_REPOS_FOR_USER
),
Copy link
Member Author

Choose a reason for hiding this comment

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

We tell our pagination reducer that it should take care of this action set.

});
3 changes: 3 additions & 0 deletions src/api/schemas/index.js
Original file line number Diff line number Diff line change
@@ -1,6 +1,9 @@
import { eventSchema } from './events';
import { repoSchema } from './repos';

export default {
EVENT: eventSchema,
EVENT_ARRAY: [eventSchema],
REPO: repoSchema,
REPO_ARRAY: [repoSchema],
};
9 changes: 9 additions & 0 deletions src/api/schemas/repos.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
import { schema } from 'normalizr';

export const repoSchema = new schema.Entity(
'repos',
{},
{
idAttribute: repo => repo.full_name.toLowerCase(),
}
);
88 changes: 68 additions & 20 deletions src/user/screens/starred-repository-list.screen.js
Original file line number Diff line number Diff line change
@@ -1,48 +1,85 @@
import React, { Component } from 'react';
import { connect } from 'react-redux';
import { bindActionCreators } from 'redux';
import { FlatList } from 'react-native';

import { getStarredRepositories } from 'user';
import { FlatList, View, ActivityIndicator } from 'react-native';

import { RestClient } from 'api';
import {
ViewContainer,
RepositoryListItem,
LoadingRepositoryListItem,
} from 'components';

const mapStateToProps = state => ({
user: state.user.user,
starredRepositories: state.user.starredRepositories,
isPendingStarredRepositories: state.user.isPendingStarredRepositories,
});
const mapStateToProps = (state, ownProps) => {
const {
auth: { user },
pagination: { ACTIVITY_GET_STARRED_REPOS_FOR_USER },
entities: { repos },
} = state;

const userId = ownProps.navigation.state.params.user.login;

const starredReposPagination = ACTIVITY_GET_STARRED_REPOS_FOR_USER[
userId
] || {
ids: [],
isFetching: true,
};
const starredRepos = starredReposPagination.ids.map(id => repos[id]);

return {
user,
starredReposPagination,
starredRepos,
userId,
};
};
Copy link
Member Author

Choose a reason for hiding this comment

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

mapStateToProps now create two props:

  • starredReposPagination: STORE.pagination.ACTIVITY_GET_STARRED_REPOS_FOR_USER['machour'] or and empty pagination
  • starredRepos: An array of repositories picked from STORE.entities.repos, filtered to contain only those whose id is present in my pagination.

Copy link
Member

Choose a reason for hiding this comment

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

Is starredReposPagination just an object that returns nextPageUrl and an isFetching bool?

Copy link
Member Author

@machour machour Mar 26, 2018

Choose a reason for hiding this comment

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

and pageCount and ids.

It's literally this in our case:

screen shot 2018-03-26 at 1 03 57 am


const mapDispatchToProps = dispatch =>
bindActionCreators({ getStarredRepositories }, dispatch);
const mapDispatchToProps = {
getStarredReposForUser: RestClient.activity.getStarredReposForUser,
};
Copy link
Member Author

Choose a reason for hiding this comment

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

this.props.getStarredReposForUser() is mapped to our API call.


class StarredRepositoryList extends Component {
props: {
user: Object,
starredRepositories: Array,
userId: String,
starredRepos: Array,
starredReposPagination: Object,
getStarredReposForUser: Function,
navigation: Object,
isPendingStarredRepositories: boolean,
getStarredRepositories: Function,
};

componentWillMount() {
const user = this.props.navigation.state.params.user;
const { getStarredReposForUser, userId } = this.props;

this.props.getStarredRepositories(user);
getStarredReposForUser(userId);
}

renderFooter = () => {
if (this.props.starredReposPagination.nextPageUrl === null) {
return null;
}

return (
<View
style={{
paddingVertical: 20,
}}
>
<ActivityIndicator animating size="large" />
</View>
);
};

render() {
const {
starredRepositories,
starredReposPagination,
starredRepos,
userId,
navigation,
isPendingStarredRepositories,
} = this.props;

const repoCount = navigation.state.params.repoCount;
const isPendingStarredRepositories =
starredRepos.length === 0 && starredReposPagination.isFetching;

return (
<ViewContainer>
Expand All @@ -52,7 +89,18 @@ class StarredRepositoryList extends Component {
)}
{!isPendingStarredRepositories && (
<FlatList
data={starredRepositories}
data={starredRepos}
onRefresh={() =>
this.props.getStarredReposForUser(userId, {
forceRefresh: true,
})
}
Copy link
Member Author

@machour machour Mar 25, 2018

Choose a reason for hiding this comment

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

Remember how our action set contains the 3 usual pending,success,error and an additional 'reset' thingy?
'reset' is dispatched when 'forceRefresh' is set to true, and it will simply wipe out STORE.pagination.ACTIVITY_GET_STARRED_REPOS_FOR_USER['machour'], so that fresh information is refetched from the GitHub endpoint

Copy link
Member

Choose a reason for hiding this comment

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

Was just about to ask about why we have reset. You've thought this through mate :)

refreshing={!starredRepos && starredReposPagination.isFetching}
onEndReached={() =>
this.props.getStarredReposForUser(userId, { loadMore: true })
Copy link
Member Author

Choose a reason for hiding this comment

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

loadMore tells the proxy to use nextPageUrl as endpoint for the next API call, instead of the original endpoint, and the new results are appended to the pagination.

}
onEndReachedThreshold={0.5}
ListFooterComponent={this.renderFooter}
renderItem={({ item }) => (
<RepositoryListItem repository={item} navigation={navigation} />
)}
Expand Down
25 changes: 0 additions & 25 deletions src/user/user.action.js
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,6 @@ import {
GET_IS_FOLLOWING,
GET_IS_FOLLOWER,
GET_REPOSITORIES,
GET_STARRED_REPOSITORIES,
Copy link
Member Author

Choose a reason for hiding this comment

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

And here, we removed our old boring actions, types, reducers. So long!

GET_FOLLOWERS,
GET_FOLLOWING,
SEARCH_USER_REPOS,
Expand Down Expand Up @@ -202,30 +201,6 @@ export const getRepositories = user => {
};
};

export const getStarredRepositories = user => {
return (dispatch, getState) => {
const url = `/users/${user.login}/starred?per_page=50`;
const accessToken = getState().auth.accessToken;

dispatch({ type: GET_STARRED_REPOSITORIES.PENDING });

v3
.getJson(url, accessToken)
.then(data => {
dispatch({
type: GET_STARRED_REPOSITORIES.SUCCESS,
payload: data,
});
})
.catch(error => {
dispatch({
type: GET_STARRED_REPOSITORIES.ERROR,
payload: error,
});
});
};
};

export const getFollowing = user => {
return (dispatch, getState) => {
const accessToken = getState().auth.accessToken;
Expand Down
18 changes: 0 additions & 18 deletions src/user/user.reducer.js
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,6 @@ import {
GET_IS_FOLLOWING,
GET_IS_FOLLOWER,
GET_REPOSITORIES,
GET_STARRED_REPOSITORIES,
GET_FOLLOWERS,
GET_FOLLOWING,
SEARCH_USER_REPOS,
Expand Down Expand Up @@ -165,23 +164,6 @@ export const userReducer = (state = initialState, action = {}) => {
error: action.payload,
isPendingRepositories: false,
};
case GET_STARRED_REPOSITORIES.PENDING:
return {
...state,
isPendingStarredRepositories: true,
};
case GET_STARRED_REPOSITORIES.SUCCESS:
return {
...state,
starredRepositories: action.payload,
isPendingStarredRepositories: false,
};
case GET_STARRED_REPOSITORIES.ERROR:
return {
...state,
error: action.payload,
isPendingStarredRepositories: false,
};
case GET_FOLLOWERS.PENDING:
return {
...state,
Expand Down
3 changes: 0 additions & 3 deletions src/user/user.type.js
Original file line number Diff line number Diff line change
Expand Up @@ -5,9 +5,6 @@ export const GET_ORGS = createActionSet('GET_ORGS');
export const GET_IS_FOLLOWING = createActionSet('GET_IS_FOLLOWING');
export const GET_IS_FOLLOWER = createActionSet('GET_IS_FOLLOWER');
export const GET_REPOSITORIES = createActionSet('GET_REPOSITORIES');
export const GET_STARRED_REPOSITORIES = createActionSet(
'GET_STARRED_REPOSITORIES'
);
export const GET_FOLLOWERS = createActionSet('GET_FOLLOWERS');
export const GET_FOLLOWING = createActionSet('GET_FOLLOWING');
export const SEARCH_USER_REPOS = createActionSet('SEARCH_USER_REPOS');
Expand Down