Repository: mirumee/ariadne-codegen (python)

Issue:
Mixin Strategy Produces Invalid Queries
### Summary
The Mixins feature is a great idea for extending the functionality of generated classes, however the mixin directive is not removed from the final generated query, resulting in errors returned from remote APIs.


### Example
Let's say that we simply want to validate that a mixin has been applied by returning a message:

```python
class RepositoryMixin:
    def test_mixin(self):
        return "This is a mixin for the Repository object!"
```
We then add it to the config TOML and include it in our GraphQL query:

```toml
files_to_include = [
    "./codegen/github_enterprise/mixins/repository_mixin.py",
]
```

```gql
query get_repository($repo_owner: String!, $repo_name: String!) {
  repository(name: $repo_name, owner: $repo_owner)
    @mixin(from: ".repository_mixin", import: "RepositoryMixin") {
    nameWithOwner
    description
  }
}
```

Looking at the generated Pydantic `BaseModel` we can see that the mixin was applied successfully:

```python
class GetRepositoryRepository(BaseModel, RepositoryMixin):
    name_with_owner: str = Field(alias="nameWithOwner")
    description: Optional[str]
```

However, the final generated query in the final generated GraphQL Client still contains the mixin directive used previously to generate our Pydantic model that inherits the mixin:

<img width="801" alt="codegen-bad-generated-query" src="https://github.com/mirumee/ariadne-codegen/assets/31744764/da4ae62c-4e13-4b09-88f1-7c5b40487bce">
<br />
<br />

Executing the `GithubEnterpriseClient.get_repository` method with this query results in `GraphQLClientHttpError: HTTP status code: 500`. Removing the mixin from the generated query fixes the error and returns the requested data, which contains a Repository on which you can successfully call the `Repository.test_mixin` method and get the expected output: `This is a mixin for the Repository object.`

### Fix


- Is the `@mixin` directive intended to be used exclusively at generation-time to inject the proper class into the inheritance chain of a given Pydantic model? 
  - If yes, then can this issue be fixed by simply programming the codegen business logic to remove the mixin from the final generated query in the GraphQL client? 
  - If no,  what is the use-case of preserving the mixin in the final generated query and how can one use it without remote API errors?




Pull request:
Remove mixin directive from operation string sent to server
This pr resolves #155 

Patch:
diff --git a/CHANGELOG.md b/CHANGELOG.md
index e243479..986a37a 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -10,6 +10,7 @@
 - Fixed handling of inline fragments inside other fragments.
 - Changed generated unions to use pydantic's discriminated unions feature.
 - Replaced HTTPX's `json=` serializer for query payloads with pydantic's `pydantic_encoder`.
+- Removed `mixin` directive from operation string sent to server.
 
 
 ## 0.6.0 (2023-04-18)
diff --git a/ariadne_codegen/client_generators/result_types.py b/ariadne_codegen/client_generators/result_types.py
index 1a1e9e1..5514d74 100644
--- a/ariadne_codegen/client_generators/result_types.py
+++ b/ariadne_codegen/client_generators/result_types.py
@@ -1,5 +1,6 @@
 import ast
