reuse last PageRev if same author and short time diff

club/templates/club/page_history.jinja

@@ -1,12 +1,8 @@
 {% extends "core/base.jinja" %}
-{% from 'core/macros_pages.jinja' import page_history %}
+{% from 'core/page/macros.jinja' import page_history %}
 
 {% block content %}
-  {% if club.page %}
+  {{ page_history(club.page) }}
-    {{ page_history(club.page) }}
-  {% else %}
-    {% trans %}No page existing for this club{% endtrans %}
-  {% endif %}
 {% endblock %}
 
 

club/templates/club/pagerev_edit.jinja

@@ -1,8 +1,12 @@
 {% extends "core/base.jinja" %}
-{% from 'core/macros_pages.jinja' import page_edit_form %}
 
 {% block content %}
-  {{ page_edit_form(form, url('club:club_edit_page', club_id=page.club.id)) }}
+  <h2>{% trans %}Edit page{% endtrans %}</h2>
+  <form action="{{ url('club:club_edit_page', club_id=page.club.id) }}" method="post">
+    {% csrf_token %}
+    {{ form.as_p() }}
+    <p><input type="submit" value="{% trans %}Save{% endtrans %}" /></p>
+  </form>
 {% endblock %}
 
 

club/tests/test_page.py

@@ -3,9 +3,10 @@ from bs4 import BeautifulSoup
 from django.test import Client
 from django.urls import reverse
 from model_bakery import baker
-from pytest_django.asserts import assertHTMLEqual
+from pytest_django.asserts import assertHTMLEqual, assertRedirects
 
-from club.models import Club
+from club.models import Club, Membership
+from core.baker_recipes import subscriber_user
 from core.markdown import markdown
 from core.models import PageRev, User
 
@@ -16,7 +17,6 @@ def test_page_display_on_club_main_page(client: Client):
     club = baker.make(Club)
     content = "# foo\nLorem ipsum dolor sit amet"
     baker.make(PageRev, page=club.page, revision=1, content=content)
-    client.force_login(baker.make(User))
     res = client.get(reverse("club:club_view", kwargs={"club_id": club.id}))
 
     assert res.status_code == 200
@@ -30,10 +30,42 @@ def test_club_main_page_without_content(client: Client):
     """Test the club view works, even if the club page is empty"""
     club = baker.make(Club)
     club.page.revisions.all().delete()
-    client.force_login(baker.make(User))
     res = client.get(reverse("club:club_view", kwargs={"club_id": club.id}))
 
     assert res.status_code == 200
     soup = BeautifulSoup(res.text, "lxml")
     detail_html = soup.find(id="club_detail")
     assert detail_html.find_all("markdown") == []
+
+
+@pytest.mark.django_db
+def test_page_revision(client: Client):
+    club = baker.make(Club)
+    revisions = baker.make(
+        PageRev, page=club.page, _quantity=3, content=iter(["foo", "bar", "baz"])
+    )
+    client.force_login(baker.make(User))
+    url = reverse(
+        "club:club_view_rev", kwargs={"club_id": club.id, "rev_id": revisions[1].id}
+    )
+    res = client.get(url)
+    assert res.status_code == 200
+    soup = BeautifulSoup(res.text, "lxml")
+    detail_html = soup.find(class_="markdown")
+    assertHTMLEqual(detail_html.decode_contents(), markdown(revisions[1].content))
+
+
+@pytest.mark.django_db
+def test_edit_page(client: Client):
+    club = baker.make(Club)
+    user = subscriber_user.make()
+    baker.make(Membership, user=user, club=club, role=3)
+    client.force_login(user)
+    url = reverse("club:club_edit_page", kwargs={"club_id": club.id})
+    content = "# foo\nLorem ipsum dolor sit amet"
+
+    res = client.get(url)
+    assert res.status_code == 200
+    res = client.post(url, data={"content": content})
+    assertRedirects(res, reverse("club:club_view", kwargs={"club_id": club.id}))
+    assert club.page.revisions.last().content == content

club/views.py

@@ -22,12 +22,14 @@
 #
 #
 
+from __future__ import annotations
+
 import csv
 import itertools
-from typing import Any
+from typing import TYPE_CHECKING, Any
 
 from django.conf import settings
-from django.contrib.auth.mixins import PermissionRequiredMixin
+from django.contrib.auth.mixins import LoginRequiredMixin, PermissionRequiredMixin
 from django.contrib.messages.views import SuccessMessageMixin
 from django.core.exceptions import NON_FIELD_ERRORS, PermissionDenied, ValidationError
 from django.core.paginator import InvalidPage, Paginator
