From 24b7e73d72d356bdd8e313e0e9a7c2130d3f22b3 Mon Sep 17 00:00:00 2001 From: Deepansh Mathur Date: Wed, 22 Dec 2021 20:50:39 +0530 Subject: [PATCH 01/23] NEW: endpoints for plio's settings and user's settings --- plio/views.py | 17 +++++++++++++++++ users/views.py | 17 +++++++++++++++++ 2 files changed, 34 insertions(+) diff --git a/plio/views.py b/plio/views.py index 35c2dde8..6b4b1a42 100644 --- a/plio/views.py +++ b/plio/views.py @@ -126,6 +126,23 @@ def perform_create(self, serializer): def perform_update(self, serializer): serializer.save(created_by=self.get_object().created_by) + @action( + detail=True, + permission_classes=[IsAuthenticated], + methods=["put"], + ) + def setting(self, request, uuid): + """Updates a plio's setting key inside it's config""" + plio = self.get_object() + config = plio.config or {"settings": {}} + config['settings'] = self.request.data + plio.config = config + plio.save() + return Response( + self.get_serializer(plio).data['config'], status=status.HTTP_200_OK + ) + + @action(detail=False, permission_classes=[IsAuthenticated]) def list_uuid(self, request): """Retrieves a list of UUIDs for all the plios""" diff --git a/users/views.py b/users/views.py index 441b8396..7806054e 100644 --- a/users/views.py +++ b/users/views.py @@ -45,6 +45,23 @@ class UserViewSet(viewsets.ModelViewSet): queryset = User.objects.all() serializer_class = UserSerializer + @action( + detail=True, + permission_classes=[IsAuthenticated], + methods=["put"], + ) + def setting(self, request, pk): + """Updates a user's setting key inside it's config""" + user = self.get_object() + if 'settings' not in user.config: + user.config['settings'] = {} + + user.config['settings'] = self.request.data + user.save() + return Response( + self.get_serializer(user).data['config'], status=status.HTTP_200_OK + ) + @action( detail=True, methods=["patch", "get"], From 7aa074e0fc537a5991a00a47e85fccc64a8411f0 Mon Sep 17 00:00:00 2001 From: Deepansh Mathur Date: Wed, 22 Dec 2021 21:35:40 +0530 Subject: [PATCH 02/23] FIX: pre commit fixes --- plio/views.py | 8 ++++---- users/views.py | 14 +++++++------- 2 files changed, 11 insertions(+), 11 deletions(-) diff --git a/plio/views.py b/plio/views.py index 96b887a1..04973af8 100644 --- a/plio/views.py +++ b/plio/views.py @@ -129,19 +129,19 @@ def perform_update(self, serializer): serializer.save(created_by=self.get_object().created_by) @action( - detail=True, - permission_classes=[IsAuthenticated], + detail=True, + permission_classes=[IsAuthenticated], methods=["put"], ) def setting(self, request, uuid): """Updates a plio's setting key inside it's config""" plio = self.get_object() config = plio.config or {"settings": {}} - config['settings'] = self.request.data + config["settings"] = self.request.data plio.config = config plio.save() return Response( - self.get_serializer(plio).data['config'], status=status.HTTP_200_OK + self.get_serializer(plio).data["config"], status=status.HTTP_200_OK ) @property diff --git a/users/views.py b/users/views.py index 7806054e..f545f297 100644 --- a/users/views.py +++ b/users/views.py @@ -46,20 +46,20 @@ class UserViewSet(viewsets.ModelViewSet): serializer_class = UserSerializer @action( - detail=True, - permission_classes=[IsAuthenticated], + detail=True, + permission_classes=[IsAuthenticated], methods=["put"], ) def setting(self, request, pk): """Updates a user's setting key inside it's config""" user = self.get_object() - if 'settings' not in user.config: - user.config['settings'] = {} - - user.config['settings'] = self.request.data + if "settings" not in user.config: + user.config["settings"] = {} + + user.config["settings"] = self.request.data user.save() return Response( - self.get_serializer(user).data['config'], status=status.HTTP_200_OK + self.get_serializer(user).data["config"], status=status.HTTP_200_OK ) @action( From c300dc6445f5227a8f03863f1c47fc86f5f8da09 Mon Sep 17 00:00:00 2001 From: Deepansh Mathur Date: Thu, 23 Dec 2021 14:30:55 +0530 Subject: [PATCH 03/23] plio settings test --- plio/tests.py | 21 +++++++++++++++++++++ 1 file changed, 21 insertions(+) diff --git a/plio/tests.py b/plio/tests.py index 8b1e3a6f..14db7db2 100644 --- a/plio/tests.py +++ b/plio/tests.py @@ -541,6 +541,27 @@ def test_updating_plio_recreates_instance_cache(self): self.assertEqual(len(cache.keys(cache_key_name)), 1) self.assertEqual(cache.get(cache_key_name)["name"], new_name) + def test_user_can_update_own_plio_settings(self): + test_settings = json.dumps( + {"player": {"configuration": {"skipEnabled": False}}} + ) + # update settings for plio_1 + response = self.client.put( + f"/api/v1/plios/{self.plio_1.uuid}/setting/", + json.loads(test_settings), + format="json", + ) + + # 200 OK returned as status + self.assertEqual(response.status_code, status.HTTP_200_OK) + # The plio should contain the new updated settings object + self.assertEqual( + Plio.objects.filter(uuid=self.plio_1.uuid) + .first() + .config["settings"]["player"], + json.loads(test_settings)["player"], + ) + class PlioDownloadTestCase(BaseTestCase): def setUp(self): From 0e3b96acb89c26126b6801144cb6395cb249744d Mon Sep 17 00:00:00 2001 From: Deepansh Mathur Date: Thu, 23 Dec 2021 15:08:05 +0530 Subject: [PATCH 04/23] Test cases fixed --- plio/tests.py | 69 ++++++++++++++++++++++++++++++++++++++++++++++++++ plio/views.py | 2 +- users/tests.py | 57 +++++++++++++++++++++++++++++++++++++++++ users/views.py | 2 +- 4 files changed, 128 insertions(+), 2 deletions(-) diff --git a/plio/tests.py b/plio/tests.py index 14db7db2..4d0aff9a 100644 --- a/plio/tests.py +++ b/plio/tests.py @@ -562,6 +562,75 @@ def test_user_can_update_own_plio_settings(self): json.loads(test_settings)["player"], ) + def test_user_cannot_update_other_user_plio_settings(self): + test_settings = json.dumps( + {"player": {"configuration": {"skipEnabled": False}}} + ) + # user_2 creates a plio + plio = Plio.objects.create( + name="Plio_by_user_2", video=self.video, created_by=self.user_2 + ) + + # user_1 tries updating the settings for the above created plio + response = self.client.put( + f"/api/v1/plios/{plio.uuid}/setting/", + json.loads(test_settings), + format="json", + ) + + # Updating other user's plio's settings should be forbidden + self.assertEqual(response.status_code, status.HTTP_403_FORBIDDEN) + + def test_org_members_can_update_org_plio_settings(self): + test_settings = json.dumps( + {"player": {"configuration": {"skipEnabled": False}}} + ) + + # add users to organization + OrganizationUser.objects.create( + organization=self.organization, user=self.user, role=self.org_view_role + ) + + OrganizationUser.objects.create( + organization=self.organization, user=self.user_2, role=self.org_view_role + ) + + # set db connection to organization schema + connection.set_schema(self.organization.schema_name) + + # create video in the org workspace + video_org = Video.objects.create( + title="Video 1", url="https://www.youtube.com/watch?v=vnISjBbrMUM" + ) + + # create plio within the org workspace by user 2 + plio_org = Plio.objects.create( + name="Plio 1", video=video_org, created_by=self.user_2 + ) + + # set organization in request and access token for user 1 + self.client.credentials( + HTTP_ORGANIZATION=self.organization.shortcode, + HTTP_AUTHORIZATION="Bearer " + self.access_token.token, + ) + + # user_1 tries to update the org plio's settings which was created + # by user_2 + response = self.client.put( + f"/api/v1/plios/{plio_org.uuid}/setting/", + json.loads(test_settings), + format="json", + ) + + # updating an org plio's settings should be possible by any org member + self.assertEqual(response.status_code, status.HTTP_200_OK) + self.assertEqual( + Plio.objects.filter(uuid=plio_org.uuid) + .first() + .config["settings"]["player"], + json.loads(test_settings)["player"], + ) + class PlioDownloadTestCase(BaseTestCase): def setUp(self): diff --git a/plio/views.py b/plio/views.py index 04973af8..8b3522f2 100644 --- a/plio/views.py +++ b/plio/views.py @@ -130,7 +130,7 @@ def perform_update(self, serializer): @action( detail=True, - permission_classes=[IsAuthenticated], + permission_classes=[IsAuthenticated, PlioPermission], methods=["put"], ) def setting(self, request, uuid): diff --git a/users/tests.py b/users/tests.py index 0ed3ca63..8db214fe 100644 --- a/users/tests.py +++ b/users/tests.py @@ -346,6 +346,63 @@ def test_updating_user_recreates_instance_cache(self): self.assertEqual(len(cache.keys(cache_key_name)), 1) self.assertEqual(cache.get(cache_key_name)["first_name"], first_name) + def test_user_can_update_its_own_settings(self): + test_settings = json.dumps( + { + "player": {"configuration": {"skipEnabled": False}}, + "app": {"appearance": {"darkMode": True}}, + } + ) + # update settings for user_1 + response = self.client.put( + f"/api/v1/users/{self.user_1.id}/setting/", + json.loads(test_settings), + format="json", + ) + + # 200 OK returned as status + self.assertEqual(response.status_code, status.HTTP_200_OK) + # The plio should contain the new updated settings object + self.assertEqual( + User.objects.filter(id=self.user_1.id).first().config["settings"], + json.loads(test_settings), + ) + + def test_only_superuser_can_update_other_user_settings(self): + test_settings = json.dumps( + { + "player": {"configuration": {"skipEnabled": False}}, + "app": {"appearance": {"darkMode": True}}, + } + ) + # try to update settings for user_2 + response = self.client.put( + f"/api/v1/users/{self.user_2.id}/setting/", + json.loads(test_settings), + format="json", + ) + + # Forbidden action + self.assertEqual(response.status_code, status.HTTP_403_FORBIDDEN) + + # make user 1 the superuser + self.user.is_superuser = True + self.user.save() + + # try the request again + response = self.client.put( + f"/api/v1/users/{self.user_2.id}/setting/", + json.loads(test_settings), + format="json", + ) + + # 200 OK returned as status + self.assertEqual(response.status_code, status.HTTP_200_OK) + self.assertEqual( + User.objects.filter(id=self.user_2.id).first().config["settings"], + json.loads(test_settings), + ) + class UserMetaTestCase(BaseTestCase): def setUp(self): diff --git a/users/views.py b/users/views.py index f545f297..0a891a28 100644 --- a/users/views.py +++ b/users/views.py @@ -47,7 +47,7 @@ class UserViewSet(viewsets.ModelViewSet): @action( detail=True, - permission_classes=[IsAuthenticated], + permission_classes=[IsAuthenticated, UserPermission], methods=["put"], ) def setting(self, request, pk): From 0533d11c09525e3df2e73240fdd3e9013cdb7e6e Mon Sep 17 00:00:00 2001 From: Deepansh Mathur Date: Thu, 23 Dec 2021 15:22:35 +0530 Subject: [PATCH 05/23] Test cases fixed --- users/tests.py | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/users/tests.py b/users/tests.py index 8db214fe..2a9720de 100644 --- a/users/tests.py +++ b/users/tests.py @@ -353,9 +353,9 @@ def test_user_can_update_its_own_settings(self): "app": {"appearance": {"darkMode": True}}, } ) - # update settings for user_1 + # update settings for user 1 response = self.client.put( - f"/api/v1/users/{self.user_1.id}/setting/", + f"/api/v1/users/{self.user.id}/setting/", json.loads(test_settings), format="json", ) @@ -364,7 +364,7 @@ def test_user_can_update_its_own_settings(self): self.assertEqual(response.status_code, status.HTTP_200_OK) # The plio should contain the new updated settings object self.assertEqual( - User.objects.filter(id=self.user_1.id).first().config["settings"], + User.objects.filter(id=self.user.id).first().config["settings"], json.loads(test_settings), ) From e8fc7207912ad6008e167502ce131f908d4a6757 Mon Sep 17 00:00:00 2001 From: Deepansh Mathur Date: Thu, 23 Dec 2021 16:14:45 +0530 Subject: [PATCH 06/23] Test cases fixed --- plio/tests.py | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/plio/tests.py b/plio/tests.py index 4d0aff9a..416c0edf 100644 --- a/plio/tests.py +++ b/plio/tests.py @@ -502,13 +502,18 @@ def test_items_sorted_with_time(self): self.assertEqual(response.data["items"][1]["id"], item_1.id) def test_retrieving_plio_sets_instance_cache(self): + # create a third plio + plio_3 = Plio.objects.create( + name="Plio 3", video=self.video, created_by=self.user + ) + # verify cache data doesn't exist - cache_key_name = get_cache_key(self.plio_1) + cache_key_name = get_cache_key(plio_3) self.assertEqual(len(cache.keys(cache_key_name)), 0) # make a get request response = self.client.get( - reverse("plios-detail", kwargs={"uuid": self.plio_1.uuid}) + reverse("plios-detail", kwargs={"uuid": plio_3.uuid}) ) # verify cache data exists From cac68ef14ecc6de60903aa076267e82226106cae Mon Sep 17 00:00:00 2001 From: Deepansh Mathur Date: Thu, 23 Dec 2021 17:43:44 +0530 Subject: [PATCH 07/23] Test cases fixed finally --- plio/tests.py | 3 +++ 1 file changed, 3 insertions(+) diff --git a/plio/tests.py b/plio/tests.py index 416c0edf..c21e8d69 100644 --- a/plio/tests.py +++ b/plio/tests.py @@ -636,6 +636,9 @@ def test_org_members_can_update_org_plio_settings(self): json.loads(test_settings)["player"], ) + # set db connection back to public (default) schema + connection.set_schema_to_public() + class PlioDownloadTestCase(BaseTestCase): def setUp(self): From 84719843dd5a61cabeee520c13b3d7b9d073e34b Mon Sep 17 00:00:00 2001 From: Deepansh Mathur Date: Fri, 14 Jan 2022 15:11:34 +0530 Subject: [PATCH 08/23] org config and settings endpoint --- .../migrations/0004_organization_config.py | 18 +++++++++++++++++ organizations/models.py | 1 + organizations/permissions.py | 10 +++++++++- organizations/serializers.py | 1 + organizations/views.py | 20 ++++++++++++++++++- users/serializers.py | 12 +++++++++++ 6 files changed, 60 insertions(+), 2 deletions(-) create mode 100644 organizations/migrations/0004_organization_config.py diff --git a/organizations/migrations/0004_organization_config.py b/organizations/migrations/0004_organization_config.py new file mode 100644 index 00000000..d5d58d85 --- /dev/null +++ b/organizations/migrations/0004_organization_config.py @@ -0,0 +1,18 @@ +# Generated by Django 3.1.1 on 2022-01-12 04:57 + +from django.db import migrations, models + + +class Migration(migrations.Migration): + + dependencies = [ + ('organizations', '0003_organization_api_key'), + ] + + operations = [ + migrations.AddField( + model_name='organization', + name='config', + field=models.JSONField(null=True), + ), + ] diff --git a/organizations/models.py b/organizations/models.py index 7897935d..2daebcfb 100644 --- a/organizations/models.py +++ b/organizations/models.py @@ -12,6 +12,7 @@ class Organization(TenantMixin, SafeDeleteModel): name = models.CharField(max_length=255) shortcode = models.SlugField() api_key = models.CharField(null=True, max_length=20) + config = models.JSONField(null=True) created_at = models.DateTimeField(auto_now_add=True) updated_at = models.DateTimeField(auto_now=True) diff --git a/organizations/permissions.py b/organizations/permissions.py index bbdd5aa4..9cafc31e 100644 --- a/organizations/permissions.py +++ b/organizations/permissions.py @@ -8,4 +8,12 @@ class OrganizationPermission(permissions.BasePermission): def has_permission(self, request, view): """View-level permissions for organization. This determines whether the request can access organization instances or not.""" - return request.user.is_superuser + return request.user.is_superuser or request.user.is_org_admin( + organization_id=int(view.kwargs["pk"]) + ) + + def has_object_permission(self, request, view, obj): + """Object-level permissions for an organization. This determines whether the request can access an organization instance or not.""" + return request.user.is_superuser or request.user.is_org_admin( + organization_id=int(view.kwargs["pk"]) + ) diff --git a/organizations/serializers.py b/organizations/serializers.py index 50b85a3d..2dad47cc 100644 --- a/organizations/serializers.py +++ b/organizations/serializers.py @@ -11,6 +11,7 @@ class Meta: "name", "shortcode", "api_key", + "config", "created_at", "updated_at", ] diff --git a/organizations/views.py b/organizations/views.py index 8cb5657f..dc872a79 100644 --- a/organizations/views.py +++ b/organizations/views.py @@ -1,7 +1,9 @@ -from rest_framework import viewsets +from rest_framework import viewsets, status from organizations.models import Organization from organizations.serializers import OrganizationSerializer from rest_framework.permissions import IsAuthenticated +from rest_framework.response import Response +from rest_framework.decorators import action from organizations.permissions import OrganizationPermission @@ -20,3 +22,19 @@ class OrganizationViewSet(viewsets.ModelViewSet): permission_classes = [IsAuthenticated, OrganizationPermission] queryset = Organization.objects.all() serializer_class = OrganizationSerializer + + @action( + detail=True, + permission_classes=[IsAuthenticated, OrganizationPermission], + methods=["put"], + ) + def setting(self, request, pk): + """Updates an org's setting key inside it's config""" + org = self.get_object() + config = org.config or {"settings": {}} + config["settings"] = self.request.data + org.config = config + org.save() + return Response( + self.get_serializer(org).data["config"], status=status.HTTP_200_OK + ) diff --git a/users/serializers.py b/users/serializers.py index ed317ff6..459ea0ee 100644 --- a/users/serializers.py +++ b/users/serializers.py @@ -40,9 +40,21 @@ def to_representation(self, instance): return cached_response response = super().to_representation(instance) + # add organizations the user is a part of to the response response["organizations"] = OrganizationSerializer( instance.organizations, many=True ).data + # for each organization the user is part of, add the user's role in + # that organization + for org in response["organizations"]: + org_user_instance = OrganizationUser.objects.filter( + user=instance, organization_id=org["id"] + ).first() + role_id = OrganizationUserSerializer(org_user_instance).data["role"] + role_name = RoleSerializer(Role.objects.filter(id=role_id).first()).data[ + "name" + ] + org.update({"role": role_name}) cache.set(cache_key, response) # set a cached version return response From 3383e280bb0716c78a22eb65814688d7f0a2d637 Mon Sep 17 00:00:00 2001 From: Deepansh Mathur Date: Sat, 15 Jan 2022 14:41:16 +0530 Subject: [PATCH 09/23] pre commit errors --- organizations/migrations/0004_organization_config.py | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/organizations/migrations/0004_organization_config.py b/organizations/migrations/0004_organization_config.py index d5d58d85..ac3206a6 100644 --- a/organizations/migrations/0004_organization_config.py +++ b/organizations/migrations/0004_organization_config.py @@ -6,13 +6,13 @@ class Migration(migrations.Migration): dependencies = [ - ('organizations', '0003_organization_api_key'), + ("organizations", "0003_organization_api_key"), ] operations = [ migrations.AddField( - model_name='organization', - name='config', + model_name="organization", + name="config", field=models.JSONField(null=True), ), ] From fb56f644724b13c846da6c462adb6b34a2a8e2c6 Mon Sep 17 00:00:00 2001 From: Deepansh Mathur Date: Sat, 15 Jan 2022 15:15:55 +0530 Subject: [PATCH 10/23] tests fix --- organizations/permissions.py | 18 ++++++++++++------ 1 file changed, 12 insertions(+), 6 deletions(-) diff --git a/organizations/permissions.py b/organizations/permissions.py index 9cafc31e..dc97bda8 100644 --- a/organizations/permissions.py +++ b/organizations/permissions.py @@ -8,12 +8,18 @@ class OrganizationPermission(permissions.BasePermission): def has_permission(self, request, view): """View-level permissions for organization. This determines whether the request can access organization instances or not.""" - return request.user.is_superuser or request.user.is_org_admin( - organization_id=int(view.kwargs["pk"]) - ) + if view.action == "update": + org_to_update = int(view.kwargs["pk"]) + return request.user.is_superuser or request.user.is_org_admin( + organization_id=org_to_update + ) + return request.user.is_superuser def has_object_permission(self, request, view, obj): """Object-level permissions for an organization. This determines whether the request can access an organization instance or not.""" - return request.user.is_superuser or request.user.is_org_admin( - organization_id=int(view.kwargs["pk"]) - ) + if view.action == "update": + org_to_update = int(view.kwargs["pk"]) + return request.user.is_superuser or request.user.is_org_admin( + organization_id=org_to_update + ) + return request.user.is_superuser From 406068714fbe0ae9b9b1c224524cc680a679f38b Mon Sep 17 00:00:00 2001 From: Deepansh Mathur Date: Fri, 21 Jan 2022 14:09:01 +0530 Subject: [PATCH 11/23] permissions bug fixed --- organizations/permissions.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/organizations/permissions.py b/organizations/permissions.py index dc97bda8..10696c89 100644 --- a/organizations/permissions.py +++ b/organizations/permissions.py @@ -8,7 +8,7 @@ class OrganizationPermission(permissions.BasePermission): def has_permission(self, request, view): """View-level permissions for organization. This determines whether the request can access organization instances or not.""" - if view.action == "update": + if view.action in ["setting"]: org_to_update = int(view.kwargs["pk"]) return request.user.is_superuser or request.user.is_org_admin( organization_id=org_to_update @@ -17,7 +17,7 @@ def has_permission(self, request, view): def has_object_permission(self, request, view, obj): """Object-level permissions for an organization. This determines whether the request can access an organization instance or not.""" - if view.action == "update": + if view.action in ["setting"]: org_to_update = int(view.kwargs["pk"]) return request.user.is_superuser or request.user.is_org_admin( organization_id=org_to_update From 3a65d78098d95212583ed7f0efd2f8d7ee46b0cd Mon Sep 17 00:00:00 2001 From: Deepansh Mathur Date: Fri, 21 Jan 2022 14:13:07 +0530 Subject: [PATCH 12/23] pre commit errors --- plio/tests.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/plio/tests.py b/plio/tests.py index 8424d7dc..72ba226f 100644 --- a/plio/tests.py +++ b/plio/tests.py @@ -636,7 +636,7 @@ def test_org_members_can_update_org_plio_settings(self): .config["settings"]["player"], json.loads(test_settings)["player"], ) - + # set db connection back to public (default) schema connection.set_schema_to_public() @@ -755,7 +755,7 @@ def test_copying_to_workspace_with_no_video(self): ) self.assertEqual(response.status_code, status.HTTP_200_OK) self.assertEqual(response.data["video"]["url"], "") - + # set db connection back to public (default) schema connection.set_schema_to_public() From fc6e51a4323450c03cd174ed57e665974dc392b0 Mon Sep 17 00:00:00 2001 From: Deepansh Mathur Date: Fri, 21 Jan 2022 15:45:29 +0530 Subject: [PATCH 13/23] Tests for org settings --- organizations/tests.py | 127 +++++++++++++++++++++++++++++++++++++++++ 1 file changed, 127 insertions(+) diff --git a/organizations/tests.py b/organizations/tests.py index d9017352..c1bb6573 100644 --- a/organizations/tests.py +++ b/organizations/tests.py @@ -91,3 +91,130 @@ def test_updating_organization_recreates_user_instance_cache(self): self.assertEqual( cache.get(cache_key_name)["organizations"][0]["name"], org_new_name ) + + def test_settings_support_only_put_method(self): + """ /settings/ action only supports PUT method """ + # some dummy settings + dummy_settings = {"setting_name": "setting_value"} + + # make the current user a superuser + self.user.is_superuser = True + self.user.save() + + # try to list the settings + response = self.client.get( + f"/api/v1/organizations/{self.organization_1.id}/setting/" + ) + self.assertEqual(response.status_code, status.HTTP_405_METHOD_NOT_ALLOWED) + + # try a POST request to settings + response = self.client.post( + f"/api/v1/organizations/{self.organization_1.id}/setting/", dummy_settings + ) + self.assertEqual(response.status_code, status.HTTP_405_METHOD_NOT_ALLOWED) + + # try a PATCH request to settings + response = self.client.patch( + f"/api/v1/organizations/{self.organization_1.id}/setting/", dummy_settings + ) + self.assertEqual(response.status_code, status.HTTP_405_METHOD_NOT_ALLOWED) + + # try a PUT request to settings + response = self.client.put( + f"/api/v1/organizations/{self.organization_1.id}/setting/", dummy_settings + ) + self.assertEqual(response.status_code, status.HTTP_200_OK) + self.assertEqual( + Organization.objects.filter(id=self.organization_1.id) + .first() + .config["settings"], + dummy_settings, + ) + + def test_superuser_can_update_any_org_settings(self): + """A superuse is allowed to update settings of any org""" + # some dummy settings + dummy_settings = {"setting_name": "setting_value"} + + # turn the current user into a superuser + self.user.is_superuser = True + self.user.save() + + # try updating settings of org 1 + response = self.client.put( + f"/api/v1/organizations/{self.organization_1.id}/setting/", dummy_settings + ) + self.assertEqual(response.status_code, status.HTTP_200_OK) + self.assertEqual( + Organization.objects.filter(id=self.organization_1.id) + .first() + .config["settings"], + dummy_settings, + ) + + # try updating settings of org 2 + response = self.client.put( + f"/api/v1/organizations/{self.organization_2.id}/setting/", dummy_settings + ) + self.assertEqual(response.status_code, status.HTTP_200_OK) + self.assertEqual( + Organization.objects.filter(id=self.organization_2.id) + .first() + .config["settings"], + dummy_settings, + ) + + def test_org_admin_can_update_own_org_settings_only(self): + """Only an admin of an org can update the org's settings""" + from rest_framework.test import APIClient + from users.models import User + from plio.tests import get_new_access_token + + # some dummy settings + dummy_settings = {"setting_name": "setting_value"} + + # user should NOT be able to update org 1 settings as + # the user is not an org admin + response = self.client.put( + f"/api/v1/organizations/{self.organization_1.id}/setting/", dummy_settings + ) + self.assertEqual(response.status_code, status.HTTP_403_FORBIDDEN) + + # create a new super user + superuser_client = APIClient() + superuser = User.objects.create(mobile="+919988776655", is_superuser=True) + superuser_access_token = get_new_access_token(superuser, self.application) + superuser_client.credentials( + HTTP_AUTHORIZATION="Bearer " + superuser_access_token.token + ) + + # Make the current user org admin for organization 1 (using the created super user above) + superuser_client.post( + reverse("organization-users-list"), + { + "user": self.user.id, + "organization": self.organization_1.id, + "role": self.org_admin_role.id, + }, + ) + + # user should be able to update org 1 settings + response = self.client.put( + f"/api/v1/organizations/{self.organization_1.id}/setting/", dummy_settings + ) + self.assertEqual(response.status_code, status.HTTP_200_OK) + self.assertEqual( + Organization.objects.filter(id=self.organization_1.id) + .first() + .config["settings"], + dummy_settings, + ) + + # but the user still should NOT be able to update settings for org 2 + response = self.client.put( + f"/api/v1/organizations/{self.organization_2.id}/setting/", dummy_settings + ) + self.assertEqual(response.status_code, status.HTTP_403_FORBIDDEN) + self.assertEqual( + Organization.objects.filter(id=self.organization_2.id).first().config, None + ) From 32c5205827a94945cef5eef745781f7f7753b833 Mon Sep 17 00:00:00 2001 From: Deepansh Mathur Date: Thu, 27 Jan 2022 16:06:40 +0530 Subject: [PATCH 14/23] Update organizations/permissions.py Co-authored-by: Aman Dalmia --- organizations/permissions.py | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/organizations/permissions.py b/organizations/permissions.py index 10696c89..e2d54897 100644 --- a/organizations/permissions.py +++ b/organizations/permissions.py @@ -18,8 +18,7 @@ def has_permission(self, request, view): def has_object_permission(self, request, view, obj): """Object-level permissions for an organization. This determines whether the request can access an organization instance or not.""" if view.action in ["setting"]: - org_to_update = int(view.kwargs["pk"]) return request.user.is_superuser or request.user.is_org_admin( - organization_id=org_to_update + organization_id=int(view.kwargs["pk"]) ) return request.user.is_superuser From cfc79753d0f7e8e62d0d4e8a3c896e833fcecff5 Mon Sep 17 00:00:00 2001 From: Deepansh Mathur Date: Fri, 28 Jan 2022 10:48:29 +0530 Subject: [PATCH 15/23] some feedback --- organizations/tests.py | 2 -- 1 file changed, 2 deletions(-) diff --git a/organizations/tests.py b/organizations/tests.py index c1bb6573..ae8e31ea 100644 --- a/organizations/tests.py +++ b/organizations/tests.py @@ -93,7 +93,6 @@ def test_updating_organization_recreates_user_instance_cache(self): ) def test_settings_support_only_put_method(self): - """ /settings/ action only supports PUT method """ # some dummy settings dummy_settings = {"setting_name": "setting_value"} @@ -132,7 +131,6 @@ def test_settings_support_only_put_method(self): ) def test_superuser_can_update_any_org_settings(self): - """A superuse is allowed to update settings of any org""" # some dummy settings dummy_settings = {"setting_name": "setting_value"} From a874df640bbe6b88d0f834771632e4e2bdf5007d Mon Sep 17 00:00:00 2001 From: Deepansh Mathur Date: Fri, 28 Jan 2022 11:56:00 +0530 Subject: [PATCH 16/23] review feedback --- organizations/views.py | 4 ++-- plio/tests.py | 33 ++++++++++++--------------------- plio/views.py | 4 ++-- users/serializers.py | 9 +++------ users/tests.py | 35 ++++++++++++++++------------------- users/views.py | 3 --- 6 files changed, 35 insertions(+), 53 deletions(-) diff --git a/organizations/views.py b/organizations/views.py index dc872a79..ccaf5e13 100644 --- a/organizations/views.py +++ b/organizations/views.py @@ -29,9 +29,9 @@ class OrganizationViewSet(viewsets.ModelViewSet): methods=["put"], ) def setting(self, request, pk): - """Updates an org's setting key inside it's config""" + """Updates an org's settings key inside it's config""" org = self.get_object() - config = org.config or {"settings": {}} + config = org.config if org.config is not None else {"settings": {}} config["settings"] = self.request.data org.config = config org.save() diff --git a/plio/tests.py b/plio/tests.py index a117f00f..59da90a0 100644 --- a/plio/tests.py +++ b/plio/tests.py @@ -530,18 +530,14 @@ def test_items_sorted_with_time(self): self.assertEqual(response.data["items"][1]["id"], item_1.id) def test_retrieving_plio_sets_instance_cache(self): - # create a third plio - plio_3 = Plio.objects.create( - name="Plio 3", video=self.video, created_by=self.user - ) # verify cache data doesn't exist - cache_key_name = get_cache_key(plio_3) + cache_key_name = get_cache_key(self.plio_1) self.assertEqual(len(cache.keys(cache_key_name)), 0) # make a get request response = self.client.get( - reverse("plios-detail", kwargs={"uuid": plio_3.uuid}) + reverse("plios-detail", kwargs={"uuid": self.plio_1.uuid}) ) # verify cache data exists @@ -575,13 +571,12 @@ def test_updating_plio_recreates_instance_cache(self): self.assertEqual(cache.get(cache_key_name)["name"], new_name) def test_user_can_update_own_plio_settings(self): - test_settings = json.dumps( - {"player": {"configuration": {"skipEnabled": False}}} - ) + test_settings = {"player": {"configuration": {"skipEnabled": False}}} + # update settings for plio_1 response = self.client.put( f"/api/v1/plios/{self.plio_1.uuid}/setting/", - json.loads(test_settings), + test_settings, format="json", ) @@ -592,13 +587,11 @@ def test_user_can_update_own_plio_settings(self): Plio.objects.filter(uuid=self.plio_1.uuid) .first() .config["settings"]["player"], - json.loads(test_settings)["player"], + test_settings["player"], ) def test_user_cannot_update_other_user_plio_settings(self): - test_settings = json.dumps( - {"player": {"configuration": {"skipEnabled": False}}} - ) + test_settings = {"player": {"configuration": {"skipEnabled": False}}} # user_2 creates a plio plio = Plio.objects.create( name="Plio_by_user_2", video=self.video, created_by=self.user_2 @@ -607,17 +600,15 @@ def test_user_cannot_update_other_user_plio_settings(self): # user_1 tries updating the settings for the above created plio response = self.client.put( f"/api/v1/plios/{plio.uuid}/setting/", - json.loads(test_settings), + test_settings, format="json", ) - # Updating other user's plio's settings should be forbidden + # updating other user's plio's settings should be forbidden self.assertEqual(response.status_code, status.HTTP_403_FORBIDDEN) def test_org_members_can_update_org_plio_settings(self): - test_settings = json.dumps( - {"player": {"configuration": {"skipEnabled": False}}} - ) + test_settings = {"player": {"configuration": {"skipEnabled": False}}} # add users to organization OrganizationUser.objects.create( @@ -651,7 +642,7 @@ def test_org_members_can_update_org_plio_settings(self): # by user_2 response = self.client.put( f"/api/v1/plios/{plio_org.uuid}/setting/", - json.loads(test_settings), + test_settings, format="json", ) @@ -661,7 +652,7 @@ def test_org_members_can_update_org_plio_settings(self): Plio.objects.filter(uuid=plio_org.uuid) .first() .config["settings"]["player"], - json.loads(test_settings)["player"], + test_settings["player"], ) # set db connection back to public (default) schema diff --git a/plio/views.py b/plio/views.py index 3215d4ae..7858a5ce 100644 --- a/plio/views.py +++ b/plio/views.py @@ -155,9 +155,9 @@ def perform_update(self, serializer): methods=["put"], ) def setting(self, request, uuid): - """Updates a plio's setting key inside it's config""" + """Updates a plio's settings""" plio = self.get_object() - config = plio.config or {"settings": {}} + config = plio.config if plio.config is not None else {"settings": {}} config["settings"] = self.request.data plio.config = config plio.save() diff --git a/users/serializers.py b/users/serializers.py index 459ea0ee..7c3c71a0 100644 --- a/users/serializers.py +++ b/users/serializers.py @@ -40,20 +40,17 @@ def to_representation(self, instance): return cached_response response = super().to_representation(instance) - # add organizations the user is a part of to the response + # add organizations the user is a part of response["organizations"] = OrganizationSerializer( instance.organizations, many=True ).data # for each organization the user is part of, add the user's role in # that organization for org in response["organizations"]: - org_user_instance = OrganizationUser.objects.filter( + org_user = OrganizationUser.objects.filter( user=instance, organization_id=org["id"] ).first() - role_id = OrganizationUserSerializer(org_user_instance).data["role"] - role_name = RoleSerializer(Role.objects.filter(id=role_id).first()).data[ - "name" - ] + role_name = Role.objects.filter(id=org_user.role.id).first().name org.update({"role": role_name}) cache.set(cache_key, response) # set a cached version diff --git a/users/tests.py b/users/tests.py index 5156618a..dec6ae36 100644 --- a/users/tests.py +++ b/users/tests.py @@ -358,42 +358,39 @@ def test_updating_user_recreates_instance_cache(self): self.assertEqual(cache.get(cache_key_name)["first_name"], first_name) def test_user_can_update_its_own_settings(self): - test_settings = json.dumps( - { - "player": {"configuration": {"skipEnabled": False}}, - "app": {"appearance": {"darkMode": True}}, - } - ) + test_settings = { + "player": {"configuration": {"skipEnabled": False}}, + "app": {"appearance": {"darkMode": True}}, + } # update settings for user 1 response = self.client.put( f"/api/v1/users/{self.user.id}/setting/", - json.loads(test_settings), + test_settings, format="json", ) # 200 OK returned as status self.assertEqual(response.status_code, status.HTTP_200_OK) - # The plio should contain the new updated settings object + # the plio should contain the new updated settings object self.assertEqual( User.objects.filter(id=self.user.id).first().config["settings"], - json.loads(test_settings), + test_settings, ) def test_only_superuser_can_update_other_user_settings(self): - test_settings = json.dumps( - { - "player": {"configuration": {"skipEnabled": False}}, - "app": {"appearance": {"darkMode": True}}, - } - ) + test_settings = { + "player": {"configuration": {"skipEnabled": False}}, + "app": {"appearance": {"darkMode": True}}, + } + # try to update settings for user_2 response = self.client.put( f"/api/v1/users/{self.user_2.id}/setting/", - json.loads(test_settings), + test_settings, format="json", ) - # Forbidden action + # forbidden action self.assertEqual(response.status_code, status.HTTP_403_FORBIDDEN) # make user 1 the superuser @@ -403,7 +400,7 @@ def test_only_superuser_can_update_other_user_settings(self): # try the request again response = self.client.put( f"/api/v1/users/{self.user_2.id}/setting/", - json.loads(test_settings), + test_settings, format="json", ) @@ -411,7 +408,7 @@ def test_only_superuser_can_update_other_user_settings(self): self.assertEqual(response.status_code, status.HTTP_200_OK) self.assertEqual( User.objects.filter(id=self.user_2.id).first().config["settings"], - json.loads(test_settings), + test_settings, ) diff --git a/users/views.py b/users/views.py index 85d50ed9..720abda1 100644 --- a/users/views.py +++ b/users/views.py @@ -51,9 +51,6 @@ class UserViewSet(viewsets.ModelViewSet): def setting(self, request, pk): """Updates a user's setting key inside it's config""" user = self.get_object() - if "settings" not in user.config: - user.config["settings"] = {} - user.config["settings"] = self.request.data user.save() return Response( From f78c2ba767cb728738ec41bc9a34963a589964d5 Mon Sep 17 00:00:00 2001 From: Deepansh Mathur Date: Fri, 28 Jan 2022 13:44:24 +0530 Subject: [PATCH 17/23] final level of feedback --- organizations/permissions.py | 9 ++------- organizations/tests.py | 10 +++++----- organizations/views.py | 4 ++-- plio/views.py | 4 ++-- users/views.py | 2 +- 5 files changed, 12 insertions(+), 17 deletions(-) diff --git a/organizations/permissions.py b/organizations/permissions.py index e2d54897..b9b550e6 100644 --- a/organizations/permissions.py +++ b/organizations/permissions.py @@ -7,13 +7,8 @@ class OrganizationPermission(permissions.BasePermission): """ def has_permission(self, request, view): - """View-level permissions for organization. This determines whether the request can access organization instances or not.""" - if view.action in ["setting"]: - org_to_update = int(view.kwargs["pk"]) - return request.user.is_superuser or request.user.is_org_admin( - organization_id=org_to_update - ) - return request.user.is_superuser + """View-level permissions for organization viewset. This determines whether the request can access organization viewset or not.""" + return True def has_object_permission(self, request, view, obj): """Object-level permissions for an organization. This determines whether the request can access an organization instance or not.""" diff --git a/organizations/tests.py b/organizations/tests.py index ae8e31ea..f8d5765f 100644 --- a/organizations/tests.py +++ b/organizations/tests.py @@ -92,7 +92,7 @@ def test_updating_organization_recreates_user_instance_cache(self): cache.get(cache_key_name)["organizations"][0]["name"], org_new_name ) - def test_settings_support_only_put_method(self): + def test_settings_support_only_patch_method(self): # some dummy settings dummy_settings = {"setting_name": "setting_value"} @@ -112,14 +112,14 @@ def test_settings_support_only_put_method(self): ) self.assertEqual(response.status_code, status.HTTP_405_METHOD_NOT_ALLOWED) - # try a PATCH request to settings - response = self.client.patch( + # try a PUT request to settings + response = self.client.put( f"/api/v1/organizations/{self.organization_1.id}/setting/", dummy_settings ) self.assertEqual(response.status_code, status.HTTP_405_METHOD_NOT_ALLOWED) - # try a PUT request to settings - response = self.client.put( + # try a PATCH request to settings + response = self.client.patch( f"/api/v1/organizations/{self.organization_1.id}/setting/", dummy_settings ) self.assertEqual(response.status_code, status.HTTP_200_OK) diff --git a/organizations/views.py b/organizations/views.py index ccaf5e13..c00bd918 100644 --- a/organizations/views.py +++ b/organizations/views.py @@ -26,12 +26,12 @@ class OrganizationViewSet(viewsets.ModelViewSet): @action( detail=True, permission_classes=[IsAuthenticated, OrganizationPermission], - methods=["put"], + methods=["patch"], ) def setting(self, request, pk): """Updates an org's settings key inside it's config""" org = self.get_object() - config = org.config if org.config is not None else {"settings": {}} + config = org.config if org.config is not None else {} config["settings"] = self.request.data org.config = config org.save() diff --git a/plio/views.py b/plio/views.py index 7858a5ce..21cb6f61 100644 --- a/plio/views.py +++ b/plio/views.py @@ -152,12 +152,12 @@ def perform_update(self, serializer): @action( detail=True, permission_classes=[IsAuthenticated, PlioPermission], - methods=["put"], + methods=["patch"], ) def setting(self, request, uuid): """Updates a plio's settings""" plio = self.get_object() - config = plio.config if plio.config is not None else {"settings": {}} + config = plio.config if plio.config is not None else {} config["settings"] = self.request.data plio.config = config plio.save() diff --git a/users/views.py b/users/views.py index 720abda1..fd53fe8e 100644 --- a/users/views.py +++ b/users/views.py @@ -46,7 +46,7 @@ class UserViewSet(viewsets.ModelViewSet): @action( detail=True, permission_classes=[IsAuthenticated, UserPermission], - methods=["put"], + methods=["patch"], ) def setting(self, request, pk): """Updates a user's setting key inside it's config""" From 228721d0c3d250bbb531a8003fe369b9316b2ef7 Mon Sep 17 00:00:00 2001 From: Deepansh Mathur Date: Fri, 28 Jan 2022 14:21:32 +0530 Subject: [PATCH 18/23] test cases fix --- organizations/tests.py | 19 ++++++++++--------- plio/tests.py | 6 +++--- users/tests.py | 6 +++--- 3 files changed, 16 insertions(+), 15 deletions(-) diff --git a/organizations/tests.py b/organizations/tests.py index f8d5765f..2bf58ac8 100644 --- a/organizations/tests.py +++ b/organizations/tests.py @@ -36,11 +36,11 @@ def test_guest_cannot_list_organization_random_token(self): response = self.client.get(reverse("organizations-list")) self.assertEqual(response.status_code, status.HTTP_401_UNAUTHORIZED) - def test_non_superuser_cannot_list_organizations(self): - """A non-superuser should not be able to list organizations""" + def test_non_superuser_can_list_organizations(self): + """A non-superuser should be able to list organizations""" # get organizations response = self.client.get(reverse("organizations-list")) - self.assertEqual(response.status_code, status.HTTP_403_FORBIDDEN) + self.assertEqual(response.status_code, status.HTTP_200_OK) def test_superuser_can_list_organizations(self): """A superuser should be able to list organizations""" @@ -139,7 +139,7 @@ def test_superuser_can_update_any_org_settings(self): self.user.save() # try updating settings of org 1 - response = self.client.put( + response = self.client.patch( f"/api/v1/organizations/{self.organization_1.id}/setting/", dummy_settings ) self.assertEqual(response.status_code, status.HTTP_200_OK) @@ -151,7 +151,7 @@ def test_superuser_can_update_any_org_settings(self): ) # try updating settings of org 2 - response = self.client.put( + response = self.client.patch( f"/api/v1/organizations/{self.organization_2.id}/setting/", dummy_settings ) self.assertEqual(response.status_code, status.HTTP_200_OK) @@ -173,12 +173,13 @@ def test_org_admin_can_update_own_org_settings_only(self): # user should NOT be able to update org 1 settings as # the user is not an org admin - response = self.client.put( + response = self.client.patch( f"/api/v1/organizations/{self.organization_1.id}/setting/", dummy_settings ) self.assertEqual(response.status_code, status.HTTP_403_FORBIDDEN) - # create a new super user + # create a new super user and a new APIClient that will be attached to the super user + # this is being done so we can use this superuser client to update our user and make them an org admin superuser_client = APIClient() superuser = User.objects.create(mobile="+919988776655", is_superuser=True) superuser_access_token = get_new_access_token(superuser, self.application) @@ -197,7 +198,7 @@ def test_org_admin_can_update_own_org_settings_only(self): ) # user should be able to update org 1 settings - response = self.client.put( + response = self.client.patch( f"/api/v1/organizations/{self.organization_1.id}/setting/", dummy_settings ) self.assertEqual(response.status_code, status.HTTP_200_OK) @@ -209,7 +210,7 @@ def test_org_admin_can_update_own_org_settings_only(self): ) # but the user still should NOT be able to update settings for org 2 - response = self.client.put( + response = self.client.patch( f"/api/v1/organizations/{self.organization_2.id}/setting/", dummy_settings ) self.assertEqual(response.status_code, status.HTTP_403_FORBIDDEN) diff --git a/plio/tests.py b/plio/tests.py index 59da90a0..0e4d8ff9 100644 --- a/plio/tests.py +++ b/plio/tests.py @@ -574,7 +574,7 @@ def test_user_can_update_own_plio_settings(self): test_settings = {"player": {"configuration": {"skipEnabled": False}}} # update settings for plio_1 - response = self.client.put( + response = self.client.patch( f"/api/v1/plios/{self.plio_1.uuid}/setting/", test_settings, format="json", @@ -598,7 +598,7 @@ def test_user_cannot_update_other_user_plio_settings(self): ) # user_1 tries updating the settings for the above created plio - response = self.client.put( + response = self.client.patch( f"/api/v1/plios/{plio.uuid}/setting/", test_settings, format="json", @@ -640,7 +640,7 @@ def test_org_members_can_update_org_plio_settings(self): # user_1 tries to update the org plio's settings which was created # by user_2 - response = self.client.put( + response = self.client.patch( f"/api/v1/plios/{plio_org.uuid}/setting/", test_settings, format="json", diff --git a/users/tests.py b/users/tests.py index dec6ae36..8fda1cc4 100644 --- a/users/tests.py +++ b/users/tests.py @@ -363,7 +363,7 @@ def test_user_can_update_its_own_settings(self): "app": {"appearance": {"darkMode": True}}, } # update settings for user 1 - response = self.client.put( + response = self.client.patch( f"/api/v1/users/{self.user.id}/setting/", test_settings, format="json", @@ -384,7 +384,7 @@ def test_only_superuser_can_update_other_user_settings(self): } # try to update settings for user_2 - response = self.client.put( + response = self.client.patch( f"/api/v1/users/{self.user_2.id}/setting/", test_settings, format="json", @@ -398,7 +398,7 @@ def test_only_superuser_can_update_other_user_settings(self): self.user.save() # try the request again - response = self.client.put( + response = self.client.patch( f"/api/v1/users/{self.user_2.id}/setting/", test_settings, format="json", From 0a568f0378047c89a00eec40c16f1e70ba999cd9 Mon Sep 17 00:00:00 2001 From: Deepansh Mathur Date: Fri, 28 Jan 2022 14:22:16 +0530 Subject: [PATCH 19/23] Update organizations/views.py Co-authored-by: Aman Dalmia --- organizations/views.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/organizations/views.py b/organizations/views.py index c00bd918..41004b3f 100644 --- a/organizations/views.py +++ b/organizations/views.py @@ -29,7 +29,7 @@ class OrganizationViewSet(viewsets.ModelViewSet): methods=["patch"], ) def setting(self, request, pk): - """Updates an org's settings key inside it's config""" + """Updates an org's settings""" org = self.get_object() config = org.config if org.config is not None else {} config["settings"] = self.request.data From e86fee82a154dc9e5863fee403e7be5ce0720832 Mon Sep 17 00:00:00 2001 From: Deepansh Mathur Date: Fri, 28 Jan 2022 14:22:21 +0530 Subject: [PATCH 20/23] Update users/views.py Co-authored-by: Aman Dalmia --- users/views.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/users/views.py b/users/views.py index fd53fe8e..bd0959cf 100644 --- a/users/views.py +++ b/users/views.py @@ -49,7 +49,7 @@ class UserViewSet(viewsets.ModelViewSet): methods=["patch"], ) def setting(self, request, pk): - """Updates a user's setting key inside it's config""" + """Updates a user's settings""" user = self.get_object() user.config["settings"] = self.request.data user.save() From b49b2c255baa2d5653cf9d7789ec953084e6ccde Mon Sep 17 00:00:00 2001 From: Deepansh Mathur Date: Mon, 31 Jan 2022 11:40:20 +0530 Subject: [PATCH 21/23] Update organizations/views.py --- organizations/views.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/organizations/views.py b/organizations/views.py index 41004b3f..919d142f 100644 --- a/organizations/views.py +++ b/organizations/views.py @@ -36,5 +36,5 @@ def setting(self, request, pk): org.config = config org.save() return Response( - self.get_serializer(org).data["config"], status=status.HTTP_200_OK + self.get_serializer(org).data["config"] ) From 83cf2d1721797add97349bb7b2c4b83fc8ce8a99 Mon Sep 17 00:00:00 2001 From: Deepansh Mathur Date: Mon, 31 Jan 2022 11:43:49 +0530 Subject: [PATCH 22/23] Update organizations/views.py Co-authored-by: Aman Dalmia --- organizations/views.py | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/organizations/views.py b/organizations/views.py index 919d142f..bfeca087 100644 --- a/organizations/views.py +++ b/organizations/views.py @@ -31,9 +31,8 @@ class OrganizationViewSet(viewsets.ModelViewSet): def setting(self, request, pk): """Updates an org's settings""" org = self.get_object() - config = org.config if org.config is not None else {} - config["settings"] = self.request.data - org.config = config + org.config = org.config if org.config is not None else {} + org.config["settings"] = self.request.data org.save() return Response( self.get_serializer(org).data["config"] From bb5f274cb3e9c0cfd1ce0788efbc7a7af67a8528 Mon Sep 17 00:00:00 2001 From: Deepansh Mathur Date: Mon, 31 Jan 2022 11:45:55 +0530 Subject: [PATCH 23/23] PR feedback --- organizations/views.py | 6 ++---- plio/views.py | 9 +++------ users/views.py | 8 +++----- 3 files changed, 8 insertions(+), 15 deletions(-) diff --git a/organizations/views.py b/organizations/views.py index bfeca087..92a7d317 100644 --- a/organizations/views.py +++ b/organizations/views.py @@ -1,4 +1,4 @@ -from rest_framework import viewsets, status +from rest_framework import viewsets from organizations.models import Organization from organizations.serializers import OrganizationSerializer from rest_framework.permissions import IsAuthenticated @@ -34,6 +34,4 @@ def setting(self, request, pk): org.config = org.config if org.config is not None else {} org.config["settings"] = self.request.data org.save() - return Response( - self.get_serializer(org).data["config"] - ) + return Response(self.get_serializer(org).data["config"]) diff --git a/plio/views.py b/plio/views.py index 21cb6f61..25d807b0 100644 --- a/plio/views.py +++ b/plio/views.py @@ -157,13 +157,10 @@ def perform_update(self, serializer): def setting(self, request, uuid): """Updates a plio's settings""" plio = self.get_object() - config = plio.config if plio.config is not None else {} - config["settings"] = self.request.data - plio.config = config + plio.config = plio.config if plio.config is not None else {} + plio.config["settings"] = self.request.data plio.save() - return Response( - self.get_serializer(plio).data["config"], status=status.HTTP_200_OK - ) + return Response(self.get_serializer(plio).data["config"]) @property def organization_shortcode(self): diff --git a/users/views.py b/users/views.py index bd0959cf..f761df4d 100644 --- a/users/views.py +++ b/users/views.py @@ -53,9 +53,7 @@ def setting(self, request, pk): user = self.get_object() user.config["settings"] = self.request.data user.save() - return Response( - self.get_serializer(user).data["config"], status=status.HTTP_200_OK - ) + return Response(self.get_serializer(user).data["config"]) @action( detail=True, @@ -221,7 +219,7 @@ def verify_otp(request): # login the user, get the new access token and return token = login_user_and_get_access_token(user, request) - return Response(token, status=status.HTTP_200_OK) + return Response(token) except OneTimePassword.DoesNotExist: return Response({"detail": "unauthorized"}, status=status.HTTP_401_UNAUTHORIZED) @@ -282,4 +280,4 @@ def generate_external_auth_access_token(request): # login the user, get the new access token and return token = login_user_and_get_access_token(user, request) - return Response(token, status=status.HTTP_200_OK) + return Response(token)