Skip to content

Commit b46164c

Browse files
committed
Merge remote-tracking branch 'origin/main' into claude/resolve-issue-1332-1lh9h
# Conflicts: # config/graphql/analysis_mutations.py # config/graphql/annotation_types.py # config/graphql/base_types.py # config/graphql/corpus_queries.py # config/graphql/document_queries.py # config/graphql/filters.py # config/graphql/moderation_mutations.py # config/graphql/og_metadata_queries.py # config/graphql/pipeline_queries.py # config/graphql/security.py # config/graphql/slug_queries.py # config/graphql/user_queries.py # config/graphql/user_types.py # docs/typing/mypy_baseline.txt # mypy.ini
2 parents 8803499 + fa1207c commit b46164c

51 files changed

Lines changed: 480 additions & 117 deletions

File tree

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

CHANGELOG.md

Lines changed: 31 additions & 0 deletions
Large diffs are not rendered by default.

config/graphql/annotation_serializers.py

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,8 @@
22
Serializers related to annotations to avoid circular imports.
33
"""
44

5+
from typing import Any
6+
57
from django.contrib.auth import get_user_model
68
from rest_framework import serializers
79

@@ -31,7 +33,7 @@ class Meta:
3133
]
3234
read_only_fields = ["id", "creator"]
3335

34-
def create(self, validated_data):
36+
def create(self, validated_data) -> Any:
3537
creator_id = validated_data.pop("creator_id", None)
3638
if creator_id:
3739
try:

config/graphql/base.py

Lines changed: 66 additions & 32 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,8 @@
22
import logging
33
import traceback
44
from abc import ABC
5+
from collections.abc import Sequence
6+
from typing import Any, ClassVar, Optional
57

68
import django.db.models
79
import graphene
@@ -21,12 +23,30 @@
2123
logger = logging.getLogger(__name__)
2224

2325

26+
def _require_io_setting(mutation_cls: type, name: str) -> Any:
27+
"""Raise ``NotImplementedError`` if ``cls.IOSettings.<name>`` is missing or ``None``."""
28+
io_settings = getattr(mutation_cls, "IOSettings", None)
29+
if io_settings is None:
30+
raise NotImplementedError(
31+
f"{mutation_cls.__name__} must define an IOSettings class."
32+
)
33+
value = getattr(io_settings, name, None)
34+
if value is None:
35+
raise NotImplementedError(
36+
f"{mutation_cls.__name__}.IOSettings.{name} must be set by the "
37+
f"subclass."
38+
)
39+
return value
40+
41+
2442
class OpenContractsNode(Node):
2543
class Meta:
2644
name = "Node"
2745

2846
@classmethod
29-
def get_node_from_global_id(cls, info, global_id, only_type=None):
47+
def get_node_from_global_id(
48+
cls, info: Any, global_id: str, only_type: Any | None = None
49+
) -> Any:
3050

3151
_type, _id = from_global_id(global_id)
3252

@@ -63,7 +83,7 @@ class Meta:
6383

6484
total_count = graphene.Int()
6585

66-
def resolve_total_count(root, info, **kwargs):
86+
def resolve_total_count(root, info, **kwargs) -> Any:
6787
if isinstance(root.iterable, django.db.models.QuerySet):
6888
return root.iterable.count()
6989
else:
@@ -72,7 +92,8 @@ def resolve_total_count(root, info, **kwargs):
7292

7393
class DRFDeletion(graphene.Mutation):
7494
class IOSettings(ABC):
75-
lookup_field = "id"
95+
lookup_field: ClassVar[str] = "id"
96+
model: ClassVar[Optional[type[django.db.models.Model]]] = None
7697

7798
class Arguments:
7899
id = graphene.String(required=False)
@@ -83,14 +104,26 @@ class Arguments:
83104
@classmethod
84105
@login_required
85106
@graphql_ratelimit(rate=RateLimits.WRITE_LIGHT)
86-
def mutate(cls, root, info, *args, **kwargs):
107+
def mutate(cls, root, info, *args, **kwargs) -> "DRFDeletion":
87108

109+
# Unlike ``DRFMutation.mutate`` below, this method intentionally does
110+
# NOT wrap the body in ``except Exception``. Errors (including the
111+
# ``NotImplementedError`` from ``_require_io_setting`` and the
112+
# ``ValueError`` for missing lookup args) propagate to the GraphQL
113+
# framework as raw execution errors.
88114
ok = False
89115

90-
id = from_global_id(kwargs.get(cls.IOSettings.lookup_field, None))[1]
116+
model = _require_io_setting(cls, "model")
117+
lookup_field = cls.IOSettings.lookup_field
118+
lookup_value = kwargs.get(lookup_field)
119+
if lookup_value is None:
120+
raise ValueError(
121+
f"'{lookup_field}' is required to identify the object to delete."
122+
)
123+
id = from_global_id(lookup_value)[1]
91124
# Filter through visible_to_user() to prevent IDOR -- returns same
92125
# DoesNotExist error whether object is missing or user lacks access.
93-
obj = cls.IOSettings.model.objects.visible_to_user(info.context.user).get(pk=id)
126+
obj = model.objects.visible_to_user(info.context.user).get(pk=id)
94127

95128
# if there's a user lock, only the lock holder (or superuser) can proceed
96129
if hasattr(obj, "user_lock") and obj.user_lock is not None:
@@ -123,11 +156,13 @@ def mutate(cls, root, info, *args, **kwargs):
123156

124157
class DRFMutation(graphene.Mutation):
125158
class IOSettings(ABC):
126-
pk_fields: list[str | int] = []
127-
lookup_field = "id"
128-
model: django.db.models.Model = None
129-
graphene_model: DjangoObjectType = None
130-
serializer = None
159+
# Frozen default — subclasses override with their own list/tuple.
160+
# Using a tuple here avoids the shared-mutable-default footgun.
161+
pk_fields: ClassVar[Sequence[str]] = ()
162+
lookup_field: ClassVar[str] = "id"
163+
model: ClassVar[Optional[type[django.db.models.Model]]] = None
164+
graphene_model: ClassVar[Optional[type[DjangoObjectType]]] = None
165+
serializer: ClassVar[Optional[type[serializers.Serializer]]] = None
131166

132167
class Arguments:
133168
pass
@@ -137,7 +172,7 @@ class Arguments:
137172
obj_id = graphene.ID()
138173

139174
@staticmethod
140-
def format_validation_error(ve):
175+
def format_validation_error(ve: serializers.ValidationError) -> str:
141176
"""Surface validation errors with clean formatting.
142177
143178
``str(ValidationError)`` renders as
@@ -158,7 +193,7 @@ def format_validation_error(ve):
158193
@classmethod
159194
@login_required
160195
@graphql_ratelimit(rate=RateLimits.WRITE_MEDIUM)
161-
def mutate(cls, root, info, *args, **kwargs):
196+
def mutate(cls, root, info, *args, **kwargs) -> "DRFMutation":
162197

