nozzlegear / ShopifySharp

ShopifySharp is a .NET library that helps developers easily authenticate with and manage Shopify stores.
https://nozzlegear.com/shopify-development-handbook
MIT License
742 stars 308 forks source link

Add ShopifySharp.Services.ShopPlanService #991

Closed nozzlegear closed 8 months ago

nozzlegear commented 8 months ago

This PR adds a new ShopPlanService class, which is just a convenience wrapper around the GraphQL API for getting a shop's current Shopify subscription. I mainly use it to check if a shop is on a development/test partner plan.

Note: I've added the new service to the ShopifySharp.Services namespace and plan to require all new services be in that namespace. In 7.0, we'll probably migrate all current services from ShopifySharp to ShopifySharp.Services as well.

clement911 commented 7 months ago

Hi @nozzlegear

I think it's a mistake to create wrapper services for specific GraphQL operations. This might lead to a huge number of tiny services that are hard to maintain.

It's easier to simply rely on the generated GraphQL Model.

You can simply do as follows to get the shop object, strongly typed.

    var svc = new GraphService(Utils.MyShopifyUrl, Utils.AccessToken);
    var shop = await svc.SendAsync<GraphQL.Shop>("""
    {
      shop {
        plan {
          displayName
          partnerDevelopment
          shopifyPlus
        }
      }
    }
""");
    var plan = shop.plan;

What do you think? Did you realize that was already possible?

The benefit of the strongly typed generated models is that it exposes all fields of all objects, as well as all queries and mutations. It will also make using further versions easy, since all is needed is the file \Entities\GraphQL\GraphQLSchema.generated.cs by running GenerateGraphQLSchema_Test.

Less importantly, I personally like having all objects, including entities and services under the single ShopifySharp namespace, as I find it results in less namespaces when consuming the library. I don't feel very strongly about that though.

nozzlegear commented 7 months ago

Hey @clement911! I've been thinking about what you said and I agree in spirit. My reasoning for adding this little helper service is mainly that I want people to see or be aware of all the little things that you can do with ShopifySharp and the GraphQL API that aren't necessarily obvious just by looking at what's available in the intellisense.

I had some ideas for other helper services that I wanted to add as well, mostly around metafields, but I'm going to rethink that. It might be better to just write about how to do those things with ShopifySharp, rather than adding code that needs to be maintained. Or I could create a ShopifySharp.GraphQL.Extras (name not final) package that uses the graph service and the generated schema's from ShopifySharp proper.

I'm not sure which way I want to proceed at the moment. It's certainly easier to write about the extra methods, and better from an SEO perspective as well to drive more eyes to this package. But they're also extra helper methods that I use across many apps, so it'd be kinda nice to have them all packaged up somewhere.

Also agreed I should have used the generated schema! I'll change it to use that in my next PR, and then deprecated this service once I decide what to do.

As for the namespaces, I'm not deadset on changing them either to be honest. If I were starting again from scratch I'd probably have services in ShopifySharp.Services and entities in ShopifySharp.Entities, but moving them might just be creating busywork for everyone with little benefit.

clement911 commented 7 months ago

If you want people to be aware of the capabilities, I would say that extra documentation is indeed the solution.

As you can see from my code sample above, the code is very terse and the generated object model makes it very easy to use already. I agree that the GraphQL service is less discoverable, but if we add wrappers for one query, we open the door for people to create wrappers for every queries and mutations, which will add very little value but decrease maintainability substantially. Once someone understands that one can access the full power of GraphQL with the GraphService, one can issue any queries/mutations very easily, without needing yet an extra wrapper.

I wouldn't worry too much about SEO.