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

CB-410: Revisions of reviews are created when drafts are made, and they remain visible #427

Open
wants to merge 5 commits into
base: master
Choose a base branch
from

Conversation

anshg1214
Copy link
Member

@anshg1214 anshg1214 commented May 21, 2022

CB-410
This PR:

  1. Deletes the older revisions of the review when the user publishes it.
  2. Adds option in the endpoint to publish the draft reviews.
  3. Fixes a bug which updates the average ratings table with the ratings even when the review is a draft.

We need to run 2 queries in our database

  1. To delete all older revisions of the review which was later published.
  2. To delete the avg_ratings for the draft reviews.

@amCap1712
Copy link
Member

At a quick look, these changes need to be made for the api as well?

@@ -374,3 +375,32 @@ def test_hide_redirect(self):
review["entity_type"], review["entity_id"]))
redirect_url = urlparse(response.location)
self.assertEqual(redirect_url.path, url_for('review.entity', id=review['id']))


def test_publish_draft_review(self):
Copy link
Member

Choose a reason for hiding this comment

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

Since we are deleting data, this test should be more comprehensive IMO. Probably fetch the actual revisions before publish, assert, then fetch again after publish to confirm the correct revisions were deleted.

Copy link
Member Author

Choose a reason for hiding this comment

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

Sounds Good! I've improved the tests.

@anshg1214 anshg1214 requested a review from amCap1712 October 3, 2022 07:59
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