Remove the nb_{yes,no} fields
These fields duplicate information that is already present in the Answer table. I guess they exist(ed) as an optimisation to avoid recomputing the number of positive and negative answers per proposition each time the list page is loaded. Given the small number of propositions we have in practice and the extra housekeeping required to keep these fields up to date, I consider simplicity matters more here.
This commit is contained in:
parent
9b6cc2b486
commit
5381aba922
5 changed files with 45 additions and 33 deletions
24
propositions/migrations/0005_remove_nb_yes_no_fields.py
Normal file
24
propositions/migrations/0005_remove_nb_yes_no_fields.py
Normal file
|
@ -0,0 +1,24 @@
|
|||
# Generated by Django 2.2.9 on 2020-01-05 15:26
|
||||
|
||||
from django.db import migrations, models
|
||||
import django.db.models.deletion
|
||||
|
||||
|
||||
class Migration(migrations.Migration):
|
||||
|
||||
dependencies = [
|
||||
("propositions", "0004_prop_renaming_and_cleaning"),
|
||||
]
|
||||
|
||||
operations = [
|
||||
migrations.RemoveField(model_name="proposition", name="nb_no",),
|
||||
migrations.RemoveField(model_name="proposition", name="nb_yes",),
|
||||
migrations.AlterField(
|
||||
model_name="answer",
|
||||
name="proposition",
|
||||
field=models.ForeignKey(
|
||||
on_delete=django.db.models.deletion.CASCADE,
|
||||
to="propositions.Proposition",
|
||||
),
|
||||
),
|
||||
]
|
|
@ -12,8 +12,6 @@ class Proposition(models.Model):
|
|||
ErnestoUser, on_delete=models.CASCADE, verbose_name="Proposé par"
|
||||
)
|
||||
link = models.URLField(blank=True)
|
||||
nb_yes = models.IntegerField(default=0, verbose_name="nombre de réponses oui")
|
||||
nb_no = models.IntegerField(default=0, verbose_name="nombre de réponses non")
|
||||
|
||||
def __str__(self):
|
||||
return self.name
|
||||
|
@ -27,10 +25,7 @@ class Answer(models.Model):
|
|||
YES = "oui"
|
||||
NO = "non"
|
||||
|
||||
REP_CHOICES = [
|
||||
(YES, "Oui"),
|
||||
(NO, "Non")
|
||||
]
|
||||
REP_CHOICES = [(YES, "Oui"), (NO, "Non")]
|
||||
|
||||
proposition = models.ForeignKey(Proposition, on_delete=models.CASCADE)
|
||||
user = models.ForeignKey(User, on_delete=models.CASCADE)
|
||||
|
|
|
@ -44,8 +44,6 @@ class PropositionCreateTest(TestCase):
|
|||
self.assertEqual(proposition.artist, "bar")
|
||||
self.assertEqual(proposition.link, "http://example.com")
|
||||
self.assertEqual(proposition.user, user.profile)
|
||||
self.assertEqual(proposition.nb_yes, 0)
|
||||
self.assertEqual(proposition.nb_no, 0)
|
||||
|
||||
|
||||
class PropositionListTest(TestCase):
|
||||
|
@ -99,11 +97,13 @@ class ReponseTest(TestCase):
|
|||
# Oui
|
||||
client.get(reverse("propositions:oui", args=(self.prop.id,)))
|
||||
self.prop.refresh_from_db()
|
||||
self.assertEqual(self.prop.nb_yes, 1)
|
||||
self.assertEqual(self.prop.nb_no, 0)
|
||||
self.assertEqual(
|
||||
list(self.prop.answer_set.values_list("answer", flat=True)), [Answer.YES],
|
||||
)
|
||||
|
||||
# Oui
|
||||
client.get(reverse("propositions:non", args=(self.prop.id,)))
|
||||
self.prop.refresh_from_db()
|
||||
self.assertEqual(self.prop.nb_yes, 0)
|
||||
self.assertEqual(self.prop.nb_no, 1)
|
||||
self.assertEqual(
|
||||
list(self.prop.answer_set.values_list("answer", flat=True)), [Answer.NO]
|
||||
)
|
||||
|
|
|
@ -8,7 +8,7 @@ app_name = "propositions"
|
|||
urlpatterns = [
|
||||
path("", views.PropositionList.as_view(), name="list"),
|
||||
path("new", views.PropositionCreate.as_view(), name="create"),
|
||||
path("<int:id>/oui", views.answer, {"rep": "oui"}, name=Answer.YES),
|
||||
path("<int:id>/non", views.answer, {"rep": "non"}, name=Answer.NO),
|
||||
path("<int:id>/oui", views.answer, {"ans": "oui"}, name=Answer.YES),
|
||||
path("<int:id>/non", views.answer, {"ans": "non"}, name=Answer.NO),
|
||||
path("<int:pk>/supprimer", PropDelete.as_view(), name="delete"),
|
||||
]
|
||||
|
|
|
@ -2,6 +2,7 @@ from django.shortcuts import redirect, get_object_or_404
|
|||
from django.urls import reverse_lazy
|
||||
from django.contrib.auth.decorators import login_required
|
||||
from django.contrib.auth.mixins import LoginRequiredMixin
|
||||
from django.db.models import Count, Q
|
||||
from django.views.generic import CreateView, DeleteView, ListView
|
||||
from django.utils.decorators import method_decorator
|
||||
from django.http import HttpResponseRedirect
|
||||
|
@ -26,7 +27,14 @@ class PropositionList(LoginRequiredMixin, ListView):
|
|||
template_name = "propositions/liste.html"
|
||||
context_object_name = "propositions"
|
||||
model = Proposition
|
||||
ordering = ["-nb_yes", "nb_no", "name"]
|
||||
|
||||
def get_queryset(self):
|
||||
return (
|
||||
Proposition.objects
|
||||
.annotate(nb_yes=Count("answer", filter=Q(answer__answer=Answer.YES)))
|
||||
.annotate(nb_no=Count("answer", filter=Q(answer__answer=Answer.NO)))
|
||||
.order_by("-nb_yes", "nb_no", "name")
|
||||
)
|
||||
|
||||
|
||||
class PropDelete(DeleteView):
|
||||
|
@ -50,24 +58,9 @@ class PropDelete(DeleteView):
|
|||
|
||||
|
||||
@login_required
|
||||
def answer(request, id, rep):
|
||||
def answer(request, id, ans):
|
||||
proposition = get_object_or_404(Proposition, id=id)
|
||||
user = request.user
|
||||
|
||||
old_answer = Answer.objects.filter(proposition=proposition, user=user)
|
||||
if old_answer.exists():
|
||||
old_answer = old_answer.get()
|
||||
if old_answer.answer == Answer.YES:
|
||||
proposition.nb_yes -= 1
|
||||
else:
|
||||
proposition.nb_no -= 1
|
||||
old_answer.delete()
|
||||
|
||||
Answer.objects.create(proposition=proposition, user=user, answer=rep)
|
||||
if rep == Answer.YES:
|
||||
proposition.nb_yes += 1
|
||||
else:
|
||||
proposition.nb_no += 1
|
||||
proposition.save()
|
||||
|
||||
Answer.objects.filter(proposition=proposition, user=user).delete()
|
||||
Answer.objects.create(proposition=proposition, user=user, answer=ans)
|
||||
return redirect("propositions:list")
|
||||
|
|
Loading…
Reference in a new issue