Refactor create method to ensure proxy-model compatibility

[sdobbelaere]

As discussed in issue #388 proxy-models with manager overrides were not respected. This pull-request attempts to rectify that.

In short, the create() method has been refactored to no longer use update() - but instead leverage the ORM Manager functionality rather then the naive approach of manually setting an instance and calling save() at the end.

There are a few things that are worth considering:

• pre-save-hook is questionable - now being run after the create as there is no instance to run it on before creation. Line 301
• refresh_from_db() really needed? The update method seemed to do something similar. Unsure why. Line 324

Feedback, thought and improvements are welcomed.

EDIT:

• [x] Fix create mutations to respect objects.create approach
• [x] Ensure pre_save_hook as run in the traditional method

[codecov-commenter]

Codecov Report

Attention: 7 lines in your changes are missing coverage. Please review.

Comparison is base (b6126e0) 87.16% compared to head (bbb4010) 87.27%. Report is 1 commits behind head on main.

Files	Patch %	Lines
strawberry_django/mutations/resolvers.py	88.33%	7 Missing :warning:

Additional details and impacted files

```diff @@ Coverage Diff @@ ## main #394 +/- ## ========================================== + Coverage 87.16% 87.27% +0.11% ========================================== Files 36 37 +1 Lines 3100 3119 +19 ========================================== + Hits 2702 2722 +20 + Misses 398 397 -1 ```

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

[sdobbelaere]

Independently of this PR, the proxy models seem to be giving me one more issue.

They are returning the wrong parent type.

Following the example in #388

When querying for the object via it's id with node()

the companyType is returned instead of clientType. This works well in with connection() listings, but not with node().

Example:

query client{
  client(id:"Q29tcGFueVR5cGU6MTE="){
    id
    name
    isCustomer
  }
}

yields unsuccessfully

{
  "data": null,
  "errors": [
    {
      "message": "Expected value of type 'ClientType' but got: <Company instance>.",
      "locations": [
        {
          "line": 2,
          "column": 3
        }
      ],
      "path": [
        "customer"
      ]
    }
  ]
}

It seems to be very hard to track down, but is actually obstructing the proxy-models to work. From what I can tell, it seems to be related to the dynamic way models are fetched.

I've tried to track down the base source of where an object is fetched and how the query sets are determined. But seem to be chasing my tail with that. Also re-fetching the object doesn't seem to resolve the issue.

Perhaps someone with more in-depth knowledge of the library can figure this out?

[sdobbelaere]

Some notes on the type issue.

Let's use this query:

            query customer($id: GlobalID!) {
                customer(id: $id) {
                    id
                    name
                    __typename
                    isCustomer
                }
            }

Running that query with the strawberry-test-client correctly yields:

{
'customer': 
    {
    'id': 'Q3VzdG9tZXJUeXBlOjEz', 
     '__typename': 'CustomerType', 
    'isCustomer': True
    }
}

Yet when running it via Daphne in graphiql, the result is incorrectly a company-instance rather then the customer-instance.

{
  "data": null,
  "errors": [
    {
      "message": "Expected value of type 'CustomerType' but got: <Company instance>.",
      "locations": [
        {
          "line": 2,
          "column": 3
        }
      ],
      "path": [
        "customer"
      ]
    }
  ]
}

Some help on this PR would be greatly appreciated.

[bellini666]

It seems to be very hard to track down, but is actually obstructing the proxy-models to work. From what I can tell, it seems to be related to the dynamic way models are fetched.

I've tried to track down the base source of where an object is fetched and how the query sets are determined. But seem to be chasing my tail with that. Also re-fetching the object doesn't seem to resolve the issue.

Perhaps someone with more in-depth knowledge of the library can figure this out?

node() will be calling resolve_node()/resolve_nodes() from the type, which for django types get injected here when not defined: https://github.com/strawberry-graphql/strawberry-graphql-django/blob/main/strawberry_django/type.py#L206

Feel free to ping me on discord in case you want to discuss more about this. As I mentioned, I have not used proxy models so far, so I'm not really sure what adjustments that needs to be done here to make them work properly, but hopefully it should be a matter of details.

[sdobbelaere]

@bellini666 My apologies for the 10.000 commits. I believe I've finally managed to complete this PR, including the typing. Please re-review.

[bellini666]

@sdobbelaere I fixed the merge conflict for you, but tests are breaking now after the latest changes

[sdobbelaere]

@bellini666 can you check and merge if you're happy with it?
I did add some comments on strawberry_django/mutations/resolvers.py:472, as the code seems to be doing nothing from what I can tell. Tests are also passing when you remove it.