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

ShopifyOrderStatus Enums need updating #64

Closed
ZubyH opened this issue Sep 3, 2016 · 7 comments · Fixed by #65
Closed

ShopifyOrderStatus Enums need updating #64

ZubyH opened this issue Sep 3, 2016 · 7 comments · Fixed by #65

Comments

@ZubyH
Copy link

ZubyH commented Sep 3, 2016

ShopifyOrderStatus is missing some new enums, so I am unable to download unfulfilled orders. I can currently only receive all open orders for a given store

@nozzlegear
Copy link
Owner

Thanks for filing this issue! As we discussed, Shopify has changed the enums it uses for fulfillment statuses. We'll need to add the four new values and mark the old ones as obsolete to be removed in ShopifySharp 3.0.

We just moved into our new house this week, and I still don't have my dev machine set up yet. I'll get this fixed as soon as I do -- hopefully later today, but possibly not until tomorrow or Tuesday.

@nozzlegear nozzlegear added the bug label Sep 4, 2016
@ZubyH
Copy link
Author

ZubyH commented Sep 4, 2016

Not a problem, I'm working around this at the moment while I'm running through testing/finalization of my app. Thought I'd raise the issue here in case other users are seeing the same thing.

Thanks!

@nozzlegear
Copy link
Owner

nozzlegear commented Sep 6, 2016

Just leaving some notes while I work through these changes.

  • There are now three versions of a FulfillmentStatus:
    1. On ShopifyOrder.FulfillmentStatus and ShopifyLineItem.FulfillmentStatus; values are still fulfilled, none and partial.
    2. When filtering the .ListAsync result; values are now shipped, partial, unshipped and any.
    3. On ShopifyFulfillment.Status; values are now pending, open, success, cancelled, error and failure.
  • ShopifyOrderStatus enum itself hasn't changed and is still used.

I'm guessing ShopifyOrder.FulfillmentStatus and ShopifyLineItem.FulfillmentStatus values have actually changed to the new shipped, partial, unshipped and any, but they just didn't update the docs.

@nozzlegear
Copy link
Owner

nozzlegear commented Sep 6, 2016

Currently considering a quick 3.0 release (and bump the other 3.0 issues to 4.0) to switch these to strings rather than hardcoded enums, and would then carefully consider removing the other enums in 4.0. Enums are more much convenient than researching the docs to determine which string needs to be used, but they quickly become a hassle and break apps when Shopify makes an unannounced change like this.

If switched to strings, the developer could at least fix their app on their own without waiting for a new ShopifySharp release. Enums would be much better suited to this if Shopify had API versioning a la Stripe, but sadly they don't.

@ZubyH
Copy link
Author

ZubyH commented Sep 6, 2016

Definitely like the idea, especially if my app hit production and this occurred - would save me being dependant on another person making a fix.

Stupid question, given that I'm not sure how you'd implement this fix, If I'm using the Nuget library instead of the actual source code as a project in my app, would I have the same freedom to amend the strings?

@nozzlegear
Copy link
Owner

Yep, the strings would be coming from your own code rather than living inside this library. Right now the ShopifyOrderFilter looks something like this:

public class ShopifyOrderFilter : BaseFilter
{
  ...
  public ShopifyFulfillmentStatus FulfillmentStatus { get; set; }
  ...
}

And it gets used like this:

var filter = new ShopifyOrderFilter()
{
  FulfillmentStatus = ShopifyFulfillmentStatus.Fulfilled
}

As it is right now, you don't have any way to fix that FulfillmentStatus when the expected values are changed (unless you're using the source code), because they're hardcoded in the lib. It wants that enum rather than the raw string that gets sent to the Shopify API behind the scenes.

If I switched it to strings, the class would look like this instead:

public class ShopifyOrderFilter : BaseFilter
{
  ...
  public string FulfillmentStatus { get; set; }
  ...
}

And it would be used like this in your app:

...

var filter = new ShopifyOrderFilter()
{
  FulfillmentStatus = "Whatever string you want"
}

Since that string is coming from your own app, you'd be able to quickly update it and change the string rather than wait for a new ShopifySharp release.

@ZubyH
Copy link
Author

ZubyH commented Sep 6, 2016

Ah! Yes that would work well. I may just extend the ShopifyOrderFilter and add my own enums, at least they're in my control that way and I can amend them faster than a ShopifySharp release.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants