Use nested modules to reflect nested resources?

[Ninigi]

When I was adding some more resources I became kind of blind to the details, pretty much lost in copy pasting and going through the same changes over and over and whenever I did a nested resource I thought "how exactly would anyone use this and know where the first id is coming from?"

For example

session |> Fulfillment.all(123123)
session |> UsageCharge.find(987987123, 123)
session |> Transaction.count(879879798)

If you know that a fulfillment means "order fulfillment", usage charge is a kind of "recurring application charge" and transaction again refers to an order, then of course you know what those IDs are, but there is no hint whatsoever in the code. My proposal would be to use nested module names like this:

session |> Order.Fulfillment.all(123123)
session |> RecurringApplicationCharge.UsageCharge.find(987987123, 123)
session |> Order.Transaction.count(879879798)

alternatively the function names could be changed to reflect the "base resource"

session |> Fulfillment.all_for_order(123123)
session |> UsageCharge.find_for_recurring_application_charge(987987123, 123)
session |> Transaction.count_for_order(879879798)

I like the nested module better, because you can just alias them and use the shorter version if you like, whereas the function names would always be long.

We could do this without breaking older versions and simply add the alternative to the existing solutions, and add a deprecation warning to the old version.

Let me know what you think @nsweeting do you think we should do this at all, and if so which of the solutions would you like better?

[Ninigi]

Just as if shopify was trying to make a point here, I was just working on Fulfillment Events, which are double nested... And of course there are also Order Events.

Right now the order events are organized in nested modules

Shopify.Order.Event.all(1)

# for the sake of consistency we should decide if this makes sense
Shopify.Order.Fulfillment.Event.find(78123, 123, 1)

Shopify's API reference is very inconsistent in how they list the events, for example there is an entry for FulfillmentEvent but none for OrderEvent (in fact I didn't find an entry for that at all, but there is an endpoint for it and it works...) and you can also get an event without nesting...

The shopify API doc is pretty messy in parts, so that's how we ended up in this place in the first place, but would be nice to settle on a way to deal with it

[nsweeting]

At the moment - the the nested module for sure seems like the cleaner approach.

Personally, I haven't yet had a need to fetch those nested resources yet. I would be curious as to how useful being able to pass the owner struct OR the owner id as an argument would be. It would provide a much more "elixir'ish" feel to things - permitting the use of pattern matching on the owner to determine what to do.

It would be cool to be able to pipeline calls and build up the nested relations within the owner as well. For example...

session
|> Shopify.Order.find(1)
|> Shopify.Transactions.all()

Which would provide a %Shopify.Order{transactions: [....]} struct. With the above, there are obvious complications with the session being the first arg (which could perhaps be passed back with the result).

Anyways - these are all larger and more ambitious tasks. Perhaps a discussion for a new version. I will look at perhaps creating a new issue where we can start compiling such features and discussions.

[nsweeting]

To add on to the this further.

I'm basically trying to envision a few different ways we could handle fetching nested relations through pipelines. Although it creates "extra" structs in the process, something akin to a new function may be useful and much more explicit than a bunch of integer ids. We could have this accept a few different argument forms: new(attributes), new({session, owner}, attributes), as well as new(session, attributes)

With the above, something like the triple nested order fulfillment event could be handled like:

session
|> Shopify.Order.new(id: 1)  # if passed a session as the first arg, returns {session, order}
|> Shopify.Fullfillment.new(id: 1)
|> Shopify.Event.find(1) # not sure about this... would basically require accepting {session, owner} as its arg as well...

Food for thought...

[Ninigi]

Shopify restructured their API reference, I think precisely because especially the nested resources were all over the place. Might help with how we think about the structure as well.