Skip to content

feat(issue): paginate issue comments and events #848

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 12 commits into from
Nov 18, 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
4 changes: 4 additions & 0 deletions src/api/actions/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -21,3 +21,7 @@ export const REPOS_GET_CONTRIBUTORS = createPaginationActionSet(
'REPOS_GET_CONTRIBUTORS'
);
export const REPOS_FORK = createActionSet('REPOS_FORK');
export const REPOS_GET_ISSUE = createActionSet('REPOS_GET_ISSUE');
export const REPOS_GET_ISSUE_TIMELINE = createPaginationActionSet(
'REPOS_GET_ISSUE_TIMELINE'
);
24 changes: 24 additions & 0 deletions src/api/client.js
Original file line number Diff line number Diff line change
Expand Up @@ -73,6 +73,7 @@ export class Client {
HTML: 'application/vnd.github.v3.html+json',
JSON: 'application/vnd.github.v3+json',
MERCY_PREVIEW: 'application/vnd.github.mercy-preview+json',
MOCKINGBIRD_PREVIEW: 'application/vnd.github.mockingbird-preview+json',
Copy link
Member

Choose a reason for hiding this comment

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

we may want to somehow keep track of when these preview APIs end up changing

Copy link
Member

Choose a reason for hiding this comment

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

Looks like this one opened in 2016, wow lol

https://developer.github.com/changes/2016-05-23-timeline-preview-api/

But still something to keep in mind 🤔

Copy link
Member Author

Choose a reason for hiding this comment

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

The other preview API, MERCY_PREVIEW, which was for fetching topics of repository, was replaced by GraphQL. If no so-called preview versions in GraphQL, we can also switch the timeline API.

RAW: 'application/vnd.github.v3.raw+json',
};

