From 3509ddf3ed1c2253da9284cf750287fc2daced1e Mon Sep 17 00:00:00 2001 From: Florian Charlier Date: Fri, 7 Apr 2023 13:23:52 +0000 Subject: [PATCH] [FIX] website_forum: prevent calling vote on multi This method was supposed to work in multi (see history) but that behavior was lost somewhere. Voting for several posts at the same time doesn't mean much in the end and the function wouldn't work anyway because: 1. Return values wouldn't work in multi 2. It would try to vote n times for each of the n posts in self and violate unique constraint. 3. If the last existing vote had opposed value to the new vote, the method would create only '0'-valued votes for all the new records. We here then clean it make sure it is not called on several posts. A test is extended to check votes and records counts. Task-3267872 closes odoo/odoo#123725 X-original-commit: dc049ce98292e08f5ecf2f20005dc227375dde75 Signed-off-by: Thibault Delavallee (tde) Signed-off-by: Florian Charlier (flch) --- addons/website_forum/models/forum_post.py | 27 +++++------ .../tests/test_forum_karma_access.py | 46 +++++++++++++++++-- 2 files changed, 55 insertions(+), 18 deletions(-) diff --git a/addons/website_forum/models/forum_post.py b/addons/website_forum/models/forum_post.py index d9bf15487b0..5e514c89a5a 100644 --- a/addons/website_forum/models/forum_post.py +++ b/addons/website_forum/models/forum_post.py @@ -612,22 +612,19 @@ class Post(models.Model): return False def vote(self, upvote=True): + self.ensure_one() Vote = self.env['forum.post.vote'] - vote_ids = Vote.search([('post_id', 'in', self._ids), ('user_id', '=', self._uid)]) - new_vote = '1' if upvote else '-1' - voted_forum_ids = set() - if vote_ids: - for vote in vote_ids: - if upvote: - new_vote = '0' if vote.vote == '-1' else '1' - else: - new_vote = '0' if vote.vote == '1' else '-1' - vote.vote = new_vote - voted_forum_ids.add(vote.post_id.id) - for post_id in set(self._ids) - voted_forum_ids: - for post_id in self._ids: - Vote.create({'post_id': post_id, 'vote': new_vote}) - return {'vote_count': self.vote_count, 'user_vote': new_vote} + existing_vote = Vote.search([('post_id', '=', self.id), ('user_id', '=', self._uid)]) + new_vote_value = '1' if upvote else '-1' + if existing_vote: + if upvote: + new_vote_value = '0' if existing_vote.vote == '-1' else '1' + else: + new_vote_value = '0' if existing_vote.vote == '1' else '-1' + existing_vote.vote = new_vote_value + else: + Vote.create({'post_id': self.id, 'vote': new_vote_value}) + return {'vote_count': self.vote_count, 'user_vote': new_vote_value} def convert_answer_to_comment(self): """ Tools to convert an answer (forum.post) to a comment (mail.message). diff --git a/addons/website_forum/tests/test_forum_karma_access.py b/addons/website_forum/tests/test_forum_karma_access.py index ea1f8a48f66..973862951f6 100644 --- a/addons/website_forum/tests/test_forum_karma_access.py +++ b/addons/website_forum/tests/test_forum_karma_access.py @@ -373,10 +373,50 @@ class TestForumKarma(TestForumCommon): self.assertEqual(post.state, 'active', 'website_forum: wrong state when validate a post after offensive') def test_vote(self): + def check_vote_records_count_and_integrity(expected_total_votes_count): + groups = self.env['forum.post.vote'].read_group([], fields=['__count'], groupby=['post_id', 'user_id'], lazy=False) + self.assertEqual(len(groups), expected_total_votes_count) + for post_user_group in groups: + self.assertEqual(post_user_group['__count'], 1) + + check_vote_records_count_and_integrity(2) self.post.create_uid.karma = KARMA['ask'] - self.user_portal.karma = KARMA['upv'] - self.post.with_user(self.user_portal).vote(upvote=True) - self.assertEqual(self.post.create_uid.karma, KARMA['ask'] + KARMA['gen_que_upv'], 'website_forum: wrong karma generation of upvoted question author') + self.user_portal.karma = KARMA['dwv'] + initial_vote_count = self.post.vote_count + post_as_portal = self.post.with_user(self.user_portal) + res = post_as_portal.vote(upvote=True) + + self.assertEqual(res['user_vote'], '1') + self.assertEqual(res['vote_count'], initial_vote_count + 1) + self.assertEqual(post_as_portal.user_vote, 1) + self.assertEqual( + self.post.create_uid.karma, + KARMA['ask'] + KARMA['gen_que_upv'], + 'website_forum: wrong karma generation of upvoted question author', + ) + + # On voting again with the same value, nothing changes + res = post_as_portal.vote(upvote=True) + self.assertEqual(res['vote_count'], initial_vote_count + 1) + self.assertEqual(res['user_vote'], '1') + self.post.invalidate_recordset() + self.assertEqual(post_as_portal.user_vote, 1) + + # On reverting vote, vote cancels + res = post_as_portal.vote(upvote=False) + self.assertEqual(res['vote_count'], initial_vote_count) + self.assertEqual(res['user_vote'], '0') + self.post.invalidate_recordset() + self.assertEqual(post_as_portal.user_vote, 0) + + # Everything works from "0" too + res = post_as_portal.vote(upvote=False) + self.assertEqual(res['vote_count'], initial_vote_count - 1) + self.assertEqual(res['user_vote'], '-1') + self.post.invalidate_recordset() + self.assertEqual(post_as_portal.user_vote, -1) + + check_vote_records_count_and_integrity(3) @mute_logger('odoo.addons.base.models.ir_model', 'odoo.models') def test_vote_crash(self):