odoo_graphql icon indicating copy to clipboard operation
odoo_graphql copied to clipboard

Invoke model method by name

Open ooraini opened this issue 2 years ago • 3 comments

Currently it's only possible to call search and write. For mutation you may want to invoke a different method, for example action_post on account.move. Perhaps an additional argument named method is sufficient

ooraini avatar Oct 30 '23 11:10 ooraini

Hi and thank you for your interest in this module.

This module does not aim to substitute the rpc protocol but to provide a better read, and secondly create/deletet/write, support on the records. I am following the standard as much as it is possible, and the standard only contains read/write/create/delete.

I understand that it could be convenient to do an action and a fetch all at once but:

  • This will generate more computation than required most of the time
  • We don't actually know the nature of the request itself (query? mutation? Subscription? ) and this kind of implementation might become a technical debt.
  • This would encourage people to use only this module which is not the goal.

I am therefore not planning on implementing that.

If you need something like that, you can wrap the rpc call on the client side so that, given the result of the rpc call, you would automaticly follow the rpc call with a graphql query.

The result is visually the same at the cost of 1 more underlying request, and the query will certainly need to include an ids parameter.

Hope it answers your demand. Have a nice day.

divad1196 avatar Oct 30 '23 17:10 divad1196

Hi @divad1196 thanks a lot for the module, it has been amazing so far! Although having a more complete GraphQl experience is tempting, It's understandable that you want to keep the scope small and focused.

I saw this comment in the code base which prompt me to open the issue. And to clarify what I meant, the GraphQl query would look something like this:

mutation {
  AccountMove(domain: [[...]], method: "action_post", vals: {...}) {
    id
  }
}

There is the issue of passing arguments, the RPC API uses a separate positional argument list and keyword argument dict. Another approach is to generate a separate mutation AccountMove_actionPost.

I'll experiment with it myself and see I f I can get it work. Feel free to close this if you consider it outside the scope. Thanks a lot, I can't emphasis how amazing this is, and how much better it's than the jsonrpc API.

ooraini avatar Oct 30 '23 18:10 ooraini

@ooraini Regarding the comment, I apologize for the confusion. Currently, the method to create a record is create and the one to update it is write, and the idea would have been, for example, to replace create by another function, meaning you would not be able to use create anymore. Another idea was to make it possible to explicitly define the kind of mutation (create, read or write). Honestly, I think these two would be bad ideas.

Getting back on your proposal, I think that, on top of being a lot more complex, would be the worst idea here. I know that, for example using Graphene, for each mutation you define a new model and you would have this kind of thing. The first issue would be the exploding number of models available, the second issue would be the higher risk of name colision.

Therefore, among all propositions, adding a parameter would be the best and this is the one I thought about when writing this module. The change would only take 1-2hours at most. The issue is that there are a lot of other thing to consider:

  • Is that a query or a mutation ? Do we use mutation regardless of the function?
  • Function privacy: should the user be able to call it (follow Odoo convention manually ?)
  • What constraint does it add to the code
  • What other feature may be asked on top of this one. I can easily see people then asking to be able to be able to either call the method on all records at once (sale.order(1,2,3).mymethod(...)) or be able to call the method one by one (for s in sale.order(1, 2, 3): s.mymethod()). or even do traversal (sales.mapped('order_line').mymethod()).
  • What could be broken (retro-compatiblity, or graphql ecosystem)
  • ...

and the other reasons already mentionned, like encouraging everybody to use graphql instead of rpc. I am not against it, I am just not sure it is the best for this module and for its users. I won't close the issue for now and see if I can make up my mind in the next days.

Have a nice day.

divad1196 avatar Oct 31 '23 21:10 divad1196

Hi,

I opening a discussion instead and closing this issue. The reason are:

  1. I feel like issues are not encouraging the discussion
  2. Issues sounds like "There is a problem" to many people. Here we are speaking about a feature proposal, not a bug.

https://github.com/divad1196/odoo_graphql/discussions/41

divad1196 avatar Aug 31 '24 09:08 divad1196