Expand Down Expand Up @@ -394,6 +395,29 @@ export class Client {
params,
schema: Schemas.REPO,
}),
getIssue: (repoId: string, issueId: number, params: SpecialParameters = {}) =>
this.get({
endpoint: `repos/${repoId}/issues/${issueId}`,
params,
fetchParameters: {
headers: {
Accept: this.Accept.FULL,
},
},
schema: Schemas.ISSUE,
}),
getIssueTimeline: (repoId: string, issueId: number, params: SpecialParameters = {}) =>
this.list({
endpoint: `repos/${repoId}/issues/${issueId}/timeline`,
params,
fetchParameters: {
headers: {
Accept: [this.Accept.MOCKINGBIRD_PREVIEW, this.Accept.FULL].join(','),
},
},
schema: [Schemas.ISSUE_TIMELINE_ITEM],
paginationArgs: [`${repoId}/${issueId}`],
}),
};
orgs = {
/**
Expand Down
1 change: 1 addition & 0 deletions src/api/queries/repo.query.js
Original file line number Diff line number Diff line change
Expand Up @@ -109,6 +109,7 @@ query ($owner: String!, $name: String!) {
...PullRequestsList
}
}
viewerPermission
}
}

Expand Down
2 changes: 2 additions & 0 deletions src/api/reducers/entities.js
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,8 @@ export const entities = (
events: {},
orgs: {},
repos: {},
issues: {},
issueTimelineItems: {},
users: {},
gqlRepos: {},
},
Expand Down
1 change: 1 addition & 0 deletions src/api/reducers/pagination.js
Original file line number Diff line number Diff line change
Expand Up @@ -85,4 +85,5 @@ export const pagination = combineReducers({
SEARCH_USERS: paginate(Actions.SEARCH_USERS),
ORGS_GET_MEMBERS: paginate(Actions.ORGS_GET_MEMBERS),
REPOS_GET_CONTRIBUTORS: paginate(Actions.REPOS_GET_CONTRIBUTORS),
REPOS_GET_ISSUE_TIMELINE: paginate(Actions.REPOS_GET_ISSUE_TIMELINE),
});
5 changes: 5 additions & 0 deletions src/api/schemas/gql-repos.js
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,11 @@ export const gqlRepoSchema = new schema.Entity(
},
processStrategy: ({ repository }) => ({
...repository,
permissions: {
admin: repository.viewerPermission === 'ADMIN',
push: repository.viewerPermission === 'ADMIN' || repository.viewerPermission === 'WRITE',
},
full_name: repository.nameWithOwner,
webUrl: `https://github.com/${repository.nameWithOwner}`,
}),
}
Expand Down
3 changes: 3 additions & 0 deletions src/api/schemas/index.js
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
import { eventSchema } from './events';
import { repoSchema } from './repos';
import { issueSchema, issueTimelineItemSchema } from './issues';
import { orgSchema } from './orgs';
import { userSchema } from './users';
import { gqlRepoSchema } from './gql-repos';
Expand All @@ -9,6 +10,8 @@ export default {
EVENT_ARRAY: [eventSchema],
REPO: repoSchema,
REPO_ARRAY: [repoSchema],
ISSUE: issueSchema,
ISSUE_TIMELINE_ITEM: issueTimelineItemSchema,
USER: userSchema,
USER_ARRAY: [userSchema],
ORG: orgSchema,
Expand Down
12 changes: 12 additions & 0 deletions src/api/schemas/issues.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
import { schema } from 'normalizr';

export const issueSchema = new schema.Entity(
'issues',
{},
{
idAttribute: issue => issue.url,
}
);

// TODO: some events have no id field, i.e. cross-referenced
export const issueTimelineItemSchema = new schema.Entity('issueTimelineItems');
4 changes: 4 additions & 0 deletions src/components/issue-description.component.js
Original file line number Diff line number Diff line change
Expand Up @@ -83,6 +83,7 @@ const MergeButtonContainer = styled.View`
export class IssueDescription extends Component {
props: {
issue: Object,
repository: Object,
diff: string,
isMergeable: boolean,
isMerged: boolean,
Expand All @@ -104,6 +105,7 @@ export class IssueDescription extends Component {
const {
diff,
issue,
repository,
isMergeable,
isMerged,
isPendingDiff,
Expand Down Expand Up @@ -219,6 +221,8 @@ export class IssueDescription extends Component {
onPress={() =>
navigation.navigate('PullMerge', {
title: t('Merge Pull Request', locale),
issue,
repository,
})
}
title={t('Merge Pull Request', locale)}
Expand Down
2 changes: 1 addition & 1 deletion src/components/issue-event-list-item.component.js
Original file line number Diff line number Diff line change
Expand Up @@ -212,7 +212,7 @@ export class IssueEventListItem extends Component {
iconName="person"
text={
<Text>
<ActorLink actor={event.assigner} onPress={this.onPressUser} />{' '}
<ActorLink actor={event.assigner || event.actor} onPress={this.onPressUser} />{' '}
{event.event}{' '}
<ActorLink actor={event.assignee} onPress={this.onPressUser} />
</Text>
Expand Down
1 change: 1 addition & 0 deletions src/components/loading-indicators/index.js
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
export * from './loading-common-item.component';
export * from './loading-container.component';
export * from './loading-event-list-item.component';
export * from './loading-members-list.component';
Expand Down
18 changes: 18 additions & 0 deletions src/components/loading-indicators/loading-common-item.component.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,18 @@
import React from 'react';
import { ActivityIndicator } from 'react-native';
import styled from 'styled-components';

import { colors } from 'config';

const Container = styled.View`
height: 44;
background-color: ${colors.white};
align-items: center;
justify-content: center;
`;

export const LoadingCommonItem = () => (
<Container>
<ActivityIndicator animating size="small" />
</Container>
);
2 changes: 1 addition & 1 deletion src/issue/issue.action.js
Original file line number Diff line number Diff line change
Expand Up @@ -95,7 +95,7 @@ const getPullRequest = url => {
};
};

const getPullRequestDetails = issue => {
export const getPullRequestDetails = issue => {
return (dispatch, getState) => {
const repoFullName = getState().repository.repository.full_name;

Expand Down
8 changes: 2 additions & 6 deletions src/issue/screens/edit-issue-comment.screen.js
Original file line number Diff line number Diff line change
Expand Up @@ -41,8 +41,6 @@ const styles = StyleSheet.create({

const mapStateToProps = state => ({
locale: state.auth.locale,
issue: state.issue.issue,
repository: state.repository.repository,
isEditingComment: state.issue.isEditingComment,
});

Expand All @@ -60,9 +58,7 @@ class EditIssueComment extends Component {
editIssueBody: Function,
editIssueComment: Function,
locale: string,
repository: Object,
navigation: Object,
issue: Object,
isEditingComment: boolean,
};

Expand All @@ -81,8 +77,8 @@ class EditIssueComment extends Component {
}

editComment = () => {
const { issue, navigation } = this.props;
const { repository, comment } = this.props.navigation.state.params;
const { navigation } = this.props;
const { issue, repository, comment } = this.props.navigation.state.params;

const repoName = repository.name;
const owner = repository.owner.login;
Expand Down
25 changes: 14 additions & 11 deletions src/issue/screens/issue-settings.screen.js
Original file line number Diff line number Diff line change
Expand Up @@ -13,15 +13,14 @@ import {
} from 'components';
import { emojifyText, t, openURLInView } from 'utils';
import { colors, fonts } from 'config';
import { v3 } from 'api';
import { getLabels } from 'repository';
import { editIssue, changeIssueLockStatus } from '../issue.action';

const mapStateToProps = state => ({
locale: state.auth.locale,
authUser: state.auth.user,
repository: state.repository.repository,
labels: state.repository.labels,
issue: state.issue.issue,
isMerged: state.issue.isMerged,
isEditingIssue: state.issue.isEditingIssue,
isPendingLabels: state.repository.isPendingLabels,
Expand Down Expand Up @@ -59,9 +58,7 @@ class IssueSettings extends Component {
getLabels: Function,
locale: string,
authUser: Object,
repository: Object,
labels: Array,
issue: Object,
isMerged: boolean,
// isEditingIssue: boolean,
isPendingLabels: boolean,
Expand All @@ -70,7 +67,9 @@ class IssueSettings extends Component {

componentDidMount() {
this.props.getLabels(
this.props.repository.labels_url.replace('{/name}', '')
`${v3.root}/repos/${
this.props.navigation.state.params.repository.full_name
}/labels`
);
}

Expand All @@ -89,7 +88,8 @@ class IssueSettings extends Component {
};

handleIssueActionPress = index => {
const { issue, navigation } = this.props;
const { navigation } = this.props;
const { issue } = navigation.state.params;
const newState = issue.state === 'open' ? 'close' : 'open';

if (index === 0) {
Expand All @@ -100,7 +100,7 @@ class IssueSettings extends Component {
};

handleLockIssueActionPress = index => {
const { issue, repository } = this.props;
const { issue, repository } = this.props.navigation.state.params;
const repoName = repository.name;
const owner = repository.owner.login;

Expand All @@ -115,7 +115,8 @@ class IssueSettings extends Component {
};

handleAddLabelActionPress = index => {
const { issue, labels } = this.props;
const { navigation, labels } = this.props;
const { issue } = navigation.state.params;
const labelChoices = [...labels.map(label => label.name)];

if (
Expand All @@ -135,7 +136,7 @@ class IssueSettings extends Component {
};

editIssue = (editParams, stateChangeParams) => {
const { issue, repository } = this.props;
const { issue, repository } = this.props.navigation.state.params;
const repoName = repository.name;
const owner = repository.owner.login;
const updateStateParams = stateChangeParams || editParams;
Expand All @@ -149,10 +150,12 @@ class IssueSettings extends Component {
);
};

openURLInBrowser = () => openURLInView(this.props.issue.html_url);
openURLInBrowser = () =>
openURLInView(this.props.navigation.state.params.issue.html_url);

render() {
const { issue, isMerged, locale, authUser, navigation } = this.props;
const { isMerged, locale, authUser, navigation } = this.props;
const { issue } = navigation.state.params;
const issueType = issue.pull_request
? t('Pull Request', locale)
: t('Issue', locale);
Expand Down
Loading