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

Add Documentation for Relay Edge implementation #59

Closed
gregziegan opened this issue Dec 1, 2015 · 28 comments
Closed

Add Documentation for Relay Edge implementation #59

gregziegan opened this issue Dec 1, 2015 · 28 comments

Comments

@gregziegan
Copy link

In the Relay mutation docs, performing RANGE_ADD requires that the new edge created by the mutation is provided in the payload.

I do not know how to provide the correct cursor/Edge construction within the relay.ClientIDMutation subclass.

The following code I wrote:

new_object_edge = graphene.Field('ObjectEdge')

I believed would solve my issue, with a proper resolve_new_object_edge(...) resolver.

However, my resolver cannot reference the relay.ConnectionField that is specified on one of the other object types in my mutation, since it only has the context of my (in this case, Django) objects.

I experimented with relay.Edge.for_node(SomeObjectType) as well but all of the solutions I have tried so far modify the schema correctly but cannot return the appropriate Edge.

The implementation of this scenario in javascript can be found here in the Relay examples directory.

Any idea of a best approach? I can take this question to stack overflow, but felt that people using Relay & Graphene would enjoy seeing an example solution in the docs.

@syrusakbary
Copy link
Member

@thebritican I'm still thinking a good way for handling the case you commented.

@gregziegan
Copy link
Author

While we investigate a solution to this, is there a way to incorporate an Edge as the payload and then write schema with vanilla python-graphql objects? Basically, can I write this in Python and reference it as an output field?

If not, my next intermediate solution was to produce the schema with Graphene and then introspect the produced schema, modifying the payload.

As for how Graphene could implement this: we only need a cursor_id field to be returned as well as node. I'm not sure how cursor_id is computed, but if we compute that string via the Edge constructor and pass it the relevant node, would that suffice? I was trying to use cursorForObjectInConnection to no avail.

@syrusakbary
Copy link
Member

@thebritican meanwhile you can use raw graphql-core objects as fields.

So this would work:

class AddTodo(relay.Mutation):
    class Input:
        text = graphene.String(required=True)
    todoEdge = graphene.Field(theGraphQLCoreType)

I will try to have this working with graphene around this week.

PS: Graphene is using graphql-relay library under the hoods, but the function cursor_for_object_in_connection is not yet implemented there, so probably I will implement this in graphql-relay before anything else.

@gregziegan
Copy link
Author

I see the cursor_for_object_in_connection and it appears functionally equivalent to the JS repo (and passes on the corresponding tests). Until it is working(?), however, using a graphQLCoreType is tough since the aforementioned method is used to generate the opaque cursor string. I'm assuming I can just generate a naive cursor in the meantime, but querying with before and after would likely be broken when relay tries to reconcile the edge with an existing connection list.

I'll take a look at this next week if it's still open and try a PR. Thanks for the great work so far!

@mikberg
Copy link

mikberg commented Jan 14, 2016

@thebritican Did you find a work-around for this? I'm creating a cursor via offset_to_cursor and trying to return it as an edge, but can't get it to work.

@syrusakbary What would the "theGraphQLCoreType" be? My connection is to objects of type Comment, so I've been trying to use CommentEdge. It produces the correct schema, the execution errors with Cannot return null for non-nullable field CommentEdge.cursor, which I've tried to set in mutate_and_get_payload in a couple of different ways ...

@rafales
Copy link

rafales commented Jan 27, 2016

Current implementation of cursor_for_object_in_connection relies heavily on the fact that data is provided as a list. What about Django ORM/SQLAlchemy where there may be thousands of records? This would cause to fetch them all. It doesn't seem like a good idea.

@adamcharnock
Copy link
Contributor

@syrusakbary I'm currently struggling with this too. I have something like this:

