From 30b84a21fbba0f310c82b2f4ea28462cc875cefe Mon Sep 17 00:00:00 2001 From: mattiagiupponi <51856725+mattiagiupponi@users.noreply.github.com> Date: Thu, 26 Sep 2024 18:16:38 +0200 Subject: [PATCH] [Fixes #12594] Error when saving a new map (#12595) * Fix maps creation issue * Fix maps creation issue * [Fixes #12594] Error when saing a new map * [Fixes #12594] Error when saing a new map * [Fixes #12594] Error when saing a new map * [Fixes #12594] Error when saing a new map * [Fixes #12594] Error when saing a new map * [Fixes #12594] Error when saing a new map --- geonode/base/api/serializers.py | 35 +++++++++++++++------------------ geonode/base/api/tests.py | 15 -------------- geonode/base/models.py | 3 +++ geonode/maps/api/tests.py | 25 +++++++++++++++++++++++ geonode/people/models.py | 9 +++++++++ geonode/security/utils.py | 2 +- 6 files changed, 54 insertions(+), 35 deletions(-) diff --git a/geonode/base/api/serializers.py b/geonode/base/api/serializers.py index 9a5ad98b09f..ac136d33bab 100644 --- a/geonode/base/api/serializers.py +++ b/geonode/base/api/serializers.py @@ -552,22 +552,6 @@ def to_representation(self, instance): return ret -class ResourceManagementField(serializers.BooleanField): - MAPPING = {"is_approved": "can_approve", "is_published": "can_publish", "featured": "can_feature"} - - def to_internal_value(self, data): - new_val = super().to_internal_value(data) - user = self.context["request"].user - user_action = self.MAPPING.get(self.field_name) - instance = self.root.instance or ResourceBase.objects.get(pk=self.root.initial_data["pk"]) - if getattr(user, user_action)(instance): - logger.debug("User can perform the action, the new value is returned") - return new_val - else: - logger.warning(f"The user does not have the perms to update the value of {self.field_name}") - return getattr(instance, self.field_name) - - class ResourceBaseSerializer(DynamicModelSerializer): pk = serializers.CharField(read_only=True) uuid = serializers.CharField(read_only=True) @@ -608,10 +592,10 @@ class ResourceBaseSerializer(DynamicModelSerializer): popular_count = serializers.CharField(required=False) share_count = serializers.CharField(required=False) rating = serializers.CharField(required=False) - featured = ResourceManagementField(required=False) + featured = serializers.BooleanField(required=False) advertised = serializers.BooleanField(required=False) - is_published = ResourceManagementField(required=False) - is_approved = ResourceManagementField(required=False) + is_published = serializers.BooleanField(required=False) + is_approved = serializers.BooleanField(required=False) detail_url = DetailUrlField(read_only=True) created = serializers.DateTimeField(read_only=True) last_updated = serializers.DateTimeField(read_only=True) @@ -751,6 +735,13 @@ def to_internal_value(self, data): data = super(ResourceBaseSerializer, self).to_internal_value(data) return data + def update(self, instance, validated_data): + user = self.context["request"].user + for field in instance.ROLE_BASED_MANAGED_FIELDS: + if not user.can_change_resource_field(instance, field) and field in validated_data: + validated_data.pop(field) + return super().update(instance, validated_data) + def save(self, **kwargs): extent = self.validated_data.pop("extent", None) instance = super().save(**kwargs) @@ -767,6 +758,12 @@ def save(self, **kwargs): logger.exception(e) raise InvalidResourceException("The standard bbox provided is invalid") instance.set_bbox_polygon(coords, srid) + + user = self.context["request"].user + for field in instance.ROLE_BASED_MANAGED_FIELDS: + if not user.can_change_resource_field(instance, field): + logger.debug("User can perform the action, the default value is set") + setattr(user, field, getattr(ResourceBase, field).field.default) return instance diff --git a/geonode/base/api/tests.py b/geonode/base/api/tests.py index b039a013eab..2c839503d74 100644 --- a/geonode/base/api/tests.py +++ b/geonode/base/api/tests.py @@ -826,21 +826,6 @@ def test_resource_settings_field(self): self.assertIsNotNone(field) self.assertTrue(field.to_internal_value(True)) - def test_resource_settings_field_non_admin(self): - """ - Non-Admin is not able to change the is_published value - if he is not the owner of the resource - """ - doc = create_single_doc("my_custom_doc") - factory = RequestFactory() - rq = factory.get("test") - rq.user = get_user_model().objects.get(username="bobby") - serializer = ResourceBaseSerializer(doc, context={"request": rq}) - field = serializer.fields["is_published"] - self.assertIsNotNone(field) - # the original value was true, so it should not return false - self.assertTrue(field.to_internal_value(False)) - def test_delete_user_with_resource(self): owner, created = get_user_model().objects.get_or_create(username="delet-owner") Dataset( diff --git a/geonode/base/models.py b/geonode/base/models.py index b22de4bf0a9..00297dca75b 100644 --- a/geonode/base/models.py +++ b/geonode/base/models.py @@ -632,6 +632,9 @@ class ResourceBase(PolymorphicModel, PermissionLevelMixin, ItemBase): Base Resource Object loosely based on ISO 19115:2003 """ + # fixing up the publishing option based on user permissions + ROLE_BASED_MANAGED_FIELDS = ["is_approved", "is_published", "featured"] + BASE_PERMISSIONS = { "read": ["view_resourcebase"], "write": ["change_resourcebase_metadata"], diff --git a/geonode/maps/api/tests.py b/geonode/maps/api/tests.py index ca4ab144877..32f020bda34 100644 --- a/geonode/maps/api/tests.py +++ b/geonode/maps/api/tests.py @@ -270,6 +270,31 @@ def test_create_map(self): self.assertIsNotNone(response_maplayer["dataset"]) self.assertIsNotNone(response.data["map"]["thumbnail_url"]) + def test_create_map_featured_status_admin(self): + """ + Post to maps/ + User with perms should be able to change the value in the post payload + """ + # Get Layers List (backgrounds) + url = reverse("maps-list") + + data = { + "title": "Map should be approved", + "featured": True, + "is_approved": False, + "is_published": False, + "data": DUMMY_MAPDATA, + "maplayers": DUMMY_MAPLAYERS_DATA, + } + # if has perms, the user should be able to change the field + # featured/approved/published + self.client.login(username="admin", password="admin") + response = self.client.post(f"{url}?include[]=data", data=data, format="json") + self.assertEqual(response.status_code, 201) + self.assertFalse(response.json()["map"]["is_published"]) + self.assertFalse(response.json()["map"]["is_approved"]) + self.assertTrue(response.json()["map"]["featured"]) + def test_create_map_with_extra_maplayer_info(self): """ Post to maps/ diff --git a/geonode/people/models.py b/geonode/people/models.py index b3c3804756c..d115d2c85f3 100644 --- a/geonode/people/models.py +++ b/geonode/people/models.py @@ -261,6 +261,15 @@ def send_mail(self, template_prefix, context): if self.email: get_adapter().send_mail(template_prefix, self.email, context) + def can_change_resource_field(self, resource, field): + match field: + case "is_approved": + return self.can_approve(resource) + case "is_published": + return self.can_publish(resource) + case "featured": + return self.can_feature(resource) + def can_approve(self, resource): return can_approve(self, resource) diff --git a/geonode/security/utils.py b/geonode/security/utils.py index b163e036806..f39b0b28dd3 100644 --- a/geonode/security/utils.py +++ b/geonode/security/utils.py @@ -783,6 +783,6 @@ def can_publish(user, resource): if is_superuser: return True elif AdvancedSecurityWorkflowManager.is_manager_publish_mode(): - return is_manager and can_publish + return is_manager else: return is_owner or is_manager