fix: relay.Node with overwritten resolve_id and without NodeId (#2844)

* fix: unneccessary local variable, error when overwritting resolve_id but
not providing a NodeId annotation

* add tests for overwritting resolve_id

* add Release.md

* fix: codecov should ignore lines, rename code to color

* Update RELEASE.md

Co-authored-by: Thiago Bellini Ribeiro <hackedbellini@gmail.com>

* Update tests/relay/test_schema.py

Co-authored-by: Thiago Bellini Ribeiro <hackedbellini@gmail.com>

* fix code suggestion

* move id_attr check to __init_subclass__

* Revert "move id_attr check to __init_subclass__": this would prevent
non-final Node Subclasses

This reverts commit 758bc50.

---------

Co-authored-by: Thiago Bellini Ribeiro <hackedbellini@gmail.com>

RELEASE.md

@@ -0,0 +1,3 @@
+Release type: patch
+
+Don't require `NodeId` annotation if resolve_id is overwritten on `Node` implemented types

strawberry/relay/types.py

@@ -217,7 +217,6 @@ def resolve_type(self, info: Info) -> Type[Node]:
             The resolved GraphQL type for the execution info
 
         """
-        schema = info.schema
         type_def = info.schema.get_type_by_name(self.type_name)
         assert isinstance(type_def, StrawberryObjectDefinition)
 

strawberry/schema/schema.py

@@ -334,7 +334,16 @@ def _resolve_node_ids(self):
             # early feedback for missing NodeID annotations
             origin = type_def.origin
             if issubclass(origin, relay.Node):
-                origin.resolve_id_attr()
+                has_custom_resolve_id = False
+                for base in origin.__mro__:
+                    if base is relay.Node:
+                        break
+                    if "resolve_id" in base.__dict__:
+                        has_custom_resolve_id = True
+                        break
+
+                if not has_custom_resolve_id:
+                    origin.resolve_id_attr()
 
     def _warn_for_federation_directives(self):
         """Raises a warning if the schema has any federation directives."""

tests/relay/test_schema.py

@@ -356,3 +356,50 @@ def fruits(self) -> List[Fruit]:
             "edges": [{"node": {"id": to_base64("Fruit", i)}} for i in range(10)]
         }
     }
+
+
+def test_overwrite_resolve_id_and_no_node_id(mocker: MockerFixture):
+    mocker.patch.object(
+        DEFAULT_SCALAR_REGISTRY[relay.GlobalID],
+        "description",
+        "__GLOBAL_ID_DESC__",
+    )
+
+    @strawberry.type
+    class Fruit(relay.Node):
+        color: str
+
+        @classmethod
+        def resolve_id(cls, root) -> str:
+            return "test"  # pragma: no cover
+
+    @strawberry.type
+    class Query:
+        fruit: Fruit
+
+    expected_type = textwrap.dedent(
+        '''
+          type Fruit implements Node {
+          """The Globally Unique ID of this object"""
+          id: GlobalID!
+          color: String!
+        }
+
+        """__GLOBAL_ID_DESC__"""
+        scalar GlobalID @specifiedBy(url: "https://relay.dev/graphql/objectidentification.htm")
+
+        """An object with a Globally Unique ID"""
+        interface Node {
+          """The Globally Unique ID of this object"""
+          id: GlobalID!
+        }
+
+        type Query {
+          fruit: Fruit!
+        }
+        '''
+    )
+
+    schema = strawberry.Schema(query=Query)
+
+    assert str(schema) == textwrap.dedent(expected_type).strip()

tests/relay/test_types.py

@@ -241,3 +241,21 @@ async def some_type_conn(self) -> AsyncIterable[SomeType]:
             ],
         }
     }
+
+
+def test_overwrite_resolve_id_and_no_node_id():
+    @strawberry.type
+    class Fruit(relay.Node):
+        color: str
+
+        @classmethod
+        def resolve_id(cls, root) -> str:
+            return "test"  # pragma: no cover
+
+    @strawberry.type
+    class Query:
+        @strawberry.field
+        def fruit(self) -> Fruit:
+            return Fruit(color="red")  # pragma: no cover
+
+    strawberry.Schema(query=Query)