Skip to content
This repository was archived by the owner on Dec 15, 2022. It is now read-only.
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
35 commits
Select commit Hold shift + click to select a range
a261186
Deautobind ErrorView
smashwilson May 7, 2019
32ca80c
Render an "open dev tools" button in <ErrorView>
smashwilson May 7, 2019
c1bf750
Flag network errors
smashwilson May 7, 2019
d65d2f0
Special-case network errors in QueryErrorView
smashwilson May 7, 2019
fdfaedb
Deautobind GitHubTabContainer
smashwilson May 7, 2019
7e374e1
Deautobind RemoteContainer
smashwilson May 7, 2019
37ea727
Deautobind GitHubTabController
smashwilson May 7, 2019
599dde1
Drill openDevTools everywhere
smashwilson May 7, 2019
8d06698
Rearrange ErrorView buttons
smashwilson May 7, 2019
a96dfd0
Missed a drilling spot
smashwilson May 7, 2019
8df7ed4
Return a distinct OFFLINE case when the fetch fails
smashwilson May 8, 2019
993b78a
OfflineView to show when we're... offline
smashwilson May 8, 2019
3139f03
Handle the Offline case in IssueishDetailContainer
smashwilson May 8, 2019
9cf844c
No-op the OFFLINE case in CommentDecorationsContainer
smashwilson May 8, 2019
9a6fd11
Handle the Offline case in RemoteContainer
smashwilson May 8, 2019
069ec5e
Handle the Offline case in ReviewsContainer
smashwilson May 8, 2019
46fa12d
Handle the Offline case in UserStore
smashwilson May 8, 2019
743b5a9
Trigger a login model update when the network availability changes
smashwilson May 8, 2019
e53f3de
Use the OfflineView for network errors in QueryErrorView
smashwilson May 8, 2019
244deb2
Revert "Drill openDevTools everywhere"
smashwilson May 8, 2019
8d7d51f
Revert "Missed a drilling spot"
smashwilson May 8, 2019
fe7fe1a
Go home `git revert` you're drunk
smashwilson May 8, 2019
87e8e8f
Return a full Error instead of a special OFFLINE token
smashwilson May 8, 2019
154cbca
Okay we actually do want to throw those
smashwilson May 8, 2019
1f5c9e7
Special-case network errors in the QueryErrorTile
smashwilson May 9, 2019
1ad5547
Rename "reportMutationErrors" to "reportRelayError" globally :see_no_…
smashwilson May 9, 2019
c37dd7a
Notify on refetch errors throughout the package
smashwilson May 9, 2019
d905027
Missed a prop
smashwilson May 9, 2019
8b7ccbe
Custom handling of network errors in the Relay error callback
smashwilson May 9, 2019
d29884d
:fire: unused openDevTools prop
smashwilson May 13, 2019
8410a12
Merge branch 'master' of github.com:atom/github into aw/die-better-wh…
smashwilson May 14, 2019
8450081
ErrorView test coverage :100:
smashwilson May 14, 2019
2fcb19b
QueryErrorView test coverage :100:
smashwilson May 14, 2019
1cffb72
Okay just skip that whole describe()
smashwilson May 14, 2019
8b47553
Test RootController::reportRelayError()
smashwilson May 14, 2019
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
11 changes: 9 additions & 2 deletions lib/containers/aggregated-reviews-container.js
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,9 @@ export class BareAggregatedReviewsContainer extends React.Component {

// only fetch summaries when we specify a summariesRenderer
summariesRenderer: PropTypes.func,

// Report errors during refetch
reportRelayError: PropTypes.func.isRequired,
}