163198
ok = False
164199
obj_id = None
@@ -175,20 +210,25 @@ def mutate(cls, root, info, *args, **kwargs):
175210
raise ValueError("No user in this request...")
176211

177212
logger.info(f"DRFMutation - kwargs: {kwargs}")
178-
serializer = cls.IOSettings.serializer
213+
serializer = _require_io_setting(cls, "serializer")
214+
model = _require_io_setting(cls, "model")
215+
# ``graphene_model`` is the class itself; ``__name__`` is the GraphQL
216+
# type name (e.g. ``"CorpusType"``). ``__class__.__name__`` would
217+
# return the metaclass name (``"SubclassWithMeta_Meta"``) which is
218+
# the bug this PR fixes — kept dereferenced as ``__name__`` below.
219+
graphene_model = _require_io_setting(cls, "graphene_model")
179220

180221
if hasattr(cls.IOSettings, "pk_fields"):
181222
for pk_field in cls.IOSettings.pk_fields:
182223
if pk_field in kwargs:
183-
if isinstance(kwargs[pk_field], list):
184-
pk_value = []
185-
for global_id in kwargs[pk_field]:
186-
# global_id is already the ID string, not a key
187-
pk_value.append(from_global_id(global_id)[1])
224+
raw_value = kwargs[pk_field]
225+
if isinstance(raw_value, list):
226+
kwargs[pk_field] = [
227+
from_global_id(global_id)[1] for global_id in raw_value
228+
]
188229
else:
189-
logger.info(f"pk field is: {kwargs.get(pk_field, None)}")
190-
pk_value = from_global_id(kwargs.get(pk_field, None))[1]
191-
kwargs[pk_field] = pk_value
230+
logger.info(f"pk field is: {raw_value}")
231+
kwargs[pk_field] = from_global_id(raw_value)[1]
192232