class CreateEvent(graphene.Mutation):
  class Input:
    description = graphene.String()
    start_date = graphene.String()
    end_date = graphene.String()
    # ...
    all_day = graphene.Boolean()

  event_edge = graphene.Field('EventNodeEdge')
  event = graphene.Field(EventNode)

  @classmethod
  def mutate(cls, instance, args, info):
    event = Event.objects.create(**args)
    cursor = cursor_for_object_in_connection([e.id for e in Event.objects.all()], event.id)

    assert cursor
    return cls(
      event=event,
      # Side question: Which Edge class should this be? I see two:
      #     graphql_relay.connection.connectiontypes.Edge
      #     graphene.relay.Edge
      event_edge=Edge(
        node=event,
        cursor=cursor,
      )
    )

Currently I'm receiving "Cannot return null for non-nullable field EventNodeEdge.cursor.". However, cursor_for_object_in_connection() is returning a string value.

Any help here would be very welcome indeed!

@adamcharnock
Copy link
Contributor

This is driving me up the wall. Still receiving Cannot return null for non-nullable field EventNodeEdge.cursor errors. I'd be willing to offer a bounty for some clear documentation if it can be done ASAP.

@syrusakbary
Copy link
Member

@adamcharnock I will work on this ASAP (is superbowl here so probably in the weekend will be difficult).
No need for bounties 😉

@adamcharnock
Copy link
Contributor

Thank you very much @syrusakbary, that'll be really useful :-)

For now I've managed a temporary workaround by just doing this.props.relay.forceFetch() after the mutation has been executed, thereby reloading all currently rendered data from the server.

@defrex
Copy link
Contributor

defrex commented Feb 10, 2016

BTW, a couple tips for anyone else looking at this. You can grab a reference to a Relay edge like so.

from graphene.relay.types import Edge

MyEdge = Edge.for_node(MyNode)

Also, if you're using array cursors for now, this is probably the most performant option.

from graphql_relay.connection.arrayconnection import cursor_for_object_in_connection

cursor_for_object_in_connection(list(MyModel.object.values_list('id', flat=True)), my_model.id)

@syrusakbary
Copy link
Member

@adamcharnock I'm revising this now.
Are you using graphene.Mutation but it seems you should be using relay.ClientIDMutation and mutate_and_get_payload function, as Edges are only relevant to Nodes.

Let me know if that's the problem.

@adamcharnock
Copy link
Contributor

@syrusakbary I could swear I replied to this! Sorry for the delay. I had actually using both relay.ClientIDMutation and 'mutate_and_get_payload ' with no luck. I may be working on this again soon so it would be great if I could figure this stuff out :-)

@syrusakbary
Copy link
Member

Let me know! I would like to find a good solution for this!! :)

@adamcharnock
Copy link
Contributor

@syrusakbary Will do.

To clarify, what I mean is I think that a working example in the docs would really help a lot. I felt I had run out of avenues to explore when I last looked at this.

@mickeyinfoshan
Copy link

Any new progress about this issue?

@mickeyinfoshan
Copy link

I've found a tricky way to get the new edge, which may help.
use offset_to_cursor in graphql_relay.connection.arrayconnection.
Here's an example:

class AddTodo(relay.ClientIDMutation):

    class Input:
        text = String(required=True)

    viewer = Field(User)
    todoEdge = Field(relay.types.Edge.for_node(Todo))

    @classmethod
    def mutate_and_get_payload(cls, input, info):
        viewer = get_viewer(info.request_context)
        todo = viewer.todos.create(text=input.get("text"))

       # get cursor here!
        cursor = offset_to_cursor(viewer.todos.count() - 1)

        edge = relay.types.Edge.for_node(Todo)(node=todo, cursor=cursor)
        return AddTodo(viewer=viewer, todoEdge=edge)

@defrex
Copy link
Contributor

defrex commented Apr 25, 2016

@mickeyinfoshan that assumes your edge is at the end of the list. If it's somewhere in the list, but you don't know the offset, this won't work.

@defrex
Copy link
Contributor

defrex commented Apr 25, 2016

Fundamentally, index-based cursors are never going to work, because you need the full context of the list to know what the index will be. In a relay context, that means a different cursor for every sort/filter combo that might be addressing the edge. The data required for that is out of control, IMO.

