Skip to content

Conversation

@RishabhKothaari
Copy link
Contributor

@RishabhKothaari RishabhKothaari commented May 26, 2020

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.

The PR model should send the review status of the PR to the view.
@RMacfarlane RMacfarlane self-assigned this May 26, 2020
Base automatically changed from master to main February 17, 2021 21:41
@kieferrm kieferrm assigned alexr00 and unassigned RMacfarlane Jun 7, 2021
@alexr00 alexr00 added this to the June 2021 milestone Jun 7, 2021
@alexr00 alexr00 modified the milestones: June 2021, July 2021 Jun 29, 2021
Copy link
Member

@alexr00 alexr00 left a 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.

@alexr00 alexr00 removed this from the July 2021 milestone Jul 16, 2021
Copy link
Member

@alexr00 alexr00 left a comment

Choose a reason for hiding this comment

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

Accidentally hit approve.

@RishabhKothaari
Copy link
Contributor Author

RishabhKothaari commented Jul 22, 2021

@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

@RishabhKothaari RishabhKothaari requested a review from alexr00 July 22, 2021 01:56
@alexr00 alexr00 added this to the July 2021 milestone Jul 22, 2021
Copy link
Member

@alexr00 alexr00 left a 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.

});
state.reviewers = reviewers;
state.events = [...state.events.filter(e => (isReviewEvent(e) ? e.state !== 'PENDING' : e)), review];
state.reviewState = review.state;
Copy link
Member

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?

Copy link
Contributor Author

@RishabhKothaari RishabhKothaari Jul 30, 2021

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

@alexr00 alexr00 modified the milestones: July 2021, August 2021 Jul 28, 2021
@RishabhKothaari RishabhKothaari requested a review from alexr00 July 30, 2021 18:46
Copy link
Member

@alexr00 alexr00 left a 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!

@alexr00 alexr00 merged commit 7fbc64c into microsoft:main Aug 12, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Approve button should be disabled when PR is alreay approved

3 participants