Defining mutations in GraphQL via fields: Is this bad practice?

user_4681998 0
Viewed 330

Suppose you have a user type, and a user has many posts. Then imagine you want to find a user, and delete all of their posts. One way to do this is to implement the following mutation field: 

field deleteAllPosts, types[Types::PostType] do
  argument :user_id, types.String

  resolve -> (obj,args,ctx){ 
    posts = Posts.where(user_id:args[:user_id])
    posts.each{|post| post.destroy}
  }
end

Then the query 

mutation {
  deleteAllPosts(user_id:1)
}

will delete all the posts of the user with id 1.

Before I did this, I thought about doing it a different way, which I've not seen anyone else do. I wanted to check that this different way doesn't have any pitfalls, or reasons I shouldn't use it.

The idea is to instead put a deletePost field for PostType, and a findUser field on mutation (which would typically be a query field). Assuming it's obvious how those fields would be defined, I would then make the query

mutation{
  findUser(id:1){
    posts{
      deletePost{
      id
      }
    }
  }
}

Is this a bad idea?

Edit in response to feedback: One thing I'm concerned about is the possibility that a user could, in principle, make the deletePost selection inside of a query. But I'm tempted to say that that's "their fault". I'd like to say "this selection can only be made if it is inside of a mutation query", but I don't think that's possible in GraphQL.

In order to avoid the XY problem, here is why I am keen to use this idea rather than the initial one. It feels more expressive (said differently, it feels less redundant). Suppose that, after a while, you decide that you want to delete all the posts for those users belonging to a particular group. Then in what I regard as the 'convention', you should create a whole new mutation field: 

field deleteAllPostsInGroup, types[Types::PostType] do
  argument :group_id, types.String

  resolve -> (obj,args,ctx){ 
    posts = Group.find_by(args[:group_id]).users.map{|u| u.posts}.flatten
    posts.each{|post| post.destroy}
  }
end

whereas in my suggested convention you just define a trivial findGroup field (but you have to define it on mutation, where it doesn't belong), and make the query:

mutation{
  findGroup(id:1){
    users{
      posts{
        deletePost{
        id
        }
      }
    }
  }
}

I suppose that really what I'm trying to do is use a query to find some data, and then mutate the data I've found. I don't know how to do this in GraphQL.

Second Edit: It seems like there is a well-defined component of this question, which I have asked here. It may turn out that one of these questions answers the other, and can be closed, but I don't know which way round yet.

graphql
1 Answers

This is basically a code quality issue and is similar to asking about the point of the DRY principle or encapsulation.

A quote from https://graphql.org/learn/queries/ reads:

In REST, any request might end up causing some side-effects on the server, but by convention it's suggested that one doesn't use GET requests to modify data. GraphQL is similar - technically any query could be implemented to cause a data write. However, it's useful to establish a convention that any operations that cause writes should be sent explicitly via a mutation.

This is a good convention as it makes maintenance, testing and debugging easier. Side-effects, whether intentional or not, can be awfully difficult to track and understand. Particularly if you have them in GraphQL queries, which can be arbitrarily large and complex. There is nothing preventing you from querying and modifying the same object and it's siblings at the same time, and doing this multiple times in one query by simple nesting. It is very easy to get this wrong.

Even if you pull it off, code readability and maintainability suffer. E.g. if you knew that only your mutations ever modified the data, and queries had no effect on it, you would immediately know where to start looking for the implementation of a particular behaviour. It is also a lot easier to reason about how your program works in general.

If you only write small, properly named, granular mutations, you can reason about what they do more easily than you could if you had a complex query which updated different data at different points.

Last but not necessarily least, sticking to conventions is useful if you ever need to transfer your work to someone else.

In short - it is all about making the lives of yourself and others easier in the future.

EDIT

OK, so I see where you are going with this - you want to give the flexibility of a GraphQL query to the mutations. Sure, this particular example would work. Not going this way would only be about the future. There is no point in discussing this if deletePost is the only operation you will ever define.

If that's not the case, then what if you wanted to delete, let's say, 5 specific user posts? Would you give extra parameters to findGroup and then pass those down the tree? But then why does findGroup method have to know about what you will do with it's results? That kind of defies the idea of a flexible query itself. What if you also wanted to perform mutations on users? More parameters for findGroup? What if users and posts can be queried in a different way, like, users by domains, posts by categories, etc.? Define the same parameters there too? How would you ensure that with every operation (especially if you do a few of them at once) all the relational links are properly erased in your database? You would have to imagine every possible combination of queries and query-mutations and code appropriately for them. Since query size is unlimited it could end up being very hard to do. And even if the purpose of an individual query-mutation (deletePost) is clear and easy to grasp, the overall query would not be. Quickly your queries would become too complex to understand even for you and you'd probably begin breaking them down to smaller ones, which would only do specific mutations. This way you'd go back to the original convention but a more complex version of it. You would probably also end up defining some regular mutations too. How would you update or add posts, e.g.? That would spread your logic all over the place.

These questions would not occur if you were writing mutations. That's slightly more work in exchange for better maintainability.

These are all potential issues in the future (and there are probably more). If these don't concern you, then please go ahead with the implementation. I personally would run away from a project that did this, but if you are really clever, I don't see anything that would technically completely prevent you from achieving what you want :]

user_37445740
Related