Stepping back, the only requirement for a cursor is that you can select before and after it. Thus, the simplest type of non-index cursor is the sort value for the list. For example, say you have a list of objects with created_date fields, ordered by created_date. In that case, your cursor could literally be the created_date value for the object, since you can always .filter(created_date__gt=cursor) to slice the results.

Optionally, if you want the sort field to be dynamic, you can use an object's unique id. In that case you would need to query the sort value matching that id and perform a similar query.

As far as I can tell, once of these two options is required to solve this issue.

@mickeyinfoshan
Copy link

@defrex It is just a tricky way to get the new cursor while performing a RANGE_ADD request, where the new edge should be the last one. It's not a clean one but a fast one for the reason that you don't need to iterate the list at all. In addition, RANGE_ADD is the only situation that a cursor is required on the server-side explicitly, as far as I'm concerned.

@varunarora
Copy link

Is there any temporary version of documentation on this? Perhaps in another branch?

@syrusakbary
Copy link
Member

syrusakbary commented Aug 6, 2016

@varunarora @defrex @thebritican in the version 1.0 of Graphene Relay edges and connections are created in a easier, cleaner and more understandable way.

Please take a look!
https://github.com/graphql-python/graphene/blob/master/graphene/relay/tests/test_connection.py

@richburdon
Copy link

Broken link (https://github.com/graphql-python/graphene/blob/1.0/graphene/relay/tests/test_connection.py)

Also, the official example doesn't seem to implement creating edges in the mutation.
https://github.com/graphql-python/graphene/blob/master/examples/starwars_relay/schema.py

So I'm still getting:

Error: Cannot query field "createItemEdge" on type "CreateItemMutationPayload".

Because My parent class defines:
items = relay.ConnectionField(Item)

@varuna82
Copy link

varuna82 commented Oct 19, 2016

Here's how I create an edge. There might be a better solutions, but hope this will help for now.

from graphql_relay.connection.arrayconnection import offset_to_cursor
cursor = offset_to_cursor(0)
edge_type = TestNode.Connection.Edge or Edge
edge = edge_type(cursor=cursor, node=new_event)

Following link explains how to create custom connection.
http://docs.graphene-python.org/en/latest/relay/connection/

@richburdon
Copy link

Thanks @varuna82 really helpful.

@reinierpd
Copy link

reinierpd commented May 31, 2017

I'm using graphene-django 1.3 and neither relay.types.Edge or relay.Edge exist. Any idea from where can i import Edge ?? @mickeyinfoshan

@syrusakbary
Copy link
Member

@reinierpd Edge is a very custom ObjectType that depends on the node of the connection, therefore will be a new Edge type per Connection node.
Once you have a specific Connection type (MyCustomConnection), you can easily retrieve it's edge with MyCustomConnection.Edge.

Closing the issue as I think Connections are better defined in later versions of Graphene.
Please comment or reopen if you feel there is still not enough info into how to us Connections/Edges now.

@rangermeier
Copy link

Since I struggled a lot to expose a newly created item as edge, here is a complete example using Graphene 2.0, building on the suggestions from @mickeyinfoshan and @varuna82. Hope this is of some help:

import graphene
from graphene import relay
from graphene_django import DjangoObjectType
from graphql_relay.connection.arrayconnection import offset_to_cursor
from myapp import models

class Topic(DjangoObjectType):
    class Meta:
        model = models.Topic
        interfaces = (relay.Node, )

TopicEdge = Topic._meta.connection.Edge

class CreateTopic(relay.ClientIDMutation):
    class Input:
        title = graphene.String(required=True)

    topic = graphene.Field(Topic)
    topic_edge = graphene.Field(TopicEdge)

    @classmethod
    def mutate_and_get_payload(cls, root, info, **input):
        topic = models.Topic.objects.create(title=input['title'], created_by=user)
        edge = TopicEdge(cursor=offset_to_cursor(0), node=topic)
        return CreateTopic(topic=topic, topic_edge=edge)

ronachong pushed a commit to ronachong/graphene that referenced this issue Nov 3, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests