Fix-293-default-enum-values #533

kuchichan · 2021-04-09T12:45:26Z

codecov · 2021-04-09T15:26:38Z

Codecov Report

Merging #533 (87caafb) into master (4f32907) will increase coverage by 0.04%.
The diff coverage is 99.47%.

@@            Coverage Diff             @@
##           master     #533      +/-   ##
==========================================
+ Coverage   98.21%   98.25%   +0.04%     
==========================================
  Files         104      104              
  Lines        5332     5513     +181     
==========================================
+ Hits         5237     5417     +180     
- Misses         95       96       +1

Impacted Files	Coverage Δ
ariadne/enums.py	`98.36% <98.90%> (+1.48%)`	⬆️
ariadne/executable_schema.py	`100.00% <100.00%> (ø)`
tests/test_enums.py	`100.00% <100.00%> (ø)`

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 4f32907...87caafb. Read the comment docs.

rafalp · 2021-04-12T13:02:06Z

ariadne/executable_schema.py

@@ -37,8 +54,103 @@ def make_executable_schema(

    assert_valid_schema(schema)

+    for type_name, field_name, arg, _ in find_enum_values_in_schema(schema):


This code block should live in separate validate_schema_enum_values function

rafalp · 2021-04-12T13:03:08Z

ariadne/executable_schema.py

    return schema


 def join_type_defs(type_defs: List[str]) -> str:
    return "\n\n".join(t.strip() for t in type_defs)
+
+
+def find_enum_values_in_schema(


Lets move this util somewhere else so executable_schema.py stays at current small and sweet size.

rafalp · 2021-04-14T20:26:37Z

ariadne/enums.py

+    return routes
+
+
+def is_invalid_enum_value(field: Union[GraphQLInputField, GraphQLArgument]) -> bool:


Move this function under validate_schema_enum_values

rafalp · 2021-04-14T20:26:56Z

ariadne/enums.py

+    return field.default_value is Undefined and field.ast_node.default_value is not None
+
+
+def validate_schema_enum_values(schema: GraphQLSchema) -> None:


Move this function before find_enum_values_in_schema.

rafalp · 2021-04-14T20:28:28Z

ariadne/executable_schema.py

@@ -22,23 +26,42 @@ def make_executable_schema(

    ast_document = parse(type_defs)
    schema = build_ast_schema(ast_document)
+    cleaned_bindables: List[SchemaBindable] = clean_bindables(*bindables)


Could be inlined in for bindable in clean_bindables(*bindables):

I would agree, but please notice, that, clean_binables (now flat_binables) are also passed in repair_default_enum_values. That is the reason of extraction of that variable.

rafalp · 2021-04-14T20:28:34Z

ariadne/executable_schema.py


    return schema


 def join_type_defs(type_defs: List[str]) -> str:
    return "\n\n".join(t.strip() for t in type_defs)
+
+
+def clean_bindables(


Suggested change

def clean_bindables(

def flatten_bindables(

rafalp · 2021-04-14T20:31:46Z

tests/test_enums.py

+    """
+    query = QueryType()
+
+    def resolv(*_, value):


Suggested change

def resolv(*_, value):

def resolve_test_enum(*_, value):

Done (for all lines, where it occurs).

rafalp · 2021-04-14T20:34:28Z

tests/test_enums.py

+    assert result.data["complex"]
+
+
+def test_input_exc_schema_should_raise_an_exception_if_undefined_enum_flat_input():


Suggested change

def test_input_exc_schema_should_raise_an_exception_if_undefined_enum_flat_input():

def test_input_exc_schema_raises_exception_for_undefined_enum_value_in_flat_input():

rafalp · 2021-04-14T20:34:53Z

tests/test_enums.py

+        make_executable_schema([enum_definition, input_schema])
+
+
+def test_input_exc_schema_should_raise_an_exception_if_undefined_enum_in_nested_input():


Suggested change

def test_input_exc_schema_should_raise_an_exception_if_undefined_enum_in_nested_input():

def test_input_exc_schema_raises_exception_for_undefined_enum_value_in_nested_object():

rafalp · 2021-04-14T20:35:40Z

tests/test_enums.py

+        make_executable_schema([enum_definition, input_schema])
+
+
+def test_input_exc_schema_should_raise_an_exception_if_undefined_enum_in_query():


Suggested change

def test_input_exc_schema_should_raise_an_exception_if_undefined_enum_in_query():

def test_input_exc_schema_raises_exception_for_undefined_enum_value_in_nested_field_arg():

rafalp · 2021-04-14T20:37:34Z

tests/test_enums.py

+    assert len(undefined) == number_of_undefined_default_enum_values
+
+
+def test_issue_293():


Test name should shortly reflect what behaviour is tested, with docstring (or plain comment) mentioning that its a regression test for issue, eg.:

def test_something(): # regression test for https://github.com/mirumee/issues/31321

rafalp · 2021-04-14T20:43:24Z

ariadne/enums.py

+def find_enum_values_in_schema(
+    schema: GraphQLSchema,
+) -> Generator[Union[ArgumentWithKeys, InputFieldWithKeys], None, None]:
+    object_types = (


Not a biggest fan of iterating schema twice to create two lists that are then iterated over again.

I have refactored it a little bit, now I rely on functools's singledispatch.

And now we iterating only once ofc ;d

rafalp · 2021-08-24T17:38:00Z

ariadne/enums.py

+    cast,
+)
+from functools import reduce, singledispatch
+import operator


Move this import under import enum

rafalp · 2021-08-24T17:39:51Z

ariadne/enums.py

+
+
+def get_value_from_mapping_value(mapping: Mapping[T, Any], key_list: List[T]) -> Any:
+    return reduce(operator.getitem, key_list, mapping)


Why not run this in for loop?

Hmm no particular reason, I found reduce more appropriate to apply collection of arguments to dict

kuchichan added 2 commits April 9, 2021 14:41

Create initial functions for validating enums in inputs

8be7635

Create iterator

90fcff1

Create iterator for enums, validation of enums

cd3e22e

rafalp suggested changes Apr 12, 2021

View reviewed changes

kuchichan added 4 commits April 12, 2021 16:03

Moving util functions to enums.py + validate_function

3f1d1ad

create function that transforms str to internal enum value

aea7869

Additional test related to bug

d317ffb

Small changes in issue test

b90e13c

kuchichan force-pushed the fix-293-default-values-schema branch from c9478bc to b90e13c Compare April 13, 2021 13:53

rafalp suggested changes Apr 14, 2021

View reviewed changes

Changes related to review

87caafb

kuchichan marked this pull request as ready for review April 15, 2021 08:26

kuchichan changed the title ~~Create initial functions for validating enums in inputs~~ Fix-293-default-enum-values Apr 15, 2021

rafalp reviewed Aug 24, 2021

View reviewed changes

ariadne/enums.py

cast,

)

from functools import reduce, singledispatch

import operator

Copy link

Contributor

rafalp Aug 24, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Move this import under import enum

rafalp reviewed Aug 24, 2021

View reviewed changes

rafalp approved these changes Oct 15, 2021

View reviewed changes

rafalp merged commit 20f2b90 into master Oct 15, 2021

rafalp deleted the fix-293-default-values-schema branch October 15, 2021 10:25

rafalp mentioned this pull request Dec 3, 2021

Fix: 293: use python represenation of enum values for default values in schema #482

Closed

6 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Fix-293-default-enum-values #533

Fix-293-default-enum-values #533

kuchichan commented Apr 9, 2021 •

edited by rafalp

Loading

codecov bot commented Apr 9, 2021 •

edited

Loading

rafalp Apr 12, 2021

kuchichan Apr 15, 2021

rafalp Apr 12, 2021

kuchichan Apr 15, 2021

rafalp Apr 14, 2021

kuchichan Apr 15, 2021

rafalp Apr 14, 2021

kuchichan Apr 15, 2021

rafalp Apr 14, 2021

kuchichan Apr 15, 2021

rafalp Apr 14, 2021

kuchichan Apr 15, 2021

rafalp Apr 14, 2021

kuchichan Apr 15, 2021

rafalp Apr 14, 2021

kuchichan Apr 15, 2021

rafalp Apr 14, 2021

kuchichan Apr 15, 2021

rafalp Apr 14, 2021

kuchichan Apr 15, 2021

rafalp Apr 14, 2021

kuchichan Apr 15, 2021

rafalp Apr 14, 2021

kuchichan Apr 15, 2021

kuchichan Apr 15, 2021

rafalp Aug 24, 2021

rafalp Aug 24, 2021

kuchichan Aug 26, 2021

		@@ -37,8 +54,103 @@ def make_executable_schema(

		assert_valid_schema(schema)

		for type_name, field_name, arg, _ in find_enum_values_in_schema(schema):

		return routes


		def is_invalid_enum_value(field: Union[GraphQLInputField, GraphQLArgument]) -> bool:

		return field.default_value is Undefined and field.ast_node.default_value is not None


		def validate_schema_enum_values(schema: GraphQLSchema) -> None:

		assert result.data["complex"]


		def test_input_exc_schema_should_raise_an_exception_if_undefined_enum_flat_input():

	def test_input_exc_schema_should_raise_an_exception_if_undefined_enum_flat_input():
	def test_input_exc_schema_raises_exception_for_undefined_enum_value_in_flat_input():

		make_executable_schema([enum_definition, input_schema])


		def test_input_exc_schema_should_raise_an_exception_if_undefined_enum_in_nested_input():

		assert len(undefined) == number_of_undefined_default_enum_values


		def test_issue_293():



		def get_value_from_mapping_value(mapping: Mapping[T, Any], key_list: List[T]) -> Any:
		return reduce(operator.getitem, key_list, mapping)

Fix-293-default-enum-values #533

Fix-293-default-enum-values #533

Conversation

kuchichan commented Apr 9, 2021 • edited by rafalp Loading

codecov bot commented Apr 9, 2021 • edited Loading

Codecov Report

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

kuchichan commented Apr 9, 2021 •

edited by rafalp

Loading

codecov bot commented Apr 9, 2021 •

edited

Loading