@@ -36,7 +38,7 @@ from django.http import Http404, HttpResponseRedirect, StreamingHttpResponse
 from django.shortcuts import get_object_or_404, redirect
 from django.urls import reverse, reverse_lazy
 from django.utils import timezone
-from django.utils.safestring import SafeString
+from django.utils.functional import cached_property
 from django.utils.timezone import now
 from django.utils.translation import gettext
 from django.utils.translation import gettext_lazy as _
@@ -61,11 +63,14 @@ from com.views import (
     PosterListBaseView,
 )
 from core.auth.mixins import CanEditMixin, PermissionOrClubBoardRequiredMixin
-from core.models import PageRev
+from core.models import Page, PageRev
-from core.views import DetailFormView, PageEditViewBase, UseFragmentsMixin
+from core.views import BasePageEditView, DetailFormView, UseFragmentsMixin
 from core.views.mixins import FragmentMixin, FragmentRenderer, TabedViewMixin
 from counter.models import Selling
 
+if TYPE_CHECKING:
+    from django.utils.safestring import SafeString
+
 
 class ClubTabsMixin(TabedViewMixin):
     def get_tabs_title(self):
@@ -75,6 +80,8 @@ class ClubTabsMixin(TabedViewMixin):
             self.object = self.object.page.club
         elif isinstance(self.object, Poster):
             self.object = self.object.club
+        elif hasattr(self, "club"):
+            self.object = self.club
         return self.object.get_display_name()
 
     def get_list_of_tabs(self):
@@ -202,7 +209,7 @@ class ClubView(ClubTabsMixin, DetailView):
         return kwargs
 
 
-class ClubRevView(ClubView):
+class ClubRevView(LoginRequiredMixin, ClubView):
     """Display a specific page revision."""
 
     def dispatch(self, request, *args, **kwargs):
@@ -216,27 +223,26 @@ class ClubRevView(ClubView):
         return kwargs
 
 
-class ClubPageEditView(ClubTabsMixin, PageEditViewBase):
+class ClubPageEditView(ClubTabsMixin, BasePageEditView):
     template_name = "club/pagerev_edit.jinja"
     current_tab = "page_edit"
 
-    def dispatch(self, request, *args, **kwargs):
+    @cached_property
-        self.club = get_object_or_404(Club, pk=kwargs["club_id"])
+    def club(self):
-        if not self.club.page:
+        return get_object_or_404(Club, pk=self.kwargs["club_id"])
-            raise Http404
-        return super().dispatch(request, *args, **kwargs)
 
-    def get_object(self):
+    @cached_property
-        self.page = self.club.page
+    def page(self) -> Page:
-        self.page.set_lock(self.request.user)
+        page = self.club.page
-        return self.page.revisions.last()
+        page.set_lock(self.request.user)
+        return page
 
     def get_success_url(self, **kwargs):
         return reverse_lazy("club:club_view", kwargs={"club_id": self.club.id})
 
 
 class ClubPageHistView(ClubTabsMixin, PermissionRequiredMixin, DetailView):
-    """Modification hostory of the page."""
+    """Modification history of the page."""
 
     model = Club
     pk_url_kwarg = "club_id"

core/templates/core/page/edit.jinja

@@ -1,8 +1,12 @@
 {% extends "core/page/base.jinja" %}
-{% from 'core/page/macros.jinja' import page_edit_form %}
 
 {% block page %}
-  {{ page_edit_form(form, url('core:page_edit', page_name=page.get_full_name())) }}
+  <h2>{% trans %}Edit page{% endtrans %}</h2>
+  <form action="{{ url('core:page_edit', page_name=page.get_full_name()) }}" method="post">
+    {% csrf_token %}
+    {{ form.as_p() }}
+    <p><input type="submit" value="{% trans %}Save{% endtrans %}" /></p>
+  </form>
 {% endblock %}
 
 

core/templates/core/page/macros.jinja

@@ -17,12 +17,3 @@
     {%- endfor -%}
   </ul>
 {% endmacro %}
-
-{% macro page_edit_form(form, url) %}
-  <h2>{% trans %}Edit page{% endtrans %}</h2>
-  <form action="{{ url }}" method="post">
-    {% csrf_token %}
-    {{ form.as_p() }}
-    <p><input type="submit" value="{% trans %}Save{% endtrans %}" /></p>
-  </form>
-{% endmacro %}

