From c05ff0c8cc5bdd911c86e348d631c1052b507ebd Mon Sep 17 00:00:00 2001 From: lauren Date: Thu, 13 May 2021 22:37:33 -0400 Subject: [PATCH 1/2] fix(polls): clean up voting logic --- intranet/apps/polls/views.py | 81 ++++++++++++++++++------------------ 1 file changed, 40 insertions(+), 41 deletions(-) diff --git a/intranet/apps/polls/views.py b/intranet/apps/polls/views.py index db0f01d7dd6..028674121ec 100644 --- a/intranet/apps/polls/views.py +++ b/intranet/apps/polls/views.py @@ -8,11 +8,14 @@ from django.contrib.auth import get_user_model from django.contrib.auth.decorators import login_required from django.core.serializers import serialize +from django.db import transaction +from django.db.models import Q from django.shortcuts import get_object_or_404, redirect, render from django.utils import timezone from ...utils.date import get_senior_graduation_year from ...utils.html import safe_html +from ...utils.locking import lock_on from ..auth.decorators import deny_restricted from .forms import PollForm from .models import Answer, Choice, Poll, Question @@ -115,8 +118,7 @@ def poll_vote_view(request, poll_id): choices = question_obj.choice_set.all() if question_obj.is_single_choice(): if choice_num and choice_num == "CLEAR": - Answer.objects.filter(user=user, question=question_obj).delete() - Answer.objects.create(user=user, question=question_obj, clear_vote=True) + Answer.objects.update_or_create(user=user, question=question_obj, defaults={"clear_vote": True, "choice": None}) messages.success(request, "Clear Vote for {}".format(question_obj)) else: try: @@ -125,51 +127,48 @@ def poll_vote_view(request, poll_id): messages.error(request, "Invalid answer choice with num {}".format(choice_num)) continue else: - Answer.objects.filter(user=user, question=question_obj).delete() - Answer.objects.create(user=user, question=question_obj, choice=choice_obj) + Answer.objects.update_or_create(user=user, question=question_obj, defaults={"clear_vote": False, "choice": choice_obj}) messages.success(request, "Voted for {} on {}".format(choice_obj, question_obj)) elif question_obj.is_many_choice(): - total_choices = request.POST.getlist(name) - if len(total_choices) == 1 and total_choices[0] == "CLEAR": - Answer.objects.filter(user=user, question=question_obj).delete() - Answer.objects.create(user=user, question=question_obj, clear_vote=True) - messages.success(request, "Clear Vote for {}".format(question_obj)) - elif "CLEAR" in total_choices: + updated_choices = request.POST.getlist(name) + print(updated_choices) + if len(updated_choices) == 1 and updated_choices[0] == "CLEAR": + with transaction.atomic(): + # Lock on the user's answers to prevent duplicates. + lock_on(user.answer_set.all()) + Answer.objects.filter(user=user, question=question_obj).delete() + Answer.objects.create(user=user, question=question_obj, clear_vote=True) + messages.success(request, "Clear Vote for {}".format(question_obj)) + elif "CLEAR" in updated_choices: messages.error(request, "Cannot select other options with Clear Vote.") + elif len(updated_choices) > question_obj.max_choices: + messages.error(request, "You have voted on too many options for {}".format(question_obj)) else: - current_choices = Answer.objects.filter(user=user, question=question_obj) - current_choices_nums = [c.choice.num if c.choice else None for c in current_choices] - # delete entries that weren't checked but in db - for c in current_choices_nums: - if c and c not in total_choices: - ch = choices.get(num=c) - logger.info("Deleting choice for %s", ch) - Answer.objects.filter(user=user, question=question_obj, choice=ch).delete() - for c in total_choices: - # gets re-checked on each loop - current_choices = Answer.objects.filter(user=user, question=question_obj) - try: - choice_obj = choices.get(num=c) - except Choice.DoesNotExist: - messages.error(request, "Invalid answer choice with num {}".format(choice_num)) - continue - else: - if (current_choices.count() + 1) <= question_obj.max_choices: - Answer.objects.filter(user=user, question=question_obj, clear_vote=True).delete() - - # Duplicate Answers have caused errors here, so let's make sure to delete any duplicates - if Answer.objects.filter(user=user, question=question_obj, choice=choice_obj).count() != 1: - Answer.objects.filter(user=user, question=question_obj, choice=choice_obj).delete() - Answer.objects.get_or_create(user=user, question=question_obj, choice=choice_obj) - - messages.success(request, "Voted for {} on {}".format(choice_obj, question_obj)) - else: - messages.error(request, "You have voted on too many options for {}".format(question_obj)) - current_choices.delete() + print(choices) + with transaction.atomic(): + # Lock on the question's answers to prevent duplicates. + lock_on(user.answer_set.all()) + + available_answers = [c.num for c in choices] + updated_answers = [int(c) for c in updated_choices if int(c) in available_answers] + current_answers = [c.choice.num for c in Answer.objects.filter(user=user, question=question_obj) if c.choice] + print(updated_answers) + print(current_answers) + + to_create = [choices.get(num=c) for c in updated_answers if c not in current_answers] + print(to_create) + for c in to_create: + Answer.objects.create(user=user, question=question_obj, choice=c) + messages.success(request, "Voted for {} on {}".format(c, question_obj)) + + to_delete = [choices.get(num=c) for c in current_answers if c not in updated_answers] + print(to_delete) + for c in to_delete: + logger.info("Deleting choice for %s", c) + Answer.objects.filter(user=user, question=question_obj).filter(Q(clear_vote=True) | Q(choice__in=to_delete)).delete() elif question_obj.is_writing(): - Answer.objects.filter(user=user, question=question_obj).delete() - Answer.objects.create(user=user, question=question_obj, answer=choice_num) + Answer.objects.update_or_create(user=user, question=question_obj, defaults={"answer": choice_num}) messages.success(request, "Answer saved for {}".format(question_obj)) questions = [] From 2ad7aaaf4b5a8174d9a5d0386110e9f81d4ba2d0 Mon Sep 17 00:00:00 2001 From: lauren Date: Fri, 14 May 2021 11:26:52 -0400 Subject: [PATCH 2/2] test(polls): add test for voting --- intranet/apps/polls/tests.py | 223 +++++++++++++++++++++++++++++++++++ intranet/apps/polls/views.py | 11 +- 2 files changed, 227 insertions(+), 7 deletions(-) diff --git a/intranet/apps/polls/tests.py b/intranet/apps/polls/tests.py index b5ee45a2e58..16b9030c072 100644 --- a/intranet/apps/polls/tests.py +++ b/intranet/apps/polls/tests.py @@ -209,3 +209,226 @@ def test_csv_results(self): # Try again response = self.client.get(reverse("poll_csv_results", kwargs={"poll_id": poll.id}), follow=True) self.assertEqual(200, response.status_code) + + def test_single_select_voting(self): + self.make_admin() + + question_data = json.dumps( + [ + { + "question": "What is your favorite color?", + "type": "STD", + "max_choices": "1", + "choices": [{"info": "Red"}, {"info": "Green"}, {"info": "Blue"}], + } + ] + ) + + self.client.post( + reverse("add_poll"), + data={ + "title": "Single Select Poll", + "description": "This is a test poll!", + "start_time": (timezone.now() - datetime.timedelta(days=1)).strftime("%Y-%m-%d %H:%M:%S"), + "end_time": (timezone.now() + datetime.timedelta(days=1)).strftime("%Y-%m-%d %H:%M:%S"), + "visible": True, + "question_data": question_data, + }, + ) + + poll = Poll.objects.get(title="Single Select Poll") + question = poll.question_set.all()[0] + + # Vote for "Red" + self.client.post( + reverse("poll_vote", kwargs={"poll_id": poll.id}), + data={"question-1": "1"}, + ) + answer_set = question.answer_set.all() + self.assertEqual(len(answer_set), 1) + self.assertEqual(answer_set[0].choice.num, 1) + self.assertEqual(answer_set[0].choice.info, "Red") + self.assertEqual(answer_set[0].clear_vote, False) + + # Vote for "Red" again + self.client.post( + reverse("poll_vote", kwargs={"poll_id": poll.id}), + data={"question-1": "1"}, + ) + answer_set = question.answer_set.all() + self.assertEqual(len(answer_set), 1) + self.assertEqual(answer_set[0].choice.num, 1) + self.assertEqual(answer_set[0].clear_vote, False) + + # Clear vote + self.client.post( + reverse("poll_vote", kwargs={"poll_id": poll.id}), + data={"question-1": "CLEAR"}, + ) + answer_set = question.answer_set.all() + self.assertEqual(len(answer_set), 1) + self.assertEqual(answer_set[0].clear_vote, True) + + # Vote for "Blue" + self.client.post( + reverse("poll_vote", kwargs={"poll_id": poll.id}), + data={"question-1": "3"}, + ) + answer_set = question.answer_set.all() + self.assertEqual(len(answer_set), 1) + self.assertEqual(answer_set[0].choice.num, 3) + self.assertEqual(answer_set[0].choice.info, "Blue") + self.assertEqual(answer_set[0].clear_vote, False) + + def test_multi_select_voting(self): + self.make_admin() + + question_data = json.dumps( + [ + { + "question": "What is your favorite color?", + "type": "APP", + "max_choices": "2", + "choices": [{"info": "Red"}, {"info": "Green"}, {"info": "Blue"}], + } + ] + ) + + self.client.post( + reverse("add_poll"), + data={ + "title": "Multi Select Poll", + "description": "This is a test poll!", + "start_time": (timezone.now() - datetime.timedelta(days=1)).strftime("%Y-%m-%d %H:%M:%S"), + "end_time": (timezone.now() + datetime.timedelta(days=1)).strftime("%Y-%m-%d %H:%M:%S"), + "visible": True, + "question_data": question_data, + }, + ) + + poll = Poll.objects.get(title="Multi Select Poll") + question = poll.question_set.all()[0] + + # Vote for "Red" and "Blue" + self.client.post( + reverse("poll_vote", kwargs={"poll_id": poll.id}), + data={"question-1": ["1", "3"]}, + ) + answer_set = question.answer_set.all() + self.assertEqual(len(answer_set), 2) + self.assertEqual(answer_set[0].choice.num, 1) + self.assertEqual(answer_set[0].choice.info, "Red") + self.assertEqual(answer_set[1].choice.num, 3) + self.assertEqual(answer_set[1].choice.info, "Blue") + self.assertEqual(answer_set[0].clear_vote, False) + self.assertEqual(answer_set[1].clear_vote, False) + + # Vote for "Red" and "Blue" again + self.client.post( + reverse("poll_vote", kwargs={"poll_id": poll.id}), + data={"question-1": ["1", "3"]}, + ) + answer_set = question.answer_set.all() + self.assertEqual(len(answer_set), 2) + self.assertEqual(answer_set[0].choice.num, 1) + self.assertEqual(answer_set[0].choice.info, "Red") + self.assertEqual(answer_set[1].choice.num, 3) + self.assertEqual(answer_set[1].choice.info, "Blue") + self.assertEqual(answer_set[0].clear_vote, False) + self.assertEqual(answer_set[1].clear_vote, False) + + # Vote for "Red", "Blue" and "Green" which should exceed max_choices and nothing should change + response = self.client.post( + reverse("poll_vote", kwargs={"poll_id": poll.id}), + data={"question-1": ["1", "2", "3"]}, + ) + answer_set = question.answer_set.all() + self.assertIn( + "You have voted on too many options for Question #1: 'What is your favorite color?'", list(map(str, list(response.context["messages"]))) + ) + self.assertEqual(len(answer_set), 2) + self.assertEqual(answer_set[0].choice.num, 1) + self.assertEqual(answer_set[0].choice.info, "Red") + self.assertEqual(answer_set[1].choice.num, 3) + self.assertEqual(answer_set[1].choice.info, "Blue") + self.assertEqual(answer_set[0].clear_vote, False) + self.assertEqual(answer_set[1].clear_vote, False) + + # Clear vote + self.client.post( + reverse("poll_vote", kwargs={"poll_id": poll.id}), + data={"question-1": "CLEAR"}, + ) + answer_set = question.answer_set.all() + self.assertEqual(len(answer_set), 1) + self.assertEqual(answer_set[0].clear_vote, True) + + # Vote for "Green" + self.client.post( + reverse("poll_vote", kwargs={"poll_id": poll.id}), + data={"question-1": ["2"]}, + ) + answer_set = question.answer_set.all() + self.assertEqual(len(answer_set), 1) + self.assertEqual(answer_set[0].choice.num, 2) + self.assertEqual(answer_set[0].choice.info, "Green") + self.assertEqual(answer_set[0].clear_vote, False) + + def test_free_response_voting(self): + self.make_admin() + + question_data = json.dumps( + [ + { + "question": "What is your favorite color?", + "type": "FRE", + "max_choices": "1", + "choices": [], + } + ] + ) + + self.client.post( + reverse("add_poll"), + data={ + "title": "Free Response Poll", + "description": "This is a test poll!", + "start_time": (timezone.now() - datetime.timedelta(days=1)).strftime("%Y-%m-%d %H:%M:%S"), + "end_time": (timezone.now() + datetime.timedelta(days=1)).strftime("%Y-%m-%d %H:%M:%S"), + "visible": True, + "question_data": question_data, + }, + ) + + poll = Poll.objects.get(title="Free Response Poll") + question = poll.question_set.all()[0] + + # Submit vote + self.client.post( + reverse("poll_vote", kwargs={"poll_id": poll.id}), + data={"question-1": "test content"}, + ) + answer_set = question.answer_set.all() + self.assertEqual(len(answer_set), 1) + self.assertEqual(answer_set[0].answer, "test content") + self.assertEqual(answer_set[0].clear_vote, False) + + # Submit vote again + self.client.post( + reverse("poll_vote", kwargs={"poll_id": poll.id}), + data={"question-1": "test content"}, + ) + answer_set = question.answer_set.all() + self.assertEqual(len(answer_set), 1) + self.assertEqual(answer_set[0].answer, "test content") + self.assertEqual(answer_set[0].clear_vote, False) + + # Submit different vote + self.client.post( + reverse("poll_vote", kwargs={"poll_id": poll.id}), + data={"question-1": "different test content"}, + ) + answer_set = question.answer_set.all() + self.assertEqual(len(answer_set), 1) + self.assertEqual(answer_set[0].answer, "different test content") + self.assertEqual(answer_set[0].clear_vote, False) diff --git a/intranet/apps/polls/views.py b/intranet/apps/polls/views.py index 028674121ec..31e1dae5b7d 100644 --- a/intranet/apps/polls/views.py +++ b/intranet/apps/polls/views.py @@ -127,11 +127,12 @@ def poll_vote_view(request, poll_id): messages.error(request, "Invalid answer choice with num {}".format(choice_num)) continue else: - Answer.objects.update_or_create(user=user, question=question_obj, defaults={"clear_vote": False, "choice": choice_obj}) + Answer.objects.update_or_create( + user=user, question=question_obj, defaults={"clear_vote": False, "choice": choice_obj} + ) messages.success(request, "Voted for {} on {}".format(choice_obj, question_obj)) elif question_obj.is_many_choice(): updated_choices = request.POST.getlist(name) - print(updated_choices) if len(updated_choices) == 1 and updated_choices[0] == "CLEAR": with transaction.atomic(): # Lock on the user's answers to prevent duplicates. @@ -144,19 +145,15 @@ def poll_vote_view(request, poll_id): elif len(updated_choices) > question_obj.max_choices: messages.error(request, "You have voted on too many options for {}".format(question_obj)) else: - print(choices) with transaction.atomic(): # Lock on the question's answers to prevent duplicates. lock_on(user.answer_set.all()) - + available_answers = [c.num for c in choices] updated_answers = [int(c) for c in updated_choices if int(c) in available_answers] current_answers = [c.choice.num for c in Answer.objects.filter(user=user, question=question_obj) if c.choice] - print(updated_answers) - print(current_answers) to_create = [choices.get(num=c) for c in updated_answers if c not in current_answers] - print(to_create) for c in to_create: Answer.objects.create(user=user, question=question_obj, choice=c) messages.success(request, "Voted for {} on {}".format(c, question_obj))