Skip to content

Commit 9796e93

Browse files
sjdemartinifiraskafri
authored andcommitted
Remove obsolete tests and add note about rationale
1 parent f67c5db commit 9796e93

File tree

1 file changed

+6
-140
lines changed

1 file changed

+6
-140
lines changed

graphene_django/tests/test_get_queryset.py

Lines changed: 6 additions & 140 deletions
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,12 @@ class TestShouldCallGetQuerySetOnForeignKey:
1616
Check that the get_queryset method is called in both forward and reversed direction
1717
of a foreignkey on types.
1818
(see issue #1111)
19+
20+
NOTE: For now, we do not expect this get_queryset method to be called for nested
21+
objects, as the original attempt to do so prevented SQL query-optimization with
22+
`select_related`/`prefetch_related` and caused N+1 queries. See discussions here
23+
https://github.com/graphql-python/graphene-django/pull/1315/files#r1015659857
24+
and here https://github.com/graphql-python/graphene-django/pull/1401.
1925
"""
2026

2127
@pytest.fixture(autouse=True)
@@ -121,73 +127,6 @@ def test_get_queryset_called_on_field(self):
121127
assert not result.errors
122128
assert result.data == {"reporter": {"firstName": "Jane"}}
123129

124-
# TODO: This test is currently expected to fail because the logic it depended on has been
125-
# removed, due to poor SQL performance and preventing query-optimization (see
126-
# https://github.com/graphql-python/graphene-django/pull/1315/files#r1015659857)
127-
@pytest.mark.xfail
128-
def test_get_queryset_called_on_foreignkey(self):
129-
# If a user tries to access a reporter through an article they should get our authorization error
130-
query = """
131-
query getArticle($id: ID!) {
132-
article(id: $id) {
133-
headline
134-
reporter {
135-
firstName
136-
}
137-
}
138-
}
139-
"""
140-
141-
result = self.schema.execute(query, variables={"id": self.articles[0].id})
142-
assert len(result.errors) == 1
143-
assert result.errors[0].message == "Not authorized to access reporters."
144-
145-
# An admin user should be able to get reporters through an article
146-
query = """
147-
query getArticle($id: ID!) {
148-
article(id: $id) {
149-
headline
150-
reporter {
151-
firstName
152-
}
153-
}
154-
}
155-
"""
156-
157-
result = self.schema.execute(
158-
query,
159-
variables={"id": self.articles[0].id},
160-
context_value={"admin": True},
161-
)
162-
assert not result.errors
163-
assert result.data["article"] == {
164-
"headline": "A fantastic article",
165-
"reporter": {"firstName": "Jane"},
166-
}
167-
168-
# An admin user should not be able to access draft article through a reporter
169-
query = """
170-
query getReporter($id: ID!) {
171-
reporter(id: $id) {
172-
firstName
173-
articles {
174-
headline
175-
}
176-
}
177-
}
178-
"""
179-
180-
result = self.schema.execute(
181-
query,
182-
variables={"id": self.reporter.id},
183-
context_value={"admin": True},
184-
)
185-
assert not result.errors
186-
assert result.data["reporter"] == {
187-
"firstName": "Jane",
188-
"articles": [{"headline": "A fantastic article"}],
189-
}
190-
191130

192131
class TestShouldCallGetQuerySetOnForeignKeyNode:
193132
"""
@@ -294,76 +233,3 @@ def test_get_queryset_called_on_node(self):
294233
)
295234
assert not result.errors
296235
assert result.data == {"reporter": {"firstName": "Jane"}}
297-
298-
# TODO: This test is currently expected to fail because the logic it depended on has been
299-
# removed, due to poor SQL performance and preventing query-optimization (see
300-
# https://github.com/graphql-python/graphene-django/pull/1315/files#r1015659857)
301-
@pytest.mark.xfail
302-
def test_get_queryset_called_on_foreignkey(self):
303-
# If a user tries to access a reporter through an article they should get our authorization error
304-
query = """
305-
query getArticle($id: ID!) {
306-
article(id: $id) {
307-
headline
308-
reporter {
309-
firstName
310-
}
311-
}
312-
}
313-
"""
314-
315-
result = self.schema.execute(
316-
query, variables={"id": to_global_id("ArticleType", self.articles[0].id)}
317-
)
318-
assert len(result.errors) == 1
319-
assert result.errors[0].message == "Not authorized to access reporters."
320-
321-
# An admin user should be able to get reporters through an article
322-
query = """
323-
query getArticle($id: ID!) {
324-
article(id: $id) {
325-
headline
326-
reporter {
327-
firstName
328-
}
329-
}
330-
}
331-
"""
332-
333-
result = self.schema.execute(
334-
query,
335-
variables={"id": to_global_id("ArticleType", self.articles[0].id)},
336-
context_value={"admin": True},
337-
)
338-
assert not result.errors
339-
assert result.data["article"] == {
340-
"headline": "A fantastic article",
341-
"reporter": {"firstName": "Jane"},
342-
}
343-
344-
# An admin user should not be able to access draft article through a reporter
345-
query = """
346-
query getReporter($id: ID!) {
347-
reporter(id: $id) {
348-
firstName
349-
articles {
350-
edges {
351-
node {
352-
headline
353-
}
354-
}
355-
}
356-
}
357-
}
358-
"""
359-
360-
result = self.schema.execute(
361-
query,
362-
variables={"id": to_global_id("ReporterType", self.reporter.id)},
363-
context_value={"admin": True},
364-
)
365-
assert not result.errors
366-
assert result.data["reporter"] == {
367-
"firstName": "Jane",
368-
"articles": {"edges": [{"node": {"headline": "A fantastic article"}}]},
369-
}

0 commit comments

Comments
 (0)