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

Move ratings and reviews when switching editions #3117

Conversation

mattlehrer
Copy link
Contributor

Fixes #2926

This is my first contribution to Bookwyrm and, more importantly, my first time looking at Django and pytest. I am not (yet) a python person. The code and tests are pretty simple but something is not making sense.

Most importantly, the ratings and reviews do move to the new edition in manual testing. If I comment out the for loop or just the line readthrough.book = new_edition then the associated test fails. Same for review.book = new_edition and my new test for reviews. All good so far. However, if I remove the new code for ratings or just the rating.book = new_edition line, the new test for ratings still passes. I feel crazy.

I hope this is at least a step in the right direction. Thanks!

Copy link
Contributor

@dato dato left a comment

Choose a reason for hiding this comment

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

This is my first contribution to Bookwyrm and, more importantly, my first time looking at Django and pytest. I am not (yet) a python person.

Welcome! And thanks for contributing. I'm a fairly new contributor myself, and interacting with the community has been a pleasure so far. <3

The code and tests are pretty simple but something is not making sense.

Ah, I cloned your PR and then followed your steps... (behaved as you described), then started reading about the models a bit and noticed...

However, if I remove the new code for ratings or just the rating.book = new_edition line, the new test for ratings still passes. I feel crazy.

... I believe this weird behavior could be accounted for by the fact that ReviewRating inherits from Review, and hence the loop over Review.objects would cover ReviewRating as well?

bookwyrm/views/books/editions.py Outdated Show resolved Hide resolved
ReviewRatings are a subclass and are included in the models.Review block
@mattlehrer mattlehrer marked this pull request as ready for review November 16, 2023 19:43
mattlehrer and others added 3 commits November 20, 2023 09:31
…thub.com:mattlehrer/bookwyrm into move-ratings-and-reviews-when-switching-editions
@mattlehrer
Copy link
Contributor Author

I fixed the lint problems, sorry!

@dato dato self-requested a review February 21, 2024 20:51
Copy link
Contributor

@dato dato left a comment

Choose a reason for hiding this comment

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

Hi again!

Apologies for the incredible delay. I sat down to re-review and test this, and everything looks good. I'm going to trigger a re-run of the tests against the latest main, and merge once they pass.

Thanks again for your contribution!

@dato dato merged commit 1fabe51 into bookwyrm-social:main Feb 21, 2024
10 checks passed
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.

Ratings aren't switched when switching editions
2 participants