193233
# Check if lookup_field exists in IOSettings and if it's in kwargs
194234
# This allows create mutations to work without requiring lookup_field
@@ -201,10 +241,8 @@ def mutate(cls, root, info, *args, **kwargs):
201241
logger.info("Lookup_field specified - update")
202242
# Filter through visible_to_user() to prevent IDOR --
203243
# returns same DoesNotExist whether missing or no access.
204-
obj = cls.IOSettings.model.objects.visible_to_user(
205-
info.context.user
206-
).get(
207-
pk=from_global_id(kwargs.get(cls.IOSettings.lookup_field, None))[1]
244+
obj = model.objects.visible_to_user(info.context.user).get(
245+
pk=from_global_id(kwargs[cls.IOSettings.lookup_field])[1]
208246
)
209247

210248
logger.info(f"Retrieved obj: {obj}")
@@ -240,9 +278,7 @@ def mutate(cls, root, info, *args, **kwargs):
240278
obj_serializer.save()
241279
ok = True
242280
message = "Success"
243-
obj_id = to_global_id(
244-
cls.IOSettings.graphene_model.__class__.__name__, obj.id
245-
)
281+
obj_id = to_global_id(graphene_model.__name__, obj.id)
246282
logger.info("Succeeded updating obj")
247283

248284
else:
@@ -262,9 +298,7 @@ def mutate(cls, root, info, *args, **kwargs):
262298

263299
ok = True
264300
message = "Success"
265-
obj_id = to_global_id(
266-
cls.IOSettings.graphene_model.__class__.__name__, obj.id
267-
)
301+
obj_id = to_global_id(graphene_model.__name__, obj.id)
268302

269303
except serializers.ValidationError as ve:
270304
logger.warning(f"Validation error in mutation: {ve.detail}")

config/graphql/conversation_types.py

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -142,7 +142,7 @@ def resolve_mentioned_resources(self, info) -> Any:
142142
from opencontractserver.corpuses.models import Corpus
143143
from opencontractserver.documents.models import Document, DocumentPath
144144

145-
def _extract_annotation_id(url: str):
145+
def _extract_annotation_id(url: str) -> Any:
146146
"""
147147
Extract annotation ID from URL query params.
148148

config/graphql/corpus_mutations.py

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@
33
"""
44

55
import logging
6+
from typing import Any
67

78
import graphene
89
from django.conf import settings
@@ -536,7 +537,7 @@ def dispatch_fork_task(
536537
_corpus_id=corpus.id,
537538
_collected=collected,
538539
_user_id=info.context.user.id,
539-
):
540+
) -> Any:
540541
fork_corpus.si(
541542
_corpus_id,
542543
_collected.document_ids,

config/graphql/custom_connections.py

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,5 @@
11
import logging
2+
from typing import Any
23

34
from graphene import Connection, Int
45

@@ -12,14 +13,14 @@ class Meta:
1213
current_page = Int()
1314
page_count = Int()
1415

15-
def resolve_current_page(root, info, **kwargs):
16+
def resolve_current_page(root, info, **kwargs) -> Any:
1617
# print(
1718
# f"PdfPageAwareConnection- resolve_total_count kwargs: {kwargs} / root {dir(root)} / iteracble "
1819
# f"{root.iterable.count()}"
1920
# )
2021
return 1
2122

22-
def resolve_page_count(root, info, **kwargs):
23+
def resolve_page_count(root, info, **kwargs) -> Any:
2324

2425
largest_page_number = max(
2526
list(root.iterable.values_list("page", flat=True).distinct())

config/graphql/custom_resolvers.py

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@
33
from __future__ import annotations
44

55
from collections.abc import Iterable
6+
from typing import Any
67

78
from graphql_relay import from_global_id
89

@@ -46,7 +47,7 @@ def _apply_filter(sequence: Iterable, predicate) -> list:
4647
return [item for item in sequence if predicate(item)]
4748

4849

49-
def resolve_doc_annotations_optimized(self, info, **kwargs):
50+
def resolve_doc_annotations_optimized(self, info, **kwargs) -> Any:
5051
"""Resolve ``docAnnotations`` while favouring prefetched data and the optimizer."""
5152

5253
if kwargs.get("after") or kwargs.get("before"):

config/graphql/document_types.py

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -123,7 +123,7 @@ class Meta:
123123
_VISIBLE_CORPUS_IDS_CACHE_KEY = "_docpath_visible_corpus_ids"
124124

125125
@classmethod
126-
def _get_visible_corpus_ids(cls, info):
126+
def _get_visible_corpus_ids(cls, info) -> Any:
127127
"""Get visible corpus IDs with request-level caching to prevent N+1 queries."""
128128
from opencontractserver.corpuses.models import Corpus
129129

@@ -1179,19 +1179,19 @@ class Meta:
11791179
connection_class = CountableConnection
11801180

11811181

1182-
def _get_corpus_folder_type():
1182+
def _get_corpus_folder_type() -> Any:
11831183
from config.graphql.corpus_types import CorpusFolderType
11841184

11851185
return CorpusFolderType
11861186

11871187

1188-
def _get_corpus_action_type():
1188+
def _get_corpus_action_type() -> Any:
11891189
from config.graphql.agent_types import CorpusActionType
11901190

11911191
return CorpusActionType
11921192

11931193

1194-
def _get_extract_type():
1194+
def _get_extract_type() -> Any:
11951195
from config.graphql.extract_types import ExtractType
11961196

11971197
return ExtractType

config/graphql/extract_types.py

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -64,7 +64,7 @@ class Meta:
6464
connection_class = CountableConnection
6565

6666

67-
def _get_datacell_qs(extract, user):
67+
def _get_datacell_qs(extract, user) -> Any:
6868
"""Return the permission-filtered, deterministically ordered queryset.
6969
7070
Note: this is a module-level function because Graphene-Django resolvers

config/graphql/ingestion_source_mutations.py

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@
33
"""
44

55
import logging
6+
from typing import Any
67

78
import graphene
89
from django.db import IntegrityError
@@ -45,7 +46,7 @@ def _parse_ingestion_source_global_id(
4546
return pk, None
4647

4748

48-
def _resolve_source_type(source_type):
49+
def _resolve_source_type(source_type) -> Any:
4950
"""Coerce a graphene Enum to its string value, defaulting to MANUAL."""
5051
if source_type is None:
5152
return IngestionSourceCategory.MANUAL

0 commit comments

Comments
 (0)