From de78cc0c495c4691419f8bfc7df1232c592f61fd Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Laur=C3=A9line=20Gu=C3=A9rin?= <zebuline@entrouvert.com>
Date: Fri, 31 Jan 2020 10:11:49 +0100
Subject: [PATCH] page: new pages are excluded from navigation by default
 (#17659)

---
 .../migrations/0041_auto_20200130_1619.py     | 19 +++++++++
 combo/data/models.py                          |  4 +-
 combo/manager/forms.py                        | 22 +++++++++-
 combo/manager/templates/combo/page_view.html  |  6 +--
 combo/manager/urls.py                         |  4 +-
 combo/manager/views.py                        | 15 ++++---
 tests/test_calendar.py                        |  2 +-
 tests/test_cells.py                           | 20 +++++-----
 tests/test_manager.py                         | 40 +++++++++++++++----
 tests/test_public.py                          | 15 +++----
 10 files changed, 106 insertions(+), 41 deletions(-)
 create mode 100644 combo/data/migrations/0041_auto_20200130_1619.py

diff --git a/combo/data/migrations/0041_auto_20200130_1619.py b/combo/data/migrations/0041_auto_20200130_1619.py
new file mode 100644
index 0000000..39a8d1a
--- /dev/null
+++ b/combo/data/migrations/0041_auto_20200130_1619.py
@@ -0,0 +1,19 @@
+# -*- coding: utf-8 -*-
+from __future__ import unicode_literals
+
+from django.db import migrations, models
+
+
+class Migration(migrations.Migration):
+
+    dependencies = [
+        ('data', '0040_auto_20200119_1017'),
+    ]
+
+    operations = [
+        migrations.AlterField(
+            model_name='page',
+            name='exclude_from_navigation',
+            field=models.BooleanField(default=True, verbose_name='Exclude from navigation'),
+        ),
+    ]
diff --git a/combo/data/models.py b/combo/data/models.py
index 395910e..2512586 100644
--- a/combo/data/models.py
+++ b/combo/data/models.py
@@ -134,7 +134,7 @@ class Page(models.Model):
     template_name = models.CharField(_('Template'), max_length=50)
     parent = models.ForeignKey('self', on_delete=models.CASCADE, null=True, blank=True)
     order = models.PositiveIntegerField()
-    exclude_from_navigation = models.BooleanField(_('Exclude from navigation'), default=False)
+    exclude_from_navigation = models.BooleanField(_('Exclude from navigation'), default=True)
     redirect_url = models.CharField(_('Redirect URL'), max_length=200, blank=True)
 
     public = models.BooleanField(_('Public'), default=True)
@@ -445,6 +445,8 @@ class Page(models.Model):
         new_page.snapshot = None
         # set order
         new_page.order = self.order + 1
+        # exclude from navigation
+        new_page.exclude_from_navigation = True
         # store new page
         new_page.save()
 
diff --git a/combo/manager/forms.py b/combo/manager/forms.py
index e32193a..ce89e31 100644
--- a/combo/manager/forms.py
+++ b/combo/manager/forms.py
@@ -49,6 +49,7 @@ class PageEditSlugForm(forms.ModelForm):
             raise ValidationError(_('Slug must be unique'), code='unique')
         return value
 
+
 class PageEditDescriptionForm(forms.ModelForm):
     class Meta:
         model = Page
@@ -107,10 +108,27 @@ class PageEditRedirectionForm(forms.ModelForm):
         return page
 
 
-class PageEditExcludeFromNavigationForm(forms.ModelForm):
+class PageEditIncludeInNavigationForm(forms.ModelForm):
+    include_in_navigation = forms.BooleanField(
+        label=_('Include in navigation menus'),
+        required=False
+    )
+
     class Meta:
         model = Page
-        fields = ('exclude_from_navigation',)
+        fields = []
+
+    def __init__(self, *args, **kwargs):
+        instance = kwargs['instance']
+        initial = kwargs.pop('initial', {})
+        initial['include_in_navigation'] = not instance.exclude_from_navigation
+        super(PageEditIncludeInNavigationForm, self).__init__(initial=initial, *args, **kwargs)
+
+    def save(self, *args, **kwargs):
+        super(PageEditIncludeInNavigationForm, self).save(*args, **kwargs)
+        self.instance.exclude_from_navigation = not self.cleaned_data['include_in_navigation']
+        self.instance.save()
+        return self.instance
 
 
 class CellVisibilityForm(forms.Form):
diff --git a/combo/manager/templates/combo/page_view.html b/combo/manager/templates/combo/page_view.html
index b9c8f0d..3d9471d 100644
--- a/combo/manager/templates/combo/page_view.html
+++ b/combo/manager/templates/combo/page_view.html
@@ -61,9 +61,9 @@
 </p>
 
 <p>
-<label>{% trans 'Exclude from navigation:' %}</label>
-{% if object.exclude_from_navigation %}{% trans 'yes' %}{% else %}{% trans 'no' %}{% endif %}
-(<a rel="popup" href="{% url 'combo-manager-page-edit-exclude-from-navigation' pk=object.id %}">{% trans 'change' %}</a>)
+<label>{% trans 'Include in navigation menus:' %}</label>
+{% if object.exclude_from_navigation %}{% trans 'no' %}{% else %}{% trans 'yes' %}{% endif %}
+(<a rel="popup" href="{% url 'combo-manager-page-edit-include-in-navigation' pk=object.id %}">{% trans 'change' %}</a>)
 </p>
 
 <p>
diff --git a/combo/manager/urls.py b/combo/manager/urls.py
index a3f55e4..0ec7fe2 100644
--- a/combo/manager/urls.py
+++ b/combo/manager/urls.py
@@ -37,8 +37,8 @@ urlpatterns = [
             name='combo-manager-page-visibility'),
         url(r'^pages/(?P<pk>\d+)/redirection$', views.page_edit_redirection,
             name='combo-manager-page-edit-redirection'),
-        url(r'^pages/(?P<pk>\d+)/exclude-from-navigation$', views.page_edit_exclude_from_navigation,
-            name='combo-manager-page-edit-exclude-from-navigation'),
+        url(r'^pages/(?P<pk>\d+)/include-in-navigation$', views.page_edit_include_in_navigation,
+            name='combo-manager-page-edit-include-in-navigation'),
         url(r'^pages/(?P<pk>\d+)/slug$', views.page_edit_slug,
             name='combo-manager-page-edit-slug'),
         url(r'^pages/(?P<pk>\d+)/title$', views.page_edit_title,
diff --git a/combo/manager/views.py b/combo/manager/views.py
index 5d28d23..28963bd 100644
--- a/combo/manager/views.py
+++ b/combo/manager/views.py
@@ -40,7 +40,7 @@ from combo import plugins
 
 from .forms import (PageEditTitleForm, PageVisibilityForm, SiteImportForm,
         PageEditRedirectionForm, PageSelectTemplateForm, PageEditSlugForm,
-        PageEditPictureForm, PageEditExcludeFromNavigationForm,
+        PageEditPictureForm, PageEditIncludeInNavigationForm,
         PageEditDescriptionForm, CellVisibilityForm)
 
 
@@ -186,11 +186,11 @@ class PageEditRedirectionView(PageEditView):
 page_edit_redirection = PageEditRedirectionView.as_view()
 
 
-class PageEditExcludeFromNavigationView(PageEditView):
-    form_class = PageEditExcludeFromNavigationForm
-    comment = _('changed navigation exclusion')
+class PageEditIncludeInNavigationView(PageEditView):
+    form_class = PageEditIncludeInNavigationForm
+    comment = _('changed navigation inclusion')
 
-page_edit_exclude_from_navigation = PageEditExcludeFromNavigationView.as_view()
+page_edit_include_in_navigation = PageEditIncludeInNavigationView.as_view()
 
 
 class PageEditSlugView(PageEditView):
@@ -361,7 +361,10 @@ class PageDuplicateView(RedirectView):
     def get_redirect_url(self, pk):
         page = Page.objects.get(pk=pk)
         new_page = page.duplicate()
-        messages.info(self.request, _('Page %s has been duplicated.') % page.title)
+        if not page.exclude_from_navigation:
+            messages.info(self.request, _('Page %s has been duplicated, it has been marked as excluded from navigation.') % page.title)
+        else:
+            messages.info(self.request, _('Page %s has been duplicated.') % page.title)
         return reverse('combo-manager-page-view', kwargs={'pk': new_page.pk})
 
 
diff --git a/tests/test_calendar.py b/tests/test_calendar.py
index 623fd08..02de952 100644
--- a/tests/test_calendar.py
+++ b/tests/test_calendar.py
@@ -173,7 +173,7 @@ def client(request, anonymous, connected):
 
 @pytest.fixture
 def cell(db):
-    page = Page.objects.create(title='whatever', slug='booking', template_name='standard')
+    page = Page.objects.create(title='whatever', slug='booking', template_name='standard', exclude_from_navigation=False)
     cell = BookingCalendar(
         page=page, title='Example Of Calendar',
         agenda_reference='default:test',
diff --git a/tests/test_cells.py b/tests/test_cells.py
index 8c6e445..624d0f6 100644
--- a/tests/test_cells.py
+++ b/tests/test_cells.py
@@ -167,16 +167,16 @@ def test_link_list_cell():
 
 def test_menu_cell():
     Page.objects.all().delete()
-    parent = page = Page(title='Page1', slug='page1', template_name='standard')
-    page.save()
-    page = Page(title='Page2', slug='page2', template_name='standard',
-            parent=parent)
-    page.save()
-    page = Page(title='Page3', slug='page3', template_name='standard',
-            parent=parent, public=False)
-    page.save()
-    cell = MenuCell(root_page=parent, order=0, page=parent)
-    cell.save()
+    parent = Page.objects.create(
+        title='Page1', slug='page1', template_name='standard',
+        exclude_from_navigation=False)
+    page = Page.objects.create(
+        title='Page2', slug='page2', template_name='standard',
+        parent=parent, exclude_from_navigation=False)
+    page = Page.objects.create(
+        title='Page3', slug='page3', template_name='standard',
+        parent=parent, public=False, exclude_from_navigation=False)
+    cell = MenuCell.objects.create(root_page=parent, order=0, page=parent)
     request = RequestFactory().get('/page1/')
     request.user = None
     ctx = {'page': parent, 'request': request}
diff --git a/tests/test_manager.py b/tests/test_manager.py
index 69ebc46..08181f6 100644
--- a/tests/test_manager.py
+++ b/tests/test_manager.py
@@ -50,15 +50,20 @@ def test_access(app, admin_user):
     assert 'Pages' in resp.text
     assert "This site doesn't have any page yet." in resp.text
 
+
 def test_add_page(app, admin_user):
     app = login(app)
     resp = app.get('/manage/', status=200)
     resp = resp.click('New')
-    assert resp.forms[0]['title'].value == 'Home' # default title for first page
+    assert resp.forms[0]['title'].value == 'Home'  # default title for first page
     resp = resp.forms[0].submit()
-    assert resp.location.endswith('/manage/pages/1/')
-    assert Page.objects.get(slug='index').title == 'Home'
-    assert Page.objects.get(slug='index').template_name == 'standard' # first one was taken
+    page = Page.objects.latest('pk')
+    assert resp.location.endswith('/manage/pages/%s/' % page.pk)
+    assert page.slug == 'index'
+    assert page.title == 'Home'
+    assert page.template_name == 'standard'  # first one was taken
+    assert page.exclude_from_navigation is True
+
 
 def test_add_second_page(app, admin_user):
     Page.objects.all().delete()
@@ -133,11 +138,17 @@ def test_edit_page(app, admin_user):
     assert 'http://www.example.net' in resp.text
     assert Page.objects.all()[0].redirect_url == 'http://www.example.net'
     # exclude from nav
-    resp = resp.click(href='.*/exclude-from-navigation')
-    resp.form['exclude_from_navigation'].checked = True
+    resp = resp.click(href='.*/include-in-navigation')
+    resp.form['include_in_navigation'].checked = False
     resp = resp.form.submit()
     resp = resp.follow()
     assert Page.objects.all()[0].exclude_from_navigation is True
+    # include from nav
+    resp = resp.click(href='.*/include-in-navigation')
+    resp.form['include_in_navigation'].checked = True
+    resp = resp.form.submit()
+    resp = resp.follow()
+    assert Page.objects.all()[0].exclude_from_navigation is False
 
 
 def test_edit_page_optional_placeholder(app, admin_user):
@@ -514,8 +525,9 @@ def test_site_export_import_missing_group(app, admin_user):
     resp = resp.form.submit()
     assert 'Missing groups: foobar' in resp.text
 
+
 def test_duplicate_page(app, admin_user):
-    page = Page.objects.create(title='One', slug='one', template_name='standard')
+    page = Page.objects.create(title='One', slug='one', template_name='standard', exclude_from_navigation=False)
     TextCell.objects.create(page=page, placeholder='content', text='Foobar', order=0)
 
     app = login(app)
@@ -524,6 +536,20 @@ def test_duplicate_page(app, admin_user):
     new_page = Page.objects.latest('pk')
     assert resp.status_int == 302
     assert resp.location.endswith('/manage/pages/%s/' % new_page.pk)
+    resp = resp.follow()
+    assert 'Page %s has been duplicated, it has been marked as excluded from navigation.' % page.title in resp.text
+    assert new_page.exclude_from_navigation is True
+
+    page.exclude_from_navigation = True
+    page.save()
+    resp = app.get('/manage/pages/%s/' % page.pk)
+    resp = resp.click('Duplicate')
+    new_page = Page.objects.latest('pk')
+    assert resp.status_int == 302
+    assert resp.location.endswith('/manage/pages/%s/' % new_page.pk)
+    resp = resp.follow()
+    assert 'Page %s has been duplicated.' % page.title in resp.text
+    assert new_page.exclude_from_navigation is True
 
 
 def test_add_edit_cell(app, admin_user):
diff --git a/tests/test_public.py b/tests/test_public.py
index dc19fd0..25bbee3 100644
--- a/tests/test_public.py
+++ b/tests/test_public.py
@@ -320,9 +320,9 @@ def test_page_skeleton(app):
     resp = app.get('/__skeleton__/?source=%s' % quote('http://example.net/badredir'))
 
     # add a page with restricted visibility
-    page = Page(title='RestrictedVisibility', slug='restrictedvisibilit',
-            template_name='standard', public=False)
-    page.save()
+    page = Page.objects.create(
+        title='RestrictedVisibility', slug='restrictedvisibilit',
+        template_name='standard', public=False, exclude_from_navigation=False)
     resp = app.get('/__skeleton__/?source=%s' % quote('http://127.0.0.1:8999/'))
     assert 'RestrictedVisibility' in resp.text
 
@@ -406,12 +406,9 @@ def test_subpage_location(app):
 
 def test_menu(app):
     Page.objects.all().delete()
-    page = Page(title='Page1', slug='index', template_name='standard')
-    page.save()
-    page = Page(title='Page2', slug='page2', template_name='standard')
-    page.save()
-    page = Page(title='Page3', slug='page3', template_name='standard', public=False)
-    page.save()
+    Page.objects.create(title='Page1', slug='index', template_name='standard', exclude_from_navigation=False)
+    Page.objects.create(title='Page2', slug='page2', template_name='standard', exclude_from_navigation=False)
+    Page.objects.create(title='Page3', slug='page3', template_name='standard', public=False, exclude_from_navigation=False)
     resp = app.get('/', status=200)
     assert 'menu-index' in resp.text
     assert 'menu-page2' in resp.text
-- 
2.20.1

