-
Notifications
You must be signed in to change notification settings - Fork 609
Let the view know about the review status of the PR #1833
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
Conversation
The PR model should send the review status of the PR to the view.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This PR is looking good! If the merge conflicts are resolved I will take another look.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Accidentally hit approve.
@alexr00 Thanks for reviewing it. I will resolve the conflicts. I hope I can get back quickly, it has been a year since I have worked on VSCPR |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for the updates! Just one more question.
webviews/common/context.tsx
Outdated
@@ -158,6 +158,7 @@ export class PRContext { | |||
}); | |||
state.reviewers = reviewers; | |||
state.events = [...state.events.filter(e => (isReviewEvent(e) ? e.state !== 'PENDING' : e)), review]; | |||
state.reviewState = review.state; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is a check needed here to make sure that the review is from the current user?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@alexr00 I did not see a need to check if the review is from the current user. As this refers to the review state of the PR for the current user. It is required to enable/disable the Approve button on the overview of the PR.
When the PR panel is shown it always refers to the review state from the current user for the PR based on the return from pullRequestOverview.ts#L153
When the reviewer makes changes to the PR review
always refers to the current user's review on the PR.
Please let me know if I am missing anything here, I am happy to make changes/updates
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good, thank you!
Fixes #473
The PR model must find the current user's review status for the PR and send it to the view.
Let me know if it requires any further changes.