From 3d2e62c858ce7b9b19bc048d76947f4330ee63a5 Mon Sep 17 00:00:00 2001 From: Yohan Boniface Date: Fri, 8 Sep 2023 09:39:28 +0200 Subject: [PATCH] Do not use Map.edit_status anymore But keep it for now, for data migration, and just in case --- umap/decorators.py | 15 +++++++-------- umap/forms.py | 20 +------------------- umap/static/umap/js/umap.permissions.js | 21 +-------------------- umap/templates/umap/map_table.html | 4 ++-- umap/urls.py | 17 ++++++++++------- umap/views.py | 14 ++++---------- 6 files changed, 25 insertions(+), 66 deletions(-) diff --git a/umap/decorators.py b/umap/decorators.py index c096c1f4..52faa127 100644 --- a/umap/decorators.py +++ b/umap/decorators.py @@ -26,9 +26,9 @@ def login_required_if_not_anonymous_allowed(view_func): return wrapper -def map_permissions_check(view_func): +def can_edit_map(view_func): """ - Used for URLs dealing with the map. + Used for URLs dealing with editing the map. """ @wraps(view_func) @@ -36,12 +36,11 @@ def map_permissions_check(view_func): map_inst = get_object_or_404(Map, pk=kwargs["map_id"]) user = request.user kwargs["map_inst"] = map_inst # Avoid rerequesting the map in the view - if map_inst.edit_status >= map_inst.EDITORS: - can_edit = map_inst.can_edit(user=user, request=request) - if not can_edit: - if map_inst.owner and not user.is_authenticated: - return simple_json_response(login_required=str(LOGIN_URL)) - return HttpResponseForbidden() + can_edit = map_inst.can_edit(user=user, request=request) + if not can_edit: + if map_inst.owner and not user.is_authenticated: + return simple_json_response(login_required=str(LOGIN_URL)) + return HttpResponseForbidden() return view_func(request, *args, **kwargs) return wrapper diff --git a/umap/forms.py b/umap/forms.py index af937ed7..636013eb 100644 --- a/umap/forms.py +++ b/umap/forms.py @@ -36,25 +36,7 @@ class SendLinkForm(forms.Form): class UpdateMapPermissionsForm(forms.ModelForm): class Meta: model = Map - fields = ("edit_status", "editors", "share_status", "owner") - - -class AnonymousMapPermissionsForm(forms.ModelForm): - def __init__(self, *args, **kwargs): - super(AnonymousMapPermissionsForm, self).__init__(*args, **kwargs) - help_text = _("Secret edit link is %s") % self.instance.get_anonymous_edit_url() - self.fields["edit_status"].help_text = _(help_text) - - STATUS = ( - (Map.ANONYMOUS, _("Everyone can edit")), - (Map.OWNER, _("Only editable with secret edit link")), - ) - - edit_status = forms.ChoiceField(choices=STATUS) - - class Meta: - model = Map - fields = ("edit_status",) + fields = ("editors", "share_status", "owner") class DataLayerForm(forms.ModelForm): diff --git a/umap/static/umap/js/umap.permissions.js b/umap/static/umap/js/umap.permissions.js index 13dcb750..1904d1da 100644 --- a/umap/static/umap/js/umap.permissions.js +++ b/umap/static/umap/js/umap.permissions.js @@ -5,7 +5,6 @@ L.U.MapPermissions = L.Class.extend({ owner: null, editors: [], share_status: null, - edit_status: null, }, initialize: function (map) { @@ -62,26 +61,10 @@ L.U.MapPermissions = L.Class.extend({ const helpText = L._('Secret edit link is:
{link}', { link: this.options.anonymous_edit_url, }) - fields.push([ - 'options.edit_status', - { - handler: 'IntSelect', - label: L._('Who can edit'), - selectOptions: this.map.options.anonymous_edit_statuses, - helpText: helpText, - }, - ]) + L.DomUtil.create('p', 'help-text', container, helpText) } } else { if (this.isOwner()) { - fields.push([ - 'options.edit_status', - { - handler: 'IntSelect', - label: L._('Who can edit'), - selectOptions: this.map.options.edit_statuses, - }, - ]) fields.push([ 'options.share_status', { @@ -151,8 +134,6 @@ L.U.MapPermissions = L.Class.extend({ for (let i = 0; i < this.options.editors.length; i++) formData.append('editors', this.options.editors[i].id) } - if (this.isOwner() || this.isAnonymousMap()) - formData.append('edit_status', this.options.edit_status) if (this.isOwner()) { formData.append('owner', this.options.owner && this.options.owner.id) formData.append('share_status', this.options.share_status) diff --git a/umap/templates/umap/map_table.html b/umap/templates/umap/map_table.html index eb8cef5a..2150838b 100644 --- a/umap/templates/umap/map_table.html +++ b/umap/templates/umap/map_table.html @@ -5,7 +5,7 @@ {% blocktrans %}Map{% endblocktrans %} {% blocktrans %}Name{% endblocktrans %} - {% blocktrans %}Who can see / edit{% endblocktrans %} + {% blocktrans %}Who can see{% endblocktrans %} {% blocktrans %}Last save{% endblocktrans %} {% blocktrans %}Owner{% endblocktrans %} {% blocktrans %}Actions{% endblocktrans %} @@ -19,7 +19,7 @@ {{ map_inst.name }} - {{ map_inst.get_share_status_display }} / {{ map_inst.get_edit_status_display }} + {{ map_inst.get_share_status_display }} {{ map_inst.modified_at }} {{ map_inst.owner }} diff --git a/umap/urls.py b/umap/urls.py index ee963ff7..630ea45a 100644 --- a/umap/urls.py +++ b/umap/urls.py @@ -13,7 +13,7 @@ from . import views from .decorators import ( jsonize_view, login_required_if_not_anonymous_allowed, - map_permissions_check, + can_edit_map, can_view_map, ) from .utils import decorated_patterns @@ -144,11 +144,6 @@ map_urls = [ views.DataLayerCreate.as_view(), name="datalayer_create", ), - re_path( - r"^map/(?P[\d]+)/datalayer/update/(?P\d+)/$", - views.DataLayerUpdate.as_view(), - name="datalayer_update", - ), re_path( r"^map/(?P[\d]+)/datalayer/delete/(?P\d+)/$", views.DataLayerDelete.as_view(), @@ -168,7 +163,15 @@ if settings.FROM_EMAIL: name="map_send_edit_link", ) ) -i18n_urls += decorated_patterns([map_permissions_check, never_cache], *map_urls) +datalayer_urls = [ + re_path( + r"^map/(?P[\d]+)/datalayer/update/(?P\d+)/$", + views.DataLayerUpdate.as_view(), + name="datalayer_update", + ), +] +i18n_urls += decorated_patterns([can_edit_map, never_cache], *map_urls) +i18n_urls += decorated_patterns([never_cache], *datalayer_urls) urlpatterns += i18n_patterns( re_path(r"^$", views.home, name="home"), re_path( diff --git a/umap/views.py b/umap/views.py index a07fbd56..a21f58e1 100644 --- a/umap/views.py +++ b/umap/views.py @@ -45,7 +45,6 @@ from .forms import ( DEFAULT_LATITUDE, DEFAULT_LONGITUDE, DEFAULT_CENTER, - AnonymousMapPermissionsForm, DataLayerForm, DataLayerPermissionsForm, AnonymousDataLayerPermissionsForm, @@ -460,7 +459,7 @@ class MapDetailMixin: (i, str(label)) for i, label in Map.SHARE_STATUS if i != Map.BLOCKED ], "anonymous_edit_statuses": [ - (i, str(label)) for i, label in AnonymousMapPermissionsForm.STATUS + (i, str(label)) for i, label in AnonymousDataLayerPermissionsForm.STATUS ], "umap_version": VERSION, } @@ -522,7 +521,6 @@ class MapDetailMixin: class PermissionsMixin: def get_permissions(self): permissions = {} - permissions["edit_status"] = self.object.edit_status permissions["share_status"] = self.object.share_status if self.object.owner: permissions["owner"] = { @@ -646,18 +644,12 @@ class MapUpdate(FormLessEditMixin, PermissionsMixin, UpdateView): class UpdateMapPermissions(FormLessEditMixin, UpdateView): model = Map pk_url_kwarg = "map_id" - - def get_form_class(self): - if self.object.owner: - return UpdateMapPermissionsForm - else: - return AnonymousMapPermissionsForm + form_class = UpdateMapPermissionsForm def get_form(self, form_class=None): form = super().get_form(form_class) user = self.request.user if self.object.owner and not user == self.object.owner: - del form.fields["edit_status"] del form.fields["share_status"] del form.fields["owner"] return form @@ -924,6 +916,8 @@ class DataLayerUpdate(FormLessEditMixin, GZipMixin, UpdateView): self.object = self.get_object() if self.object.map != self.kwargs["map_inst"]: return HttpResponseForbidden() + if not self.object.can_edit(user=self.request.user, request=self.request): + return HttpResponseForbidden() if not self.is_unmodified(): return HttpResponse(status=412) return super(DataLayerUpdate, self).post(request, *args, **kwargs)