feat: Make the starred repositories list paged #750

machour · 2018-03-25T15:45:00Z

Question	Response
Version?	v1.4.1
Devices tested?	iPhone 7
Bug fix?	no
New feature?	yes
Includes tests?	no
All Tests pass?	yes

Description

In order to further explain the new API, this PR demonstrate how we turn a limited list of repositories (50), into a paged list displaying all repositories. I did that on the starred repositories list displayed on a user profile.

By chance, the file changes are well ordered, so I'll break down the migration steps in comments.

Once merged, we will have two screens using the new pagination API (events & starred repos), so next PR will be about a little refactoring:

Adding a new <PaginatedFlatList /> component and using it in both screens.
Adding a getPagination() helper to reduce the amount code needed in mapStateToProps

…api" This reverts commit fdabf02.

coveralls · 2018-03-25T15:50:50Z

Coverage increased (+0.01%) to 43.4% when pulling 4a98a8b on machour:feat/paged-starred-repos into 7f82b48 on gitpoint:master.

machour

Added walk through.

machour · 2018-03-25T15:47:10Z

src/api/actions/index.js

@@ -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'
+);


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.

machour · 2018-03-25T15:50:41Z

src/api/client.js

+        nextPageUrl: this.getNextPageUrl(response),
+        schema: Schemas.REPO_ARRAY,
+      }));
+    },


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.

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?

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

machour · 2018-03-25T15:51:38Z

src/api/reducers/pagination.js

@@ -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
+  ),


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

machour · 2018-03-25T15:52:35Z

src/api/schemas/repos.js

+  {
+    idAttribute: repo => repo.full_name,
+  }
+);


This is where we define our Repository schema for Normalizr.

For now, we simply tell it to use the repository full name as an identifier.

Above is everything that is required to get going. All the logic will happen under the hood:

When client.activity.getStarredReposForUser('machour') is called, the dispatch proxy does this:

Analyzes the call and extract the following information:

namespace: activity

method: getStarredReposForUser

parameters: ['machour']

Automatically guess the action set to be used from 'namespace' and 'method' : ACTIVITY_GET_STARRED_REPOS_FOR_USER !

Checks if this is a paginated call, by checking if STORE.pagination.ACTIVITY_GET_STARRED_REPOS_FOR_USER exists. It does since we added it to the paginate() reducer.

Checks if we already have data inside STORE.pagination.ACTIVITY_GET_STARRED_REPOS_FOR_USER['machour']

'machour' is a '-' separated string of our parameters.

So client.namespace.method("foo", "bar", 42) would have checked inside:
STORE.pagination.NAMESPACE_METHOD['foo-bar-42']

No data, "ACTIVITY_GET_STARRED_REPOS_FOR_USER.pending" is dispatched, the API call is performed, and the returned json is treated as an array of repositories (as we said in the schema key)

After "success" is dispatched, we end up with this in our store:

Ah this makes so much sense, and it makes things so so clean by abstracting it all away 👏 👏

machour · 2018-03-25T16:25:02Z

src/user/screens/starred-repository-list.screen.js

+    starredRepos,
+    userId,
+  };
+};


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.

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

and pageCount and ids.

It's literally this in our case:

machour · 2018-03-25T16:25:26Z

src/user/screens/starred-repository-list.screen.js

-  bindActionCreators({ getStarredRepositories }, dispatch);
+const mapDispatchToProps = {
+  getStarredReposForUser: RestClient.activity.getStarredReposForUser,
+};


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

machour · 2018-03-25T16:27:56Z

src/user/screens/starred-repository-list.screen.js

+              this.props.getStarredReposForUser(userId, {
+                forceRefresh: true,
+              })
+            }


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

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

machour · 2018-03-25T16:29:42Z

src/user/screens/starred-repository-list.screen.js

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


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.

machour · 2018-03-25T16:30:25Z

src/user/user.action.js

@@ -12,7 +12,6 @@ import {
  GET_IS_FOLLOWING,
  GET_IS_FOLLOWER,
  GET_REPOSITORIES,
-  GET_STARRED_REPOSITORIES,


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

housseindjirdeh

WOW this is so good. So so good.

The number of screens we have that literally follow the exact same logic and can be simplified and we cana easily have paginated logic throughout the app :)