core/tests/test_page.py

@@ -1,33 +1,72 @@
+from datetime import timedelta
+
+import freezegun
 import pytest
+from bs4 import BeautifulSoup
 from django.conf import settings
 from django.contrib.auth.models import Permission
 from django.test import Client
 from django.urls import reverse
+from django.utils.timezone import now
 from model_bakery import baker
 from pytest_django.asserts import assertHTMLEqual, assertRedirects
 
+from club.models import Club
 from core.baker_recipes import board_user, subscriber_user
 from core.markdown import markdown
 from core.models import AnonymousUser, Page, PageRev, User
 
 
 @pytest.mark.django_db
-def test_edit_page(client: Client):
+class TestEditPage:
-    user = board_user.make()
+    def test_edit_page(self, client: Client):
-    page = baker.prepare(Page)
+        user = board_user.make()
-    page.save(force_lock=True)
+        page = baker.prepare(Page)
-    page.view_groups.add(user.groups.first())
+        page.save(force_lock=True)
-    client.force_login(user)
+        page.view_groups.add(user.groups.first())
+        page.edit_groups.add(user.groups.first())
+        client.force_login(user)
 
-    url = reverse("core:page_edit", kwargs={"page_name": page._full_name})
+        url = reverse("core:page_edit", kwargs={"page_name": page._full_name})
-    res = client.get(url)
+        res = client.get(url)
-    assert res.status_code == 200
+        assert res.status_code == 200
 
-    res = client.post(url, data={"content": "Hello World"})
+        res = client.post(url, data={"content": "Hello World"})
-    assertRedirects(res, reverse("core:page", kwargs={"page_name": page._full_name}))
+        assertRedirects(
-    revision = page.revisions.last()
+            res, reverse("core:page", kwargs={"page_name": page._full_name})
-    assert revision.content == "Hello World"
+        )
+        revision = page.revisions.last()
+        assert revision.content == "Hello World"
 
+    def test_pagerev_reused(self, client):
+        """Test that the previous revision is edited, if same author and small time diff"""
+        user = baker.make(User, is_superuser=True)
+        page = baker.prepare(Page)
+        page.save(force_lock=True)
+        first_rev = baker.make(PageRev, author=user, page=page, date=now())
+        client.force_login(user)
+        url = reverse("core:page_edit", kwargs={"page_name": page._full_name})
+        client.post(url, data={"content": "Hello World"})
+        assert page.revisions.count() == 1
+        assert page.revisions.last() == first_rev
+        first_rev.refresh_from_db()
+        assert first_rev.author == user
+        assert first_rev.content == "Hello World"
+
+    def test_pagerev_not_reused(self, client):
+        """Test that a new revision is created if too much time
+        passed since the last one.
+        """
+        user = baker.make(User, is_superuser=True)
+        page = baker.prepare(Page)
+        page.save(force_lock=True)
+        first_rev = baker.make(PageRev, author=user, page=page, date=now())
+        client.force_login(user)
+        url = reverse("core:page_edit", kwargs={"page_name": page._full_name})
+        with freezegun.freeze_time(now() + timedelta(minutes=30)):
+            client.post(url, data={"content": "Hello World"})
+        assert page.revisions.count() == 2
+        assert page.revisions.last() != first_rev
 
 
 @pytest.mark.django_db

core/views/forms.py

@@ -21,7 +21,7 @@
 #
 #
 import re
-from datetime import date, datetime
+from datetime import date, datetime, timedelta
 from io import BytesIO
 
 from captcha.fields import CaptchaField
@@ -47,7 +47,7 @@ from phonenumber_field.widgets import RegionalPhoneNumberWidget
 from PIL import Image
 
 from antispam.forms import AntiSpamEmailField
-from core.models import Gift, Group, Page, SithFile, User
+from core.models import Gift, Group, Page, PageRev, SithFile, User
 from core.utils import resize_image
 from core.views.widgets.ajax_select import (
     AutoCompleteSelect,
@@ -55,6 +55,7 @@ from core.views.widgets.ajax_select import (
     AutoCompleteSelectMultipleGroup,
     AutoCompleteSelectUser,
 )
+from core.views.widgets.markdown import MarkdownInput
 
 # Widgets
 
@@ -379,6 +380,41 @@ class PageForm(forms.ModelForm):
         )
 
 
