From 4adaa5f730c2e0355152b4d4f0c2fe49d331c336 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Fr=C3=A9d=C3=A9ric=20P=C3=A9ters?= Date: Sun, 28 Oct 2018 13:00:43 +0100 Subject: [PATCH] general: revamp cell visibility options (#17001) --- combo/data/models.py | 42 ++++--------- combo/manager/forms.py | 61 +++++++++++++++++++ combo/manager/static/css/combo.manager.css | 4 ++ .../templates/combo/cell_visibility.html | 18 +++++- combo/manager/templates/combo/page_view.html | 3 +- combo/manager/views.py | 4 +- tests/test_manager.py | 51 +++++++++++++--- tests/test_public.py | 53 +++++++++++++++- 8 files changed, 193 insertions(+), 43 deletions(-) diff --git a/combo/data/models.py b/combo/data/models.py index 964b0c15..3430fa01 100644 --- a/combo/data/models.py +++ b/combo/data/models.py @@ -65,21 +65,22 @@ class PostException(Exception): def element_is_visible(element, user=None): - if hasattr(element, 'restricted_to_unlogged') and element.restricted_to_unlogged: - return bool(user is None or user.is_anonymous()) if element.public: + if getattr(element, 'restricted_to_unlogged', None) is True: + return (user is None or user.is_anonymous()) return True - if user is None: - return False - if user.is_anonymous(): + if user is None or user.is_anonymous(): return False if user.is_superuser: return True page_groups = element.groups.all() if not page_groups: - # user is logged in, no group restriction in place - return True - return len(set(page_groups).intersection(user.groups.all())) > 0 + groups_ok = True + else: + groups_ok = len(set(page_groups).intersection(user.groups.all())) > 0 + if getattr(element, 'restricted_to_unlogged', None) is True: + return not(groups_ok) + return groups_ok class Placeholder(object): @@ -478,6 +479,9 @@ class CellBase(six.with_metaclass(CellMeta, models.Model)): extra_css_class = models.CharField(_('Extra classes for CSS styling'), max_length=100, blank=True) public = models.BooleanField(_('Public'), default=True) + # restricted_to_unlogged is actually an invert switch, it is used for mark + # a cell as only visibile to unlogged users but also, when groups are set, + # to mark the cell as visible to all but those groups. restricted_to_unlogged = models.BooleanField( _('Restrict to unlogged users'), default=False) groups = models.ManyToManyField(Group, verbose_name=_('Groups'), blank=True) @@ -632,28 +636,6 @@ class CellBase(six.with_metaclass(CellMeta, models.Model)): return model_forms.modelform_factory(self.__class__, fields=['slug', 'extra_css_class']) - def get_visibility_form_class(self): - # generate a form with the 'public' model attribute inverted to create - # a 'Private' checkbox. - class OppositeBooleanField(forms.BooleanField): - def prepare_value(self, value): - return not value # toggle the value when loaded from the model - - def to_python(self, value): - value = super(OppositeBooleanField, self).to_python(value) - return not value - - def formfield_callback(field): - if field.name == 'public': - return OppositeBooleanField( - label=_('Restrict to logged-in users'), - required=False) - return field.formfield() - - return model_forms.modelform_factory(self.__class__, - fields=['restricted_to_unlogged', 'public', 'groups'], - formfield_callback=formfield_callback) - def get_extra_manager_context(self): return {} diff --git a/combo/manager/forms.py b/combo/manager/forms.py index bc5e024d..bbfd4820 100644 --- a/combo/manager/forms.py +++ b/combo/manager/forms.py @@ -16,6 +16,7 @@ from django import forms from django.conf import settings +from django.contrib.auth.models import Group from django.core.exceptions import ValidationError from django.template.loader import get_template, TemplateDoesNotExist from django.utils.translation import ugettext_lazy as _ @@ -99,5 +100,65 @@ class PageEditExcludeFromNavigationForm(forms.ModelForm): fields = ('exclude_from_navigation',) +def get_groups_as_choices(): + return [(x.id, x.name) for x in Group.objects.all().order_by('name')] + + +class CellVisibilityForm(forms.Form): + visibility = forms.ChoiceField( + label=_('Visibility'), + choices=[('all', _('Everybody')), + ('logged', _('Logged users')), + ('unlogged', _('Unlogged users')), + ('groups-any', _('Users with one of these groups')), + ('groups-none', _('Users with none of these groups'))]) + groups = forms.MultipleChoiceField( + label=_('Groups'), + required=False, + choices=get_groups_as_choices) + + def __init__(self, *args, **kwargs): + self.instance = instance = kwargs.pop('instance') + initial = kwargs.get('initial') + initial['groups'] = [x.id for x in instance.groups.all()] + if instance.public: + if instance.restricted_to_unlogged: + initial['visibility'] = 'unlogged' + else: + initial['visibility'] = 'all' + else: + initial['visibility'] = 'logged' + if initial['groups']: + if instance.restricted_to_unlogged: + initial['visibility'] = 'groups-none' + else: + initial['visibility'] = 'groups-any' + super(CellVisibilityForm, self).__init__(*args, **kwargs) + + def save(self): + if self.cleaned_data['visibility'] == 'all': + self.instance.public = True + self.instance.restricted_to_unlogged = False + self.instance.groups = [] + elif self.cleaned_data['visibility'] == 'logged': + self.instance.public = False + self.instance.restricted_to_unlogged = False + self.instance.groups = [] + elif self.cleaned_data['visibility'] == 'unlogged': + self.instance.public = True + self.instance.restricted_to_unlogged = True + self.instance.groups = [] + elif self.cleaned_data['visibility'] == 'groups-any': + self.instance.public = False + self.instance.restricted_to_unlogged = False + self.instance.groups = self.cleaned_data['groups'] + elif self.cleaned_data['visibility'] == 'groups-none': + self.instance.public = False + self.instance.restricted_to_unlogged = True + self.instance.groups = self.cleaned_data['groups'] + self.instance.save() + return self.instance + + class SiteImportForm(forms.Form): site_json = forms.FileField(label=_('Site Export File')) diff --git a/combo/manager/static/css/combo.manager.css b/combo/manager/static/css/combo.manager.css index 206e03aa..5ac04df9 100644 --- a/combo/manager/static/css/combo.manager.css +++ b/combo/manager/static/css/combo.manager.css @@ -83,6 +83,10 @@ div.cell h3 span.visibility-summary::before { font-family: FontAwesome; } +div.cell h3 span.visibility-summary.visibility-off::before { + content: "\f070"; /* fa-eye-slash */ + font-family: FontAwesome; +} div.cell h3 span.visibility-summary { max-width: 30%; diff --git a/combo/manager/templates/combo/cell_visibility.html b/combo/manager/templates/combo/cell_visibility.html index 81d4be6f..010ca8ef 100644 --- a/combo/manager/templates/combo/cell_visibility.html +++ b/combo/manager/templates/combo/cell_visibility.html @@ -7,7 +7,7 @@ {% block content %} -
+ {% csrf_token %} {{ form.as_p }}
@@ -18,5 +18,21 @@ {% trans 'Cancel' %} {% endif %}
+
+ {% endblock %} diff --git a/combo/manager/templates/combo/page_view.html b/combo/manager/templates/combo/page_view.html index 9705f874..fd212564 100644 --- a/combo/manager/templates/combo/page_view.html +++ b/combo/manager/templates/combo/page_view.html @@ -107,7 +107,8 @@ {{cell.get_additional_label|default_if_none:""}} {% if not cell.public %} - + {% if cell.groups.all|length %} {% for group in cell.groups.all %}{{group.name}}{% if not forloop.last %}, {% endif %}{% endfor %} {% else %} diff --git a/combo/manager/views.py b/combo/manager/views.py index 73eeb859..c4a44ed2 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, - PageEditDescriptionForm) + PageEditDescriptionForm, CellVisibilityForm) class HomepageView(ListView): @@ -412,7 +412,7 @@ class PageCellVisibilityView(PageEditCellView): template_name = 'combo/cell_visibility.html' def get_form_class(self): - return self.object.get_visibility_form_class() + return CellVisibilityForm page_cell_visibility = PageCellVisibilityView.as_view() diff --git a/tests/test_manager.py b/tests/test_manager.py index 6d05f282..6149c125 100644 --- a/tests/test_manager.py +++ b/tests/test_manager.py @@ -9,7 +9,7 @@ import mock from django.core.files.storage import default_storage from django.core.urlresolvers import reverse from django.conf import settings -from django.contrib.auth.models import User +from django.contrib.auth.models import User, Group from django.template import TemplateSyntaxError from django.test import override_settings from django.utils.http import urlencode @@ -442,21 +442,56 @@ def test_edit_cell_visibility(app, admin_user): app = login(app) resp = app.get('/manage/pages/%s/' % page.id) resp = resp.click(href='/data_textcell-%s/visibility' % cell.id) - assert resp.form['cdata_textcell-%s-restricted_to_unlogged' % cell.id].checked is False - assert resp.form['cdata_textcell-%s-public' % cell.id].checked is False - resp.form['cdata_textcell-%s-public' % cell.id].checked = True + assert resp.form['cdata_textcell-%s-visibility' % cell.id].value == 'all' + resp.form['cdata_textcell-%s-visibility' % cell.id] = 'logged' resp = resp.form.submit('submit') assert TextCell.objects.get(id=cell.id).public is False + assert TextCell.objects.get(id=cell.id).restricted_to_unlogged is False + assert TextCell.objects.get(id=cell.id).groups.count() == 0 resp = app.get('/manage/pages/%s/' % page.id) resp = resp.click(href='/data_textcell-%s/visibility' % cell.id) - assert resp.form['cdata_textcell-%s-restricted_to_unlogged' % cell.id].checked is False - assert resp.form['cdata_textcell-%s-public' % cell.id].checked is True - resp.form['cdata_textcell-%s-public' % cell.id].checked = False - resp.form['cdata_textcell-%s-restricted_to_unlogged' % cell.id].checked = True + assert resp.form['cdata_textcell-%s-visibility' % cell.id].value == 'logged' + resp.form['cdata_textcell-%s-visibility' % cell.id] = 'unlogged' resp = resp.form.submit('submit') assert TextCell.objects.get(id=cell.id).restricted_to_unlogged is True assert TextCell.objects.get(id=cell.id).public is True + assert TextCell.objects.get(id=cell.id).groups.count() == 0 + + resp = app.get('/manage/pages/%s/' % page.id) + resp = resp.click(href='/data_textcell-%s/visibility' % cell.id) + assert resp.form['cdata_textcell-%s-visibility' % cell.id].value == 'unlogged' + resp.form['cdata_textcell-%s-visibility' % cell.id] = 'all' + resp = resp.form.submit('submit') + assert TextCell.objects.get(id=cell.id).restricted_to_unlogged is False + assert TextCell.objects.get(id=cell.id).public is True + assert TextCell.objects.get(id=cell.id).groups.count() == 0 + + group1 = Group(name='A group') + group1.save() + group2 = Group(name='Another group') + group2.save() + + resp = app.get('/manage/pages/%s/' % page.id) + resp = resp.click(href='/data_textcell-%s/visibility' % cell.id) + resp.form['cdata_textcell-%s-visibility' % cell.id] = 'groups-any' + resp.form['cdata_textcell-%s-groups' % cell.id] = [group1.id] + resp = resp.form.submit('submit') + assert TextCell.objects.get(id=cell.id).restricted_to_unlogged is False + assert TextCell.objects.get(id=cell.id).public is False + assert TextCell.objects.get(id=cell.id).groups.count() == 1 + assert TextCell.objects.get(id=cell.id).groups.all()[0].name == 'A group' + + resp = app.get('/manage/pages/%s/' % page.id) + resp = resp.click(href='/data_textcell-%s/visibility' % cell.id) + resp.form['cdata_textcell-%s-visibility' % cell.id] = 'groups-none' + resp.form['cdata_textcell-%s-groups' % cell.id] = [group2.id] + resp = resp.form.submit('submit') + assert TextCell.objects.get(id=cell.id).restricted_to_unlogged is True + assert TextCell.objects.get(id=cell.id).public is False + assert TextCell.objects.get(id=cell.id).groups.count() == 1 + assert TextCell.objects.get(id=cell.id).groups.all()[0].name == 'Another group' + def test_edit_cell_options(app, admin_user): Page.objects.all().delete() diff --git a/tests/test_public.py b/tests/test_public.py index cfecf425..47942fd8 100644 --- a/tests/test_public.py +++ b/tests/test_public.py @@ -8,7 +8,7 @@ import re import os from django.conf import settings -from django.contrib.auth.models import User +from django.contrib.auth.models import User, Group from django.core.urlresolvers import reverse from django.db import connection from django.utils.http import quote @@ -25,6 +25,15 @@ pytestmark = pytest.mark.django_db from .test_manager import admin_user, login +@pytest.fixture +def normal_user(): + try: + user = User.objects.get(username='normal-user') + user.groups = [] + except User.DoesNotExist: + user = User.objects.create_user(username='normal-user', email='', password='normal-user') + return user + def test_missing_index(app): Page.objects.all().delete() resp = app.get('/', status=200) @@ -64,6 +73,48 @@ def test_page_contents_unlogged_only(app, admin_user): resp = app.get('/', status=200) assert not 'Foobar' in resp.text +def test_page_contents_group_presence(app, normal_user): + group = Group(name='plop') + group.save() + Page.objects.all().delete() + page = Page(title='Home', slug='index', template_name='standard') + page.save() + cell = TextCell(page=page, placeholder='content', text='Foobar', order=0, + public=False) + cell.save() + cell.groups = [group] + resp = app.get('/', status=200) + assert 'Foobar' not in resp.text + + app = login(app, username='normal-user', password='normal-user') + resp = app.get('/', status=200) + assert 'Foobar' not in resp.text + + normal_user.groups = [group] + resp = app.get('/', status=200) + assert 'Foobar' in resp.text + +def test_page_contents_group_absence(app, normal_user): + group = Group(name='plop') + group.save() + Page.objects.all().delete() + page = Page(title='Home', slug='index', template_name='standard') + page.save() + cell = TextCell(page=page, placeholder='content', text='Foobar', order=0, + public=False, restricted_to_unlogged=True) + cell.save() + cell.groups = [group] + resp = app.get('/', status=200) + assert 'Foobar' not in resp.text + + app = login(app, username='normal-user', password='normal-user') + resp = app.get('/', status=200) + assert 'Foobar' in resp.text + + normal_user.groups = [group] + resp = app.get('/', status=200) + assert 'Foobar' not in resp.text + def test_page_footer_acquisition(app): Page.objects.all().delete() page = Page(title='Home', slug='index', template_name='standard') -- 2.19.1