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.

Sign up for GitHub

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

build: update shopify SDK #274

Merged
merged 3 commits into from
Oct 24, 2023
Merged

build: update shopify SDK #274

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
38f83c1
build: update shopify sdk
ankush Oct 24, 2023
3acebaa
fix: product sync
ankush Oct 24, 2023
b1dc35c
fix: Correctly wrap function
ankush Oct 24, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions ecommerce_integrations/shopify/connection.py
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
import base64
import functools
import hashlib
import hmac
import json
Expand All @@ -21,6 +22,7 @@
def temp_shopify_session(func):
"""Any function that needs to access shopify api needs this decorator. The decorator starts a temp session that's destroyed when function returns."""

@functools.wraps(func)
def wrapper(*args, **kwargs):

# no auth in testing
Expand Down
2 changes: 1 addition & 1 deletion ecommerce_integrations/shopify/constants.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@
SETTING_DOCTYPE = "Shopify Setting"
OLD_SETTINGS_DOCTYPE = "Shopify Settings"

API_VERSION = "2023-07"
API_VERSION = "2023-04"

WEBHOOK_EVENTS = [
"orders/create",
Expand Down
4 changes: 1 addition & 3 deletions ecommerce_integrations/shopify/page/shopify_import_products/shopify_import_products.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -33,9 +33,7 @@ shopify.ProductImporter = class {
}

async checkSyncStatus() {

const { message: jobs } = await frappe.db.get_list("RQ Job", {filters: {"status": ("in", ("queued", "started"))}});

const jobs = await frappe.db.get_list("RQ Job", {filters: {"status": ("in", ("queued", "started"))}});
Copy link
Contributor

Choose a reason for hiding this comment

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

Is there any way we can run version-specific code from js? Because, this doesn't work in version-13 as it doesn't have RQ Jobs.

Copy link
Member Author

Choose a reason for hiding this comment

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

We can check version from frappe.boot.versions (I don't remember actual key, but it should be there)

Copy link
Contributor

Choose a reason for hiding this comment

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

Yes. Got it.

parseInt(frappe.boot.versions.frappe); // returns 13, 14, or 15

Which is better? Calling separate functions based on the frappe version or having separate branches of this repo like version-13 and version-15?

Or is version-13 outdated enough to warrant an upgrade?

Copy link
Member Author

Choose a reason for hiding this comment

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

V13 will be EOL in 2 months. Maintaining multiple branches is kind of a hassle.

Copy link
Member Author

Choose a reason for hiding this comment

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

Diff code based on condition is fine.

Copy link
Contributor

Choose a reason for hiding this comment

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

Understood. But again, we only have one customer on v13 using this. I'm not sure if there are any others still on v13 with this app.

I could just fork this and keep the old code on it instead of changing the upstream for one single customer.

It will also prevent any future updates messing with v13.

this.syncRunning = jobs.find(job => job.job_name == 'shopify.job.sync.all.products') !== undefined;

if (this.syncRunning) {
Expand Down
2 changes: 1 addition & 1 deletion requirements.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,2 +1,2 @@
ShopifyAPI==8.2.0 # update after resolving pyjwt conflict in frappe
ShopifyAPI==12.3.0 # update after resolving pyjwt conflict in frappe
boto3~=1.18.65