constructor(props) {
Expand Down Expand Up @@ -76,8 +79,12 @@ export class BareAggregatedReviewsContainer extends React.Component {
commentCursor: null,
},
null,
() => {
this.emitter.emit('did-refetch');
err => {
if (err) {
this.props.reportRelayError('Unable to refresh reviews', err);
} else {
this.emitter.emit('did-refetch');
}
callback();
},
{force: true},
Expand Down
8 changes: 6 additions & 2 deletions lib/containers/comment-decorations-container.js
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,8 @@ export default class CommentDecorationsContainer extends React.Component {
commands: PropTypes.object.isRequired,
localRepository: PropTypes.object.isRequired,
loginModel: GithubLoginModelPropType.isRequired,

reportRelayError: PropTypes.func.isRequired,
};

render() {
Expand All @@ -46,7 +48,7 @@ export default class CommentDecorationsContainer extends React.Component {
}

renderWithToken(token, {repoData}) {
if (!token || token === UNAUTHENTICATED || token === INSUFFICIENT) {
if (!token || token === UNAUTHENTICATED || token === INSUFFICIENT || token instanceof Error) {
// we're not going to prompt users to log in to render decorations for comments
// just let it go and move on with our lives.
return null;
Expand Down Expand Up @@ -177,7 +179,9 @@ export default class CommentDecorationsContainer extends React.Component {
}

return (
<AggregatedReviewsContainer pullRequest={queryProps.currentPullRequest}>
<AggregatedReviewsContainer
pullRequest={queryProps.currentPullRequest}
reportRelayError={this.props.reportRelayError}>
{({errors, summaries, commentThreads}) => {
if (errors && errors.length > 0) {
// eslint-disable-next-line no-console
Expand Down
12 changes: 3 additions & 9 deletions lib/containers/github-tab-container.js
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,6 @@ import PropTypes from 'prop-types';
import yubikiri from 'yubikiri';

import {GithubLoginModelPropType, RefHolderPropType} from '../prop-types';
import {autobind} from '../helpers';
import OperationStateObserver, {PUSH, PULL, FETCH} from '../models/operation-state-observer';
import GitHubTabController from '../controllers/github-tab-controller';
import ObserveModel from '../views/observe-model';
Expand All @@ -18,12 +17,7 @@ export default class GitHubTabContainer extends React.Component {
rootHolder: RefHolderPropType.isRequired,
}

constructor(props) {
super(props);
autobind(this, 'fetchRepositoryData', 'renderRepositoryData');

this.state = {};
}
state = {};

static getDerivedStateFromProps(props, state) {
if (props.repository !== state.lastRepository) {
Expand All @@ -36,7 +30,7 @@ export default class GitHubTabContainer extends React.Component {
return null;
}

fetchRepositoryData(repository) {
fetchRepositoryData = repository => {
return yubikiri({
workingDirectory: repository.getWorkingDirectoryPath(),
allRemotes: repository.getRemotes(),
Expand All @@ -59,7 +53,7 @@ export default class GitHubTabContainer extends React.Component {
);
}

renderRepositoryData(data) {
renderRepositoryData = data => {
if (!data || this.props.repository.isLoading()) {
return (
<GitHubTabController
Expand Down
21 changes: 18 additions & 3 deletions lib/containers/issueish-detail-container.js
Original file line number Diff line number Diff line change
Expand Up @@ -48,7 +48,7 @@ export default class IssueishDetailContainer extends React.Component {
switchToIssueish: PropTypes.func.isRequired,
onTitleChange: PropTypes.func.isRequired,
destroy: PropTypes.func.isRequired,
reportMutationErrors: PropTypes.func.isRequired,
reportRelayError: PropTypes.func.isRequired,

// Item context
itemType: ItemTypePropType.isRequired,
Expand All @@ -66,6 +66,17 @@ export default class IssueishDetailContainer extends React.Component {
renderWithToken = tokenData => {
const token = tokenData && tokenData.token;

if (token instanceof Error) {
return (
<QueryErrorView
error={token}
login={this.handleLogin}
retry={this.handleTokenRetry}
logout={this.handleLogout}
/>
);
}

if (token === UNAUTHENTICATED) {
return <GithubLoginView onLogin={this.handleLogin} />;
}
Expand Down Expand Up @@ -193,7 +204,9 @@ export default class IssueishDetailContainer extends React.Component {

if (props.repository.issueish.__typename === 'PullRequest') {
return (
<AggregatedReviewsContainer pullRequest={props.repository.issueish}>
<AggregatedReviewsContainer
pullRequest={props.repository.issueish}
reportRelayError={this.props.reportRelayError}>
{aggregatedReviews => this.renderWithCommentResult(token, repoData, {props, retry}, aggregatedReviews)}
</AggregatedReviewsContainer>
);
Expand Down Expand Up @@ -247,7 +260,7 @@ export default class IssueishDetailContainer extends React.Component {
onTabSelected={this.props.onTabSelected}
onOpenFilesTab={this.props.onOpenFilesTab}
endpoint={this.props.endpoint}
reportMutationErrors={this.props.reportMutationErrors}
reportRelayError={this.props.reportRelayError}

workspace={this.props.workspace}
commands={this.props.commands}
Expand Down Expand Up @@ -283,4 +296,6 @@ export default class IssueishDetailContainer extends React.Component {
handleLogin = token => this.props.loginModel.setToken(this.props.endpoint.getLoginAccount(), token);

handleLogout = () => this.props.loginModel.removeToken(this.props.endpoint.getLoginAccount());

handleTokenRetry = () => this.props.loginModel.didUpdate();
}
28 changes: 17 additions & 11 deletions lib/containers/remote-container.js
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,6 @@ import PropTypes from 'prop-types';
import {QueryRenderer, graphql} from 'react-relay';

import {incrementCounter} from '../reporter-proxy';
import {autobind} from '../helpers';
import {
RemotePropType, RemoteSetPropType, BranchSetPropType, OperationStateObserverPropType, EndpointPropType,
} from '../prop-types';
Expand Down Expand Up @@ -35,13 +34,7 @@ export default class RemoteContainer extends React.Component {
onPushBranch: PropTypes.func.isRequired,
}

constructor(props) {
super(props);

autobind(this, 'fetchToken', 'renderWithToken', 'renderWithResult', 'handleLogin', 'handleLogout');
}

fetchToken(loginModel) {
fetchToken = loginModel => {
return loginModel.getToken(this.props.endpoint.getLoginAccount());
}

Expand All @@ -53,11 +46,22 @@ export default class RemoteContainer extends React.Component {
);
}

renderWithToken(token) {
renderWithToken = token => {
if (token === null) {
return <LoadingView />;
}

if (token instanceof Error) {
return (
<QueryErrorView
error={token}
retry={this.handleTokenRetry}
login={this.handleLogin}
logout={this.handleLogout}
/>
);
}

if (token === UNAUTHENTICATED) {
return <GithubLoginView onLogin={this.handleLogin} />;
}
Expand Down Expand Up @@ -137,13 +141,15 @@ export default class RemoteContainer extends React.Component {
);
}

handleLogin(token) {
handleLogin = token => {
incrementCounter('github-login');
this.props.loginModel.setToken(this.props.endpoint.getLoginAccount(), token);
}

handleLogout() {
handleLogout = () => {
incrementCounter('github-logout');
this.props.loginModel.removeToken(this.props.endpoint.getLoginAccount());
}

handleTokenRetry = () => this.props.loginModel.didUpdate();
}
21 changes: 18 additions & 3 deletions lib/containers/reviews-container.js
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,7 @@ export default class ReviewsContainer extends React.Component {
tooltips: PropTypes.object.isRequired,

// Action methods
reportMutationErrors: PropTypes.func.isRequired,
reportRelayError: PropTypes.func.isRequired,
}

render() {
Expand All @@ -58,6 +58,17 @@ export default class ReviewsContainer extends React.Component {
return <LoadingView />;
}

if (token instanceof Error) {
return (
<QueryErrorView
error={token}
retry={this.handleTokenRetry}
login={this.handleLogin}
logout={this.handleLogout}
/>
);
}

if (token === UNAUTHENTICATED) {
return <GithubLoginView onLogin={this.handleLogin} />;
}
Expand Down Expand Up @@ -173,14 +184,16 @@ export default class ReviewsContainer extends React.Component {
}

return (
<AggregatedReviewsContainer pullRequest={props.repository.pullRequest}>
<AggregatedReviewsContainer
pullRequest={props.repository.pullRequest}
reportRelayError={this.props.reportRelayError}>
{({errors, summaries, commentThreads, refetch}) => {
if (errors && errors.length > 0) {
return errors.map((err, i) => (
<ErrorView
key={`error-${i}`}
title="Pagination error"
descriptions={[err.stack || err.toString()]}
descriptions={[err.stack]}
/>
));
}
Expand Down Expand Up @@ -241,4 +254,6 @@ export default class ReviewsContainer extends React.Component {
handleLogin = token => this.props.loginModel.setToken(this.props.endpoint.getLoginAccount(), token);

handleLogout = () => this.props.loginModel.removeToken(this.props.endpoint.getLoginAccount());

handleTokenRetry = () => this.props.loginModel.didUpdate();
}
6 changes: 3 additions & 3 deletions lib/controllers/emoji-reactions-controller.js
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ export class BareEmojiReactionsController extends React.Component {
tooltips: PropTypes.object.isRequired,

// Action methods
reportMutationErrors: PropTypes.func.isRequired,
reportRelayError: PropTypes.func.isRequired,
}

render() {
Expand All @@ -36,15 +36,15 @@ export class BareEmojiReactionsController extends React.Component {
try {
await addReactionMutation(this.props.relay.environment, this.props.reactable.id, content);
} catch (err) {
this.props.reportMutationErrors('Unable to add reaction emoji', err);
this.props.reportRelayError('Unable to add reaction emoji', err);
}
};

removeReaction = async content => {
try {
await removeReactionMutation(this.props.relay.environment, this.props.reactable.id, content);
} catch (err) {
this.props.reportMutationErrors('Unable to remove reaction emoji', err);
this.props.reportRelayError('Unable to remove reaction emoji', err);
}
};
}
Expand Down
10 changes: 2 additions & 8 deletions lib/controllers/github-tab-controller.js
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,6 @@ import PropTypes from 'prop-types';
import {
GithubLoginModelPropType, RefHolderPropType, RemoteSetPropType, BranchSetPropType, OperationStateObserverPropType,
} from '../prop-types';
import {autobind} from '../helpers';
import GitHubTabView from '../views/github-tab-view';

export default class GitHubTabController extends React.Component {
Expand All @@ -24,11 +23,6 @@ export default class GitHubTabController extends React.Component {
isLoading: PropTypes.bool.isRequired,
}

constructor(props) {
super(props);
autobind(this, 'handlePushBranch', 'handleRemoteSelect');
}

render() {
const gitHubRemotes = this.props.allRemotes.filter(remote => remote.isGithubRepo());
const currentBranch = this.props.branches.getHeadBranch();
Expand Down Expand Up @@ -64,14 +58,14 @@ export default class GitHubTabController extends React.Component {
);
}

handlePushBranch(currentBranch, targetRemote) {
handlePushBranch = (currentBranch, targetRemote) => {
return this.props.repository.push(currentBranch.getName(), {
remote: targetRemote,
setUpstream: true,
});
}

handleRemoteSelect(e, remote) {
handleRemoteSelect = (e, remote) => {
e.preventDefault();
return this.props.repository.setConfig('atomGithub.currentRemote', remote.getName());
}
Expand Down
6 changes: 3 additions & 3 deletions lib/controllers/issueish-detail-controller.js
Original file line number Diff line number Diff line change
Expand Up @@ -61,7 +61,7 @@ export class BareIssueishDetailController extends React.Component {
onTitleChange: PropTypes.func.isRequired,
switchToIssueish: PropTypes.func.isRequired,
destroy: PropTypes.func.isRequired,
reportMutationErrors: PropTypes.func.isRequired,
reportRelayError: PropTypes.func.isRequired,

// Item context
itemType: ItemTypePropType.isRequired,
Expand Down Expand Up @@ -147,7 +147,7 @@ export class BareIssueishDetailController extends React.Component {
openReviews={this.openReviews}
switchToIssueish={this.props.switchToIssueish}
destroy={this.props.destroy}
reportMutationErrors={this.props.reportMutationErrors}
reportRelayError={this.props.reportRelayError}

itemType={this.props.itemType}
refEditor={this.props.refEditor}
Expand All @@ -170,7 +170,7 @@ export class BareIssueishDetailController extends React.Component {
issue={repository.issue}
switchToIssueish={this.props.switchToIssueish}
tooltips={this.props.tooltips}
reportMutationErrors={this.props.reportMutationErrors}
reportRelayError={this.props.reportRelayError}
/>
);
}
Expand Down
8 changes: 4 additions & 4 deletions lib/controllers/reviews-controller.js
Original file line number Diff line number Diff line change
Expand Up @@ -66,7 +66,7 @@ export class BareReviewsController extends React.Component {
tooltips: PropTypes.object.isRequired,

// Action methods
reportMutationErrors: PropTypes.func.isRequired,
reportRelayError: PropTypes.func.isRequired,
}

constructor(props) {
Expand Down Expand Up @@ -262,7 +262,7 @@ export class BareReviewsController extends React.Component {
addEvent('resolve-comment-thread', {package: 'github'});
} catch (err) {
this.showThreadID(thread.id);
this.props.reportMutationErrors('Unable to resolve the comment thread', err);
this.props.reportRelayError('Unable to resolve the comment thread', err);
}
}
}
Expand All @@ -278,7 +278,7 @@ export class BareReviewsController extends React.Component {
this.highlightThread(thread.id);
addEvent('unresolve-comment-thread', {package: 'github'});
} catch (err) {
this.props.reportMutationErrors('Unable to unresolve the comment thread', err);
this.props.reportRelayError('Unable to unresolve the comment thread', err);
}
}
}
Expand Down Expand Up @@ -337,7 +337,7 @@ export class BareReviewsController extends React.Component {
}
}

this.props.reportMutationErrors('Unable to submit your comment', error);
this.props.reportRelayError('Unable to submit your comment', error);
} finally {
this.setState({postingToThreadID: null});
}
Expand Down
Loading