Skip to content

Conversation

mllemango
Copy link
Contributor

@mllemango mllemango commented Jan 29, 2021

WHY are these changes introduced?

Fixes #361

WHAT is this pull request doing?

Introducing new resource FulfillmentEvent that maps to the fulfillment_event api https://shopify.dev/docs/admin-api/rest/reference/shipping-and-fulfillment/fulfillmentevent

Checklist

  • I have updated the CHANGELOG (if applicable)
  • I have followed the Shopify Python guide


def save(self):
status = self.attributes['status']
if status not in ['label_printed', 'label_purchased', 'attempted_delivery', 'ready_for_pickup', 'picked_up', 'confirmed', 'in_transit', 'out_for_delivery', 'delivered', 'failure']:
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Unsure if hardcoding a list of accepted status' is the best way to go about this

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As for whether to hard code them: Are these likely to change in the future at all/frequently? And is there another way to reference them?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No other way to reference status as far as I know

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah I didn't think so, but wanted to ask. I would maybe just assign a variable for the array but otherwise if that's the only option, that's the only option 🤷

Copy link

@tanema tanema left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good to me

@Paulinakhew Paulinakhew merged commit e2fdcab into master Feb 1, 2021
@Paulinakhew Paulinakhew deleted the melaniewpaulinak/add-fulfillment-event branch February 1, 2021 18:42
@NabeelAhsen NabeelAhsen mentioned this pull request Mar 17, 2021
2 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

How to create a FulfillmentEvent
5 participants