Skip to content
This repository was archived by the owner on Dec 15, 2022. It is now read-only.

Commit 3d6eaff

Browse files
committed
Call a single render prop with review and comment thread collections
1 parent da9c239 commit 3d6eaff

File tree

3 files changed

+35
-50
lines changed

3 files changed

+35
-50
lines changed

lib/controllers/pr-reviews-controller.js

Lines changed: 3 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -61,6 +61,8 @@ export default class PullRequestReviewsController extends React.Component {
6161
return null;
6262
}
6363

64+
const reviews = this.props.pullRequest.reviews.edges.map(edge => edge.node);
65+
6466
const commentThreads = Object.keys(this.state).reverse().map(rootCommentId => {
6567
return {
6668
rootCommentId,
@@ -80,13 +82,7 @@ export default class PullRequestReviewsController extends React.Component {
8082
return (
8183
<Fragment>
8284
{this.renderCommentFetchingContainers()}
83-
{commentThreads.map(commentThread => {
84-
return (
85-
<Fragment key={`pr-comment-${commentThread.rootCommentID}`}>
86-
{this.props.children(commentThread)}
87-
</Fragment>
88-
);
89-
})}
85+
{this.props.children({reviews, commentThreads})}
9086
</Fragment>
9187
);
9288
}

lib/views/multi-file-patch-view.js

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -386,15 +386,16 @@ export default class MultiFilePatchView extends React.Component {
386386
// "forceRerender" ensures that the PullRequestCommentsView re-renders each time that the MultiFilePatchView does.
387387
// It doesn't re-query for reviews, but it does re-check patch visibility.
388388
return (
389-
<PullRequestReviewsContainer pullRequest={this.props.pullRequest} forceRerender={{}}>
390-
{commentThread => (
389+
<PullRequestReviewsContainer forceRerender={{}}>
390+
{({commentThreads}) => commentThreads.map(commentThread => (
391391
<PullRequestReviewCommentThreadView
392+
key={`pr-comment-${commentThread.rootCommentID}`}
392393
commentThread={commentThread}
393394
getBufferRowForDiffPosition={this.props.multiFilePatch.getBufferRowForDiffPosition}
394395
isPatchVisible={this.props.multiFilePatch.isPatchVisible}
395396
switchToIssueish={this.props.switchToIssueish}
396397
/>
397-
)}
398+
))}
398399
</PullRequestReviewsContainer>
399400
);
400401
} else {

test/controllers/pr-reviews-controller.test.js

Lines changed: 28 additions & 40 deletions
Original file line numberDiff line numberDiff line change
@@ -1,19 +1,14 @@
11
import React from 'react';
22
import {shallow} from 'enzyme';
33
import {reviewBuilder} from '../builder/pr';
4-
import {inspect} from 'util';
54

65
import PullRequestReviewsController from '../../lib/controllers/pr-reviews-controller';
76
import PullRequestReviewCommentsContainer from '../../lib/containers/pr-review-comments-container';
87

98
import {PAGE_SIZE, PAGINATION_WAIT_TIME_MS} from '../../lib/helpers';
109

11-
function SomeView(props) {
12-
return <pre>{inspect(props, {depth: 3})}</pre>;
13-
}
14-
1510
describe('PullRequestReviewsController', function() {
16-
function buildApp(opts, overrideProps = {}, childFn = SomeView) {
11+
function buildApp(opts, overrideProps = {}) {
1712
const o = {
1813
relayHasMore: () => false,
1914
relayLoadMore: () => {},
@@ -47,15 +42,13 @@ describe('PullRequestReviewsController', function() {
4742
isLoading: o.relayIsLoading,
4843
},
4944
pullRequest: {reviews},
45+
children: () => {},
5046
...overrideProps,
5147
};
5248

53-
return (
54-
<PullRequestReviewsController {...props}>
55-
{childFn}
56-
</PullRequestReviewsController>
57-
);
49+
return <PullRequestReviewsController {...props} />;
5850
}
51+
5952
it('returns null if props.pullRequest is falsy', function() {
6053
const wrapper = shallow(buildApp({}, {pullRequest: null}));
6154
assert.isNull(wrapper.getElement());
@@ -79,7 +72,7 @@ describe('PullRequestReviewsController', function() {
7972
assert.strictEqual(containers.at(1).prop('review').id, review2.id);
8073
});
8174

82-
it('calls the child render prop for each comment thread', function() {
75+
it('calls the render prop with all reviews and all comment threads', function() {
8376
const review1 = reviewBuilder()
8477
.id(10)
8578
.submittedAt('2019-01-01T10:00:00Z')
@@ -100,11 +93,19 @@ describe('PullRequestReviewsController', function() {
10093
const reviewSpecs = [review1, review2];
10194
const passThroughProp = 'I only exist for the children';
10295

103-
const wrapper = shallow(buildApp({reviewSpecs}, {}, thread => {
104-
return <ChildComponent thread={thread} passThroughProp={passThroughProp} />;
96+
const wrapper = shallow(buildApp({reviewSpecs}, {
97+
children: ({reviews, commentThreads}) => (
98+
<ChildComponent
99+
reviews={reviews}
100+
threads={commentThreads}
101+
passThroughProp={passThroughProp}
102+
/>
103+
),
105104
}));
106105

107-
assert.isFalse(wrapper.exists('ChildComponent'));
106+
assert.strictEqual(wrapper.find('ChildComponent').prop('passThroughProp'), passThroughProp);
107+
assert.deepEqual(wrapper.find('ChildComponent').prop('reviews').map(review => review.id), [10, 20]);
108+
assert.lengthOf(wrapper.find('ChildComponent').prop('threads'), 0);
108109

109110
wrapper.find(PullRequestReviewCommentsContainer).at(0).prop('collectComments')({
110111
reviewId: review1.id,
@@ -113,12 +114,11 @@ describe('PullRequestReviewsController', function() {
113114
fetchingMoreComments: false,
114115
});
115116

116-
assert.lengthOf(wrapper.find('ChildComponent'), 1);
117-
assert.strictEqual(wrapper.find('ChildComponent').at(0).prop('passThroughProp'), passThroughProp);
118-
assert.deepEqual(wrapper.find('ChildComponent').at(0).prop('thread'), {
119-
rootCommentId: '1',
120-
comments: [review1.comments.edges[0].node],
121-
});
117+
assert.strictEqual(wrapper.find('ChildComponent').prop('passThroughProp'), passThroughProp);
118+
assert.deepEqual(wrapper.find('ChildComponent').prop('reviews').map(review => review.id), [10, 20]);
119+
assert.deepEqual(wrapper.find('ChildComponent').prop('threads'), [
120+
{rootCommentId: '1', comments: [review1.comments.edges[0].node]},
121+
]);
122122

123123
wrapper.find(PullRequestReviewCommentsContainer).at(1).prop('collectComments')({
124124
reviewId: review2.id,
@@ -127,25 +127,13 @@ describe('PullRequestReviewsController', function() {
127127
fetchingMoreComments: false,
128128
});
129129

130-
assert.lengthOf(wrapper.find('ChildComponent'), 3);
131-
132-
assert.strictEqual(wrapper.find('ChildComponent').at(0).prop('passThroughProp'), passThroughProp);
133-
assert.deepEqual(wrapper.find('ChildComponent').at(0).prop('thread'), {
134-
rootCommentId: '4',
135-
comments: [review2.comments.edges[2].node],
136-
});
137-
138-
assert.strictEqual(wrapper.find('ChildComponent').at(1).prop('passThroughProp'), passThroughProp);
139-
assert.deepEqual(wrapper.find('ChildComponent').at(1).prop('thread'), {
140-
rootCommentId: '2',
141-
comments: [review2.comments.edges[0].node],
142-
});
143-
144-
assert.strictEqual(wrapper.find('ChildComponent').at(2).prop('passThroughProp'), passThroughProp);
145-
assert.deepEqual(wrapper.find('ChildComponent').at(2).prop('thread'), {
146-
rootCommentId: '1',
147-
comments: [review1.comments.edges[0].node, review2.comments.edges[1].node],
148-
});
130+
assert.strictEqual(wrapper.find('ChildComponent').prop('passThroughProp'), passThroughProp);
131+
assert.deepEqual(wrapper.find('ChildComponent').prop('reviews').map(review => review.id), [10, 20]);
132+
assert.deepEqual(wrapper.find('ChildComponent').prop('threads'), [
133+
{rootCommentId: '4', comments: [review2.comments.edges[2].node]},
134+
{rootCommentId: '2', comments: [review2.comments.edges[0].node]},
135+
{rootCommentId: '1', comments: [review1.comments.edges[0].node, review2.comments.edges[1].node]},
136+
]);
149137
});
150138

151139
describe('collectComments', function() {

0 commit comments

Comments
 (0)