From a31b5c4d472b4c3a4bfa02fb13d593d4e016963a Mon Sep 17 00:00:00 2001
From: Alexander <devkral@web.de>
Date: Tue, 12 Dec 2023 18:13:52 +0100
Subject: [PATCH] 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 758bc502c873ac1fdb6e0ef347c2cb88c436693e.

---------

Co-authored-by: Thiago Bellini Ribeiro <hackedbellini@gmail.com>
---
 RELEASE.md                  |  3 +++
 strawberry/relay/types.py   |  1 -
 strawberry/schema/schema.py | 11 ++++++++-
 tests/relay/test_schema.py  | 47 +++++++++++++++++++++++++++++++++++++
 tests/relay/test_types.py   | 18 ++++++++++++++
 5 files changed, 78 insertions(+), 2 deletions(-)
 create mode 100644 RELEASE.md

diff --git a/RELEASE.md b/RELEASE.md
new file mode 100644
index 0000000000..903f873bbc
--- /dev/null
+++ b/RELEASE.md
@@ -0,0 +1,3 @@
+Release type: patch
+
+Don't require `NodeId` annotation if resolve_id is overwritten on `Node` implemented types
diff --git a/strawberry/relay/types.py b/strawberry/relay/types.py
index dcb1f8fbbc..200452c0f4 100644
--- a/strawberry/relay/types.py
+++ b/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)
 
diff --git a/strawberry/schema/schema.py b/strawberry/schema/schema.py
index 8fae4ca303..dcb961baad 100644
--- a/strawberry/schema/schema.py
+++ b/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."""
diff --git a/tests/relay/test_schema.py b/tests/relay/test_schema.py
index 4f727e8a33..dbe98aee03 100644
--- a/tests/relay/test_schema.py
+++ b/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()
diff --git a/tests/relay/test_types.py b/tests/relay/test_types.py
index 6fcb4529f0..3887454b36 100644
--- a/tests/relay/test_types.py
+++ b/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)