Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[Fixes #12594] Error when saving a new map #12595

Merged
merged 8 commits into from
Sep 26, 2024
Merged
43 changes: 24 additions & 19 deletions geonode/base/api/serializers.py
Original file line number Diff line number Diff line change
Expand Up @@ -73,6 +73,10 @@
logger = logging.getLogger(__name__)


# fixing up the publishing option based on user permissions
SETTINGS_MAPPING = {"is_approved": "can_approve", "is_published": "can_publish", "featured": "can_feature"}


def user_serializer():
import geonode.people.api.serializers as ser

Expand Down Expand Up @@ -552,22 +556,6 @@
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)
Expand Down Expand Up @@ -608,10 +596,10 @@
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)
Expand Down Expand Up @@ -751,6 +739,15 @@
data = super(ResourceBaseSerializer, self).to_internal_value(data)
return data

def update(self, instance, validated_data):
user = self.context["request"].user
for field, user_action in SETTINGS_MAPPING.items():
if not getattr(user, user_action)(instance) and field in validated_data:
# in case the user does not have the perms to do the action
# we reset the default values
validated_data.pop(field, None)

Check warning on line 748 in geonode/base/api/serializers.py

View check run for this annotation

Codecov / codecov/patch

geonode/base/api/serializers.py#L748

Added line #L748 was not covered by tests
return super().update(instance, validated_data)

def save(self, **kwargs):
extent = self.validated_data.pop("extent", None)
instance = super().save(**kwargs)
Expand All @@ -767,6 +764,14 @@
logger.exception(e)
raise InvalidResourceException("The standard bbox provided is invalid")
instance.set_bbox_polygon(coords, srid)

user = self.context["request"].user
for field, user_action in SETTINGS_MAPPING.items():
if not getattr(user, user_action)(instance):
# in case the user does not have the perms to do the action
# we reset the default values
logger.debug("User can perform the action, the default value is set")
setattr(user, field, getattr(ResourceBase, field).field.default)
return instance


Expand Down
15 changes: 0 additions & 15 deletions geonode/base/api/tests.py
Original file line number Diff line number Diff line change
Expand Up @@ -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(
Expand Down
25 changes: 25 additions & 0 deletions geonode/maps/api/tests.py
Original file line number Diff line number Diff line change
Expand Up @@ -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/
Expand Down
2 changes: 1 addition & 1 deletion geonode/security/utils.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Loading