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

Custom order filtering for ShipStation import XML #50

Open
wants to merge 6 commits into
base: master
Choose a base branch
from

Conversation

BillBushee
Copy link

I integrated this pull request #48 into my fork and then modified the order controller getOrders method to look for another plugin with a method named oneShipStationGetOrderCriteria. If the method exists, the returned ElementCriteriaModel will be used to find orders for the XML feed. If the user defined method is not found, then oneShipStation's default behavior won't change. I also updated the readme file to document the change.

Increased custom fields code to truncate content at 1000 characters (limit of internal notes field length)
Further revised code to truncate certain fields at 100 characters and the rest at 1000 characters
Added option to override criteria for getOrders
Added paragraph and code example for overriding default getOrder search criteria
@brianjhanson
Copy link
Contributor

Hey @BillBushee,

This looks like a great addition to the plugin! It looks good to me at a glance, but we'll need to carve out some time to test it out a bit before pressing the big green button. I apologize in advance if this PR hangs out for awhile.

Thanks again for the contribution!

Fix bug in discount() method - was using number_format() resulting in a comma being added to float field.
@BillBushee
Copy link
Author

Hi @brianjhanson,

I also discovered a little bug in the XML Service discount() method. It's using number_format() on the discount total. By default, that returns a comma as a number separator, so a discount of $1,000 or more breaks the import. I switched it to round() in my fork, since that's what the rest of the code appears to use. Would you like me to submit a separate pull request for that?

@brianjhanson
Copy link
Contributor

Hey @BillBushee,

No need to open a new PR, especially since the change looks ver minor. Again, apologies this PR is kind of just hanging out. Hopefully over the long weekend I can carve out some time to get this merged in 🤞

Thanks for your patience!

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.

2 participants