Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Support for Proxy Models #319

Closed
jm2242 opened this issue Nov 6, 2017 · 13 comments
Closed

Support for Proxy Models #319

jm2242 opened this issue Nov 6, 2017 · 13 comments

Comments

@jm2242
Copy link
Contributor

jm2242 commented Nov 6, 2017

Is there support for querying proxy models? I'm trying to query a model that has a bunch of proxy models but am getting an error. Here's an abbreviated look at my setup:

class Column:
...

class FieldColumn(Column):
  proxy=True
class Query(graphene.ObjecType):
    column = graphene.Field(ColumnType)
    all_columns = DjangoFilterConnectionField(ColumnType)

and

class ColumnType(DjangoObjectType):
    class Meta:
        model = Column
        filter_fields = ['sheet__name', 'name']
        interfaces = (relay.Node, )
        exclude_fields = [  
            'legislation_type', 'project_stat_type', ... # weirdly have to exclude these fields because the 'type' in the variable name makes the field introspector sad
        ]

my query is:

query {
	allColumns(sheet_Name:"Simple Sheet") {
		edges {
      node{
        id, name
      }
    }
  }
}

Here's the trace:

Traceback (most recent call last):
  File "/Users/jonathanmares/.virtualenvs/quorum/lib/python2.7/site-packages/promise/promise.py", line 65, in try_catch
    return (handler(*args, **kwargs), None)
  File "/Users/jonathanmares/.virtualenvs/quorum/lib/python2.7/site-packages/graphql/execution/executor.py", line 375, in <lambda>
    resolved
  File "/Users/jonathanmares/.virtualenvs/quorum/lib/python2.7/site-packages/graphql/execution/executor.py", line 405, in complete_value
    return complete_object_value(exe_context, return_type, field_asts, info, result)
  File "/Users/jonathanmares/.virtualenvs/quorum/lib/python2.7/site-packages/graphql/execution/executor.py", line 499, in complete_object_value
    field_asts
graphql.error.base.GraphQLError: Expected value of type "ColumnType" but got: FieldColumn.

I know this PR https://github.com/graphql-python/graphene-django/pull/156/files is waiting to be merged, but I tried running my query with the proposed changes and still get the same error.

@spockNinja
Copy link
Contributor

Following that traceback, I think the root of the problem is here.

We are checking for _meta.model to match between the actual returned object and the model assigned in the DjangoObjectType meta.

So I think to support proxy models, we might want to check _meta.model._meta.concrete_model instead? I'm not sure on the django version support for concrete_model, but if it has always been there, that just might work. Otherwise, we might have to check _meta.model._meta.proxy and handle that case specially.

@spockNinja
Copy link
Contributor

I went back through django source to 1.7, and it looks like concrete_model has always been there. Would you like to whip up a Pull Request with that change and see if it solves the problem for you?

@jm2242
Copy link
Contributor Author

jm2242 commented Nov 16, 2017

excellent! that was it, thanks for your help. I'll get a PR up with a test or two and some docs

@serates
Copy link

serates commented Jan 13, 2018

@jm2242 Did you get this PR up? If so could you link it here so i could follow it since my issue should also be fixed by this PR.

@jm2242
Copy link
Contributor Author

jm2242 commented Jan 13, 2018

I have not, but I will do this today or tomorrow!

@filipeximenes
Copy link

filipeximenes commented Oct 8, 2018

Did this get fixed? Is there a temporary solution to it?

@jm2242
Copy link
Contributor Author

jm2242 commented Oct 9, 2018

Yes I believe this PR does address this, at least basic support #373

@jm2242 jm2242 closed this as completed Oct 9, 2018
@filipeximenes
Copy link

@jm2242 thanks for getting back. I just noticed that it was failing for me because I was trying to use it in model relationship and at the model level I was defining the relationship through the proxy model. So if someone finds this, just make sure you don't use proxy models in the ForeignKeys of your models.
Not super urgent, but should we treat this as bug? As in, should graphene know how to handle this case?

@jm2242
Copy link
Contributor Author

jm2242 commented Oct 9, 2018

Maybe post a minimal example, and someone can pick it up and fix it?

@filipeximenes
Copy link

filipeximenes commented Mar 22, 2019

A bit late but the example would be:

class Model1(models.Model):
     ....

class ProxyModel1(Model1):
    ....
    class Meta:
        proxy = True

class Model2(models.Model):
    relationship = models.ForeignKey(ProxyModel1)
    ....

@zbyte64
Copy link
Collaborator

zbyte64 commented Mar 22, 2019

The test case currently has an Article that has a foreign key to a proxy model Reporter

Currently at:

def test_proxy_model_support():

To match the example given here they foreign key would be adjusted to CNNReporter

@abettke
Copy link

abettke commented Mar 27, 2019

IRT #599, I've created #603 to try and better address this issue.

@dopeboy
Copy link

dopeboy commented May 8, 2019

Let's take this over to #603, thanks all.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

7 participants