Thanks a million. I've left a few comments just asking questions but feel free to merge this right in.

To fully digest how to use it, I'm going to incorporate this logic to a separate screen and use this PR as reference 👏

housseindjirdeh · 2018-03-25T23:41:35Z

src/api/client.js

+        nextPageUrl: this.getNextPageUrl(response),
+        schema: Schemas.REPO_ARRAY,
+      }));
+    },


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?

housseindjirdeh · 2018-03-25T23:42:49Z

src/api/schemas/repos.js

+  {
+    idAttribute: repo => repo.full_name,
+  }
+);


Ah this makes so much sense, and it makes things so so clean by abstracting it all away 👏 👏

housseindjirdeh · 2018-03-25T23:44:40Z

src/user/screens/starred-repository-list.screen.js

+              this.props.getStarredReposForUser(userId, {
+                forceRefresh: true,
+              })
+            }


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

housseindjirdeh · 2018-03-25T23:46:30Z

src/user/screens/starred-repository-list.screen.js

+    starredRepos,
+    userId,
+  };
+};


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

machour · 2018-03-26T10:35:15Z

Made a tiny change: the repo id (full name) is now stored in lower case.

This make things more consistent internally and will save us a headache if for some reason, one endpoint respond with a lowercased name.

machour · 2018-03-26T10:36:26Z

Sadely, last change made an important comment outdated, reposting it:

Here's all the logic that will happen under the hood:

When client.activity.getStarredReposForUser('machour') is called, the dispatch proxy does this:

Analyzes the call and extract the following information:

namespace: activity
method: getStarredReposForUser
parameters: ['machour']

Automatically guess the action set to be used from 'namespace' and 'method' : ACTIVITY_GET_STARRED_REPOS_FOR_USER !
Checks if this is a paginated call, by checking if STORE.pagination.ACTIVITY_GET_STARRED_REPOS_FOR_USER exists. It does since we added it to the paginate() reducer.
Checks if we already have data inside STORE.pagination.ACTIVITY_GET_STARRED_REPOS_FOR_USER['machour']

'machour' is a '-' separated string of our parameters.

So client.namespace.method("foo", "bar", 42) would have checked inside:
STORE.pagination.NAMESPACE_METHOD['foo-bar-42']

No data, "ACTIVITY_GET_STARRED_REPOS_FOR_USER.pending" is dispatched, the API call is performed, and the returned json is treated as an array of repositories (as we said in the schema key)
After "success" is dispatched, we end up with this in our store:

machour added 4 commits March 25, 2018 16:26

feat: Make the starred repositories list paged using the new api

fdabf02

Revert "feat: Make the starred repositories list paged using the new …

0e4f213

…api" This reverts commit fdabf02.

feat: Make the starred repositories list paged using the new api

60ca6ba

fix: remove leftovers

e66e4a9

machour commented Mar 25, 2018

View reviewed changes

housseindjirdeh approved these changes Mar 25, 2018

View reviewed changes

fix: make repo id always lowercase

4a98a8b

machour mentioned this pull request Mar 26, 2018

Rest: Migrate paginated calls to the new API #751

Open

13 tasks

machour merged commit 22add17 into gitpoint:master Mar 26, 2018

machour deleted the feat/paged-starred-repos branch March 31, 2018 11:02

feat: Make the starred repositories list paged #750

feat: Make the starred repositories list paged #750

Uh oh!

Conversation

machour commented Mar 25, 2018

Description

Uh oh!

coveralls commented Mar 25, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

machour left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

machour Mar 25, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

machour Mar 26, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

machour Mar 25, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

housseindjirdeh left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

machour commented Mar 26, 2018

Uh oh!

machour commented Mar 26, 2018

Uh oh!

Uh oh!

coveralls commented Mar 25, 2018 •

edited

Loading

machour Mar 25, 2018 •

edited

Loading

machour Mar 26, 2018 •

edited

Loading

machour Mar 25, 2018 •

edited

Loading