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.

Sign up for GitHub

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

Make non-existent resource report raise 404 #11778

Open
wants to merge 3 commits into
base: dev/7.6.x
Choose a base branch
from
+22 −1
Open

Make non-existent resource report raise 404 #11778

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
095f523
put Resource orm query in try/except Resource.DoesNotExist handling, …
whatisgalen Feb 10, 2025
b0a4c7f
create test for deleted resource report 404, re #11772
whatisgalen Feb 10, 2025
ca4c0f4
use Resource proxy delete in test, re #11772
whatisgalen Feb 10, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 4 additions & 1 deletion arches/app/views/resource.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -930,7 +930,10 @@ def get(self, request, resourceid=None):
@method_decorator(can_read_resource_instance, name="dispatch")
class ResourceReportView(MapBaseManagerView):
def get(self, request, resourceid=None):
resource = Resource.objects.only("graph_id").get(pk=resourceid)
try:
resource = Resource.objects.only("graph_id").get(pk=resourceid)
except Resource.DoesNotExist:
raise Http404(_("Resource does not exist"))
graph = Graph.objects.get(graphid=resource.graph_id)

try:
Expand Down
18 changes: 18 additions & 0 deletions tests/views/resource_tests.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -354,3 +354,21 @@ def test_get_related_resource(self):
ret = reference_resource.get_related_resources(user=user)
relationship = ret["resource_relationships"][0]["relationshiptype_label"]
self.assertEqual(relationship, en_preflabel)

def test_resource_report_good(self):
self.client.login(username="admin", password="admin")
url = reverse(
"resource_report", kwargs={"resourceid": self.resource_instance_id}
)
response = self.client.get(url)
self.assertEqual(response.status_code, 200)

def test_resource_report_bad(self):
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Because test_resource_report_good tests if the status is 200, that name makes sense. In this case though, I think what's being tested isn't whether the response is bad (which I would take to mean a 500) but rather whether it's missing. So I think more intuitive name would be test_resource_report_for_missing_resource or something like that.

self.client.login(username="admin", password="admin")
resource = Resource.objects.get(resourceinstanceid=self.resource_instance_id)
resource.delete()
url = reverse(
"resource_report", kwargs={"resourceid": self.resource_instance_id}
)
response = self.client.get(url)
self.assertEqual(response.status_code, 404)