+class PageRevisionForm(forms.ModelForm):
+    """Form to add a new revision to a page.
+
+    Notes:
+        Saving this form won't always result in a new revision.
+        If the previous revision on the same page was made less
+        than 20 minutes ago by the same author,
+        then the latter will be edited and the new revision won't be created.
+    """
+
+    UPDATE_THRESHOLD = timedelta(minutes=20)
+
+    class Meta:
+        model = PageRev
+        fields = ["title", "content"]
+        widgets = {"content": MarkdownInput}
+
+    def __init__(self, *args, author: User, page: Page, **kwargs):
+        super().__init__(*args, **kwargs)
+        self.author = author
+        self.page = page
+
+    def save(self, commit=True):  # noqa FBT002
+        revision: PageRev = self.instance
+        if (
+            revision._state.adding
+            or revision.author != self.author
+            or revision.date + self.UPDATE_THRESHOLD < now()
+        ):
+            revision.author = self.author
+            revision.page = self.page
+            revision.id = None  # if id is None, Django will create a new record
+        return super().save(commit=commit)
+
+
 class GiftForm(forms.ModelForm):
     class Meta:
         model = Gift

core/views/page.py

@@ -13,26 +13,19 @@
 #
 #
 
-from django.contrib.auth.mixins import PermissionRequiredMixin
+from django.contrib.auth.mixins import PermissionRequiredMixin, UserPassesTestMixin
 from django.db.models import F, OuterRef, Subquery
 from django.db.models.functions import Coalesce
-
-# This file contains all the views that concern the page model
-from django.forms.models import modelform_factory
 from django.http import Http404
-from django.shortcuts import redirect
+from django.shortcuts import get_object_or_404, redirect
 from django.urls import reverse_lazy
+from django.utils.functional import cached_property
 from django.views.generic import DetailView, ListView
 from django.views.generic.edit import CreateView, DeleteView, UpdateView
 
-from core.auth.mixins import (
+from core.auth.mixins import CanEditPropMixin, CanViewMixin
-    CanEditMixin,
-    CanEditPropMixin,
-    CanViewMixin,
-)
 from core.models import Page, PageRev
-from core.views.forms import PageForm, PagePropForm
+from core.views.forms import PageForm, PagePropForm, PageRevisionForm
-from core.views.widgets.markdown import MarkdownInput
 
 
 class PageNotFound(Http404):
@@ -161,36 +154,37 @@ class PagePropView(CanEditPagePropMixin, UpdateView):
         return self.page
 
 
-class PageEditViewBase(CanEditMixin, UpdateView):
+class BasePageEditView(UserPassesTestMixin, UpdateView):
     model = PageRev
-    form_class = modelform_factory(
+    form_class = PageRevisionForm
-        model=PageRev, fields=["title", "content"], widgets={"content": MarkdownInput}
-    )
     template_name = "core/page/edit.jinja"
 
+    def test_func(self):
+        return self.request.user.can_edit(self.page)
+
+    @cached_property
+    def page(self) -> Page:
+        page = get_page_or_404(full_name=self.kwargs["page_name"])
+        page.set_lock(self.request.user)
+        return page
+
     def get_object(self, *args, **kwargs):
-        self.page = get_page_or_404(full_name=self.kwargs["page_name"])
-        self.page.set_lock(self.request.user)
         return self.page.revisions.last()
 
     def get_context_data(self, **kwargs):
         return super().get_context_data(**kwargs) | {"page": self.page}
 
-    def form_valid(self, form):
+    def get_form_kwargs(self):
-        # TODO : factor that, but first make some tests
+        return super().get_form_kwargs() | {
-        rev = form.instance
+            "author": self.request.user,
-        new_rev = PageRev(title=rev.title, content=rev.content)
+            "page": self.page,
-        new_rev.author = self.request.user
+        }
-        new_rev.page = self.page
-        form.instance = new_rev
-        return super().form_valid(form)
 
 
-class PageEditView(PageEditViewBase):
+class PageEditView(BasePageEditView):
     def dispatch(self, request, *args, **kwargs):
-        self.object = self.get_object()
+        if self.page.need_club_redirection:
-        if self.object and self.object.page.need_club_redirection:
+            return redirect("club:club_edit_page", club_id=self.page.club.id)
-            return redirect("club:club_edit_page", club_id=self.object.page.club.id)
         return super().dispatch(request, *args, **kwargs)