Skip to content

Commit

Permalink
fix delete perms
Browse files Browse the repository at this point in the history
  • Loading branch information
lastminutediorama committed Aug 31, 2024
1 parent bf98c36 commit e7efd3d
Show file tree
Hide file tree
Showing 5 changed files with 52 additions and 57 deletions.
15 changes: 3 additions & 12 deletions src/planscape/collaboration/permissions.py
Original file line number Diff line number Diff line change
Expand Up @@ -149,7 +149,6 @@ def can_remove(user: User, scenario: Scenario):
# or if we can just make use of PlanningArea perms here
class ProjectAreaNotePermission(CheckPermissionMixin):
@staticmethod
# TODO: prefetch planning area for this project area?
def can_view(user: User, project_area_note: ProjectAreaNote):
if is_creator(user, project_area_note.project_area):
return True
Expand All @@ -164,16 +163,8 @@ def can_add(user: User, project_area: ProjectArea):
return True
return check_for_permission(user.id, planning_area, "view_planningarea")

# we need to implement this, I believe?
# TODO: should the owner of the project area be able to remove notes, also?
@staticmethod
def can_change(user: User, project_area_note: ProjectAreaNote):
def can_remove(user: User, project_area_note: ProjectAreaNote):
planning_area = project_area_note.project_area.scenario.planning_area
return is_creator(user, planning_area) or is_creator(user, planning_area)

# creators of the planning area or authors of notes can remove a note
@staticmethod
def can_remove(user: User, project_area: ProjectArea):
print(f"we are getting called to remove a thing? {project_area}")
print(f" Do we actually have the planning area? {planning_area}")
planning_area = project_area.scenario.planning_area
return is_creator(user, planning_area) or is_creator(user, project_area)
return is_creator(user, planning_area) or is_creator(user, project_area_note)
14 changes: 2 additions & 12 deletions src/planscape/planning/permissions.py
Original file line number Diff line number Diff line change
Expand Up @@ -56,23 +56,13 @@ def has_permission(self, request, view):
project_area = ProjectArea.objects.get(id=project_area_id)
return ProjectAreaNotePermission.can_add(request.user, project_area)
case _:
# scenario filters this on the queryset
# TODO: review if this is necessary
return True

def has_object_permission(self, request, view, object):
project_area = object.project_area
print(f" We have a project_area? {project_area}")
print(
f" We are trying to get object permission for this view action: {view.action}"
)

match view.action:
case "update" | "partial_update":
# TODO: we should never get here for a note
method = ProjectAreaNotePermission.can_change
case "destroy":
print(f"we got here, were trying to remove something...")
method = ProjectAreaNotePermission.can_remove
case _:
method = ProjectAreaNotePermission.can_view
return method(request.user, project_area)
return method(request.user, object)
2 changes: 2 additions & 0 deletions src/planscape/planning/serializers.py
Original file line number Diff line number Diff line change
Expand Up @@ -387,5 +387,7 @@ class Meta:
"project_area",
"user_id",
"user_name",
# TODO: add this with serializer method field
# "can_delete",
)
model = ProjectAreaNote
77 changes: 45 additions & 32 deletions src/planscape/planning/tests/test_v2_views.py
Original file line number Diff line number Diff line change
Expand Up @@ -878,13 +878,14 @@ def test_create_note(self):
new_note,
content_type="application/json",
)
self.assertEqual(response.status_code, 201)
self.assertEqual(response.status_code, status.HTTP_201_CREATED)
response_data = response.json()
self.assertEqual(
response_data["content"], "Here is a note about a project area."
)
self.assertEqual(response_data["user_id"], self.user.pk)

# this fails if a user is projectarea owner, but not planningarea owner or note owner
def test_create_note_as_projectarea_owner(self):
self.client.force_authenticate(self.other_user)
new_note = json.dumps(
Expand All @@ -900,7 +901,7 @@ def test_create_note_as_projectarea_owner(self):
new_note,
content_type="application/json",
)
self.assertEqual(response.status_code, 403)
self.assertEqual(response.status_code, status.HTTP_403_FORBIDDEN)

def test_create_note_without_permission(self):
self.client.force_authenticate(self.other_user)
Expand All @@ -917,7 +918,7 @@ def test_create_note_without_permission(self):
new_note,
content_type="application/json",
)
self.assertEqual(response.status_code, 403)
self.assertEqual(response.status_code, status.HTTP_403_FORBIDDEN)

def test_get_notes_for_project_area(self):
self.client.force_authenticate(self.user)
Expand All @@ -944,35 +945,47 @@ def test_get_notes_for_project_area(self):
content_type="application/json",
)
response_data = response.json()
print(f"Here is the get note response: {response_data}")
self.assertEqual(response.status_code, 200)
self.assertEqual(len(response_data), 3)

# TODO:
# def test_delete_note(self):
# self.client.force_authenticate(self.user)
# new_note = ProjectAreaNote.objects.create(
# project_area=self.project_area, user=self.user
# )
# response = self.client.delete(
# reverse(
# "api:planning:projectarea-notes-detail",
# kwargs={"pk": new_note.pk},
# ),
# content_type="application/json",
# )
# self.assertEqual(response.status_code, status.HTTP_204_NO_CONTENT)

# def test_delete_nonexistent_note(self):
# self.client.force_authenticate(self.user)
# new_note = ProjectAreaNote.objects.create(
# project_area=self.project_area, user=self.user
# )
# response = self.client.delete(
# reverse(
# "api:planning:projectarea-notes-detail",
# kwargs={"pk": (new_note.pk + 1)},
# ),
# content_type="application/json",
# )
# self.assertEqual(response.status_code, status.HTTP_404_NOT_FOUND)
def test_delete_note(self):
self.client.force_authenticate(self.user)
new_note = ProjectAreaNote.objects.create(
project_area=self.project_area, user=self.user
)
response = self.client.delete(
reverse(
"api:planning:projectarea-notes-detail",
kwargs={"pk": new_note.pk},
),
content_type="application/json",
)
self.assertEqual(response.status_code, status.HTTP_204_NO_CONTENT)

def test_delete_nonexistent_note(self):
self.client.force_authenticate(self.user)
new_note = ProjectAreaNote.objects.create(
project_area=self.project_area, user=self.user
)
response = self.client.delete(
reverse(
"api:planning:projectarea-notes-detail",
kwargs={"pk": (new_note.pk + 1)},
),
content_type="application/json",
)
self.assertEqual(response.status_code, status.HTTP_404_NOT_FOUND)

def test_delete_note_no_permissions(self):
self.client.force_authenticate(self.other_user)
new_note = ProjectAreaNote.objects.create(
project_area=self.project_area, user=self.user
)
response = self.client.delete(
reverse(
"api:planning:projectarea-notes-detail",
kwargs={"pk": new_note.pk},
),
content_type="application/json",
)
self.assertEqual(response.status_code, status.HTTP_403_FORBIDDEN)
1 change: 0 additions & 1 deletion src/planscape/planning/views_v2.py
Original file line number Diff line number Diff line change
Expand Up @@ -225,7 +225,6 @@ def create(self, request, *args, **kwargs):
# ensure that user has projectarea / planningarea permission first
def get_queryset(self):
project_area_id = self.request.query_params.get("project_area_pk")
print(f" Do we have a project area: {project_area_id}")
if project_area_id:
return (
super()
Expand Down

0 comments on commit e7efd3d

Please sign in to comment.