Skip to content
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

fix: review request placeholder files #847

Merged
merged 3 commits into from
Jul 20, 2023

Conversation

alexanderleegs
Copy link
Contributor

@alexanderleegs alexanderleegs commented Jul 20, 2023

This PR fixes an issue where compareDiff was not filtering placeholder files. This affected any review requests which involved placeholder files, as our frontend does not handle that specific file type. To fix this, we moved the filter from compareFullReviewRequest to compareDiff, which is called by both relevant endpoints.

Tests

  • create a review request when the only file changes are placeholder files which are hidden
  • FE should block creation of review request
  • created a review request when there are file changes other than placeholder files, but is subsequently removed, leaving only placeholder file changes in the review request
  • FE shows 0 file changes but does not block closing and merging for review request
  • server should not throw any error

@alexanderleegs alexanderleegs requested a review from a team July 20, 2023 03:28
@kishore03109
Copy link
Contributor

@alexanderleegs @caando sorry may I get your help to dot down the edge cases that yall tested already?

@caando
Copy link
Contributor

caando commented Jul 20, 2023

@kishore03109 we tested this 2 cases:

Test: attempt to create a review request when the only file changes are placeholder files which are hidden
Response: FE blocks creation of review request

Test: created a review request when there are file changes other than placeholder files, but is subsequently removed, leaving only placeholder file changes in the review request
Response: FE shows 0 file changes but does not block closing and merging for review request

Neither cases led to a server error

@kishore03109 kishore03109 merged commit e67e5c2 into develop Jul 20, 2023
@mergify mergify bot deleted the fix/review-request-placeholder-files branch July 20, 2023 05:25
@kishore03109 kishore03109 mentioned this pull request Jul 20, 2023
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.

3 participants