Skip to content

Commit

Permalink
fix: False Positives in Default Resolver Setup
Browse files Browse the repository at this point in the history
During the configuration of default resolvers for Frappe DocTypes and fields, the type_name is utilized to determine if it is a doctype. Additional resolver setup is then performed on the fields of the identified doctype.


However, there are instances where GraphQLEnumTypes have similar names to doctypes, in which case, further resolver setup should not be applied. This PR ensures that resolver setup is only performed on GraphQLObjectTypes.

Co-authored-by: Fahim Ali Zain <[email protected]>

Merge-request: ROMMAN-MR-627
Merged-by: Fahim Ali Zain <[email protected]>
  • Loading branch information
fahimalizain authored and Space committed Jan 23, 2023
1 parent 807d355 commit fd34023
Show file tree
Hide file tree
Showing 2 changed files with 44 additions and 3 deletions.
7 changes: 5 additions & 2 deletions frappe_graphql/utils/resolver/__init__.py
Original file line number Diff line number Diff line change
@@ -1,4 +1,7 @@
from graphql import GraphQLSchema, GraphQLType, GraphQLResolveInfo, GraphQLNonNull
from graphql import (
GraphQLSchema, GraphQLType, GraphQLResolveInfo,
GraphQLNonNull, GraphQLObjectType
)

import frappe
from frappe.model.meta import Meta
Expand All @@ -19,7 +22,7 @@ def setup_default_resolvers(schema: GraphQLSchema):
# Setup custom resolvers for DocTypes
for type_name, gql_type in schema.type_map.items():
dt = get_singular_doctype(type_name)
if not dt:
if not dt or not isinstance(gql_type, GraphQLObjectType):
continue

meta = frappe.get_meta(dt)
Expand Down
40 changes: 39 additions & 1 deletion frappe_graphql/utils/resolver/tests/test_document_resolver.py
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@
import frappe

from frappe_graphql.graphql import get_schema, execute
from graphql import GraphQLArgument, GraphQLField, GraphQLScalarType, GraphQLString
from graphql import GraphQLArgument, GraphQLField, GraphQLScalarType, GraphQLString, GraphQLEnumType

"""
The following aspects of Document Resolver is tested here:
Expand Down Expand Up @@ -60,6 +60,44 @@ def test_get_administrator(self):
self.assertEqual(admin.get("name"), "Administrator")
self.assertEqual(admin.get("full_name"), "Administrator")

def test_enum_type_with_doctype_name(self):
"""
Default Resolvers are setup only for Doctype GQLTypes
The GQLType name is used to determine if it's a DocType or not.
There are cases where some other GQLType (eg: GQLEnumType) is named as a DocType.
In such cases, the default resolver should not be set. Trying to set it will result in
an error.
We will make a GQLEnumType named "ToDo" which is a valid Frappe Doctype.
We will try to ping on the schema to validate no errors are thrown.
"""
from .. import setup_default_resolvers

schema = get_schema()
todo_enum = GraphQLEnumType(
name="ToDo",
values={
"TODO": "TODO",
"DONE": "DONE"
}
)
schema.type_map["ToDo"] = todo_enum

# Rerun setup_default_resolvers
setup_default_resolvers(schema)

# Run Ping! to validate schema
r = execute(
query="""
query Ping {
ping
}
"""
)
self.assertIsNone(r.get("errors"))
self.assertEqual(r.get("data").get("ping"), "pong")

"""
LINK_FIELD_TESTS
"""
Expand Down

0 comments on commit fd34023

Please sign in to comment.