-from typing import Dict, List, Optional, Set, Tuple, cast
+from copy import deepcopy
+from typing import Any, Dict, List, Optional, Set, Tuple, cast
 
 from graphql import (
     DirectiveNode,
@@ -21,8 +22,10 @@ from graphql import (
     SelectionNode,
     SelectionSetNode,
     StringValueNode,
+    Visitor,
     is_abstract_type,
     print_ast,
+    visit,
 )
 
 from ..codegen import (
@@ -167,7 +170,9 @@ class ResultTypesGenerator:
         return self._class_defs
 
     def get_operation_as_str(self) -> str:
-        operation_str = print_ast(self.operation_definition)
+        operation_str = print_ast(
+            self._get_operation_definition_without_mixin_directive()
+        )
         if self._fragments_used_as_mixins or self._unpacked_fragments:
             for used_fragment in sorted(self._get_all_related_fragments()):
                 operation_str += "\n\n" + print_ast(
@@ -518,3 +523,16 @@ class ResultTypesGenerator:
             elif isinstance(node, FieldNode) and node.selection_set:
                 names = names.union(self._get_fragments_names(node.selection_set))
         return names
+
+    def _get_operation_definition_without_mixin_directive(self):
+        class RemoveMixinVisitor(Visitor):
+            @staticmethod
+            def enter_field(node: FieldNode, *_args: Any) -> FieldNode:
+                node.directives = tuple(
+                    d for d in node.directives or [] if d.name.value != MIXIN_NAME
+                )
+                return node
+
+        operation_def = deepcopy(self.operation_definition)
+        visit(operation_def, RemoveMixinVisitor())
+        return operation_def


Test patch:
diff --git a/tests/client_generators/result_types_generator/test_get_operation_as_str.py b/tests/client_generators/result_types_generator/test_get_operation_as_str.py
index ba924d7..47720af 100644
--- a/tests/client_generators/result_types_generator/test_get_operation_as_str.py
+++ b/tests/client_generators/result_types_generator/test_get_operation_as_str.py
@@ -7,6 +7,7 @@ from graphql import (
     parse,
 )
 
+from ariadne_codegen.client_generators.constants import MIXIN_NAME
 from ariadne_codegen.client_generators.result_types import ResultTypesGenerator
 
 from ...utils import format_graphql_str
@@ -158,3 +159,39 @@ def test_get_operation_as_str_returns_str_with_fragment_used_by_another_fragment
 
     assert "fragment TestFragment on CustomType" in result
     assert "fragment NestedFragment on CustomType2" in result
+
+
+def test_get_operation_as_str_returns_representation_without_mixin_directive():
+    query_str = format_graphql_str(
+        """
+        query CustomQuery {
+            query2 @mixin(from: ".mixins", import: "TestMixin") {
+                id
+                field1 @mixin(from: ".mixins", import: "TestMixin1") {
+                    fielda
+                }
+                field2 @mixin(from: ".mixins", import: "TestMixin2") {
+                    fieldb
+                }
+            }
+        }
+        """
+    )
+    generator = ResultTypesGenerator(
+        schema=build_ast_schema(parse(SCHEMA_STR)),
+        operation_definition=cast(
+            OperationDefinitionNode, parse(query_str).definitions[0]
+        ),
+        enums_module_name="enums",
+    )
+
+    result = generator.get_operation_as_str()
+
+    assert MIXIN_NAME not in result
+    # tests that operation definition still contains mixin directive
+    field_query2 = generator.operation_definition.selection_set.selections[0]
+    field_field1 = field_query2.selection_set.selections[1]
+    field_field2 = field_query2.selection_set.selections[2]
+    assert field_query2.directives[0].name.value == MIXIN_NAME
+    assert field_field1.directives[0].name.value == MIXIN_NAME
+    assert field_field2.directives[0].name.value == MIXIN_NAME
diff --git a/tests/main/clients/extended_models/expected_client/client.py b/tests/main/clients/extended_models/expected_client/client.py
index 5df95fd..4d92509 100644
--- a/tests/main/clients/extended_models/expected_client/client.py
+++ b/tests/main/clients/extended_models/expected_client/client.py
@@ -12,7 +12,7 @@ class Client(AsyncBaseClient):
         query = gql(
             """
             query getQueryA {
-              queryA @mixin(from: ".mixins_a", import: "MixinA") @mixin(from: ".common_mixins", import: "CommonMixin") {
+              queryA {
                 fieldA
               }
             }
@@ -27,7 +27,7 @@ class Client(AsyncBaseClient):
         query = gql(
             """
             query getQueryB {
-              queryB @mixin(from: ".mixins_b", import: "MixinB") @mixin(from: ".common_mixins", import: "CommonMixin") {
+              queryB {
                 fieldB
               }
             }