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

Jobs should be fetched and scheduled based upon the selected Shop(#2r65b1c) #228

Merged
merged 16 commits into from
Sep 1, 2022

Conversation

disha1202
Copy link
Contributor

@disha1202 disha1202 commented Aug 23, 2022

Related Issues

Closes #

Short Description and Why It's Useful

  1. Updated the logic to fetch pending jobs whose shopId is either the selected shop or empty
  2. Used ShopifyConfigAndShopLocation (View Entity) instead of ShopifyConfig for fetching shopifyConfig and shopId on login and productStore change
  3. Updated the state to store shopifyConfig instead of shopifyConfigId as we're storing shopId as well
  4. Passed shopId while scheduling service

Screenshots of Visual Changes before/after (If There Are Any)

IMPORTANT NOTICE - Remember to add changelog entry

Contribution and Currently Important Rules Acceptance

const shopifyConfigs = resp.data.docs;
commit(types.USER_SHOPIFY_CONFIG_LIST_UPDATED, shopifyConfigs);
commit(types.USER_CURRENT_SHOPIFY_CONFIG_UPDATED, shopifyConfigs.length > 0 ? shopifyConfigs[0].shopifyConfigId : "");
if (resp.status === 200 && !hasError(resp) && resp.data?.docs.length > 0) {
Copy link
Contributor

Choose a reason for hiding this comment

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

I think we should null for docs as well

@@ -11,8 +11,8 @@ const userModule: Module<UserState, RootState> = {
token: '',
current: null,
instanceUrl: '',
shopifyConfigs: [],
currentShopifyConfigId: "",
shopifyConfigs: {},
Copy link
Contributor

Choose a reason for hiding this comment

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

I think this should be array only

@@ -3,6 +3,6 @@ export const USER_TOKEN_CHANGED = SN_USER + '/TOKEN_CHANGED'
export const USER_END_SESSION = SN_USER + '/END_SESSION'
export const USER_INFO_UPDATED = SN_USER + '/INFO_UPDATED'
export const USER_INSTANCE_URL_UPDATED = SN_USER + '/INSTANCE_URL_UPDATED'
export const USER_SHOPIFY_CONFIG_LIST_UPDATED = SN_USER + '/SHOPIFY_CONFIG_LIST_UPDATED'
export const USER_SHOPIFY_CONFIGS_UPDATED = SN_USER + '/SHOPIFY_CONFIG_LIST_UPDATED'
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
export const USER_SHOPIFY_CONFIGS_UPDATED = SN_USER + '/SHOPIFY_CONFIG_LIST_UPDATED'
export const USER_SHOPIFY_CONFIGS_UPDATED = SN_USER + '/SHOPIFY_CONFIGS_UPDATED'

'statusId': "SERVICE_PENDING",
'systemJobEnumId': job.systemJobEnumId
} as any

if(job?.runtimeData?.shopifyConfigId) {
payload['shopifyConfigId'] = this.state.user.currentShopifyConfig?.shopifyConfigId
Copy link
Contributor

Choose a reason for hiding this comment

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

Store this.state.user.currentShopifyConfig in a variable and check for it's value in if condition itself and remove optional chaining.

@@ -529,13 +540,13 @@ const actions: ActionTree<JobState, RootState> = {
'systemJobEnumId': job.systemJobEnumId,
'tempExprId': job.jobStatus, // Need to remove this as we are passing frequency in SERVICE_TEMP_EXPR, currently kept it for backward compatibility
'parentJobId': job.parentJobId,
'recurrenceTimeZone': this.state.user.current.userTimeZone
'recurrenceTimeZone': this.state.user.current.userTimeZone,
'shopId': job.status === "SERVICE_PENDING" ? job.shopId : this.state.user.currentShopifyConfig.shopId,
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
'shopId': job.status === "SERVICE_PENDING" ? job.shopId : this.state.user.currentShopifyConfig.shopId,
'shopId': job.runtimeData?.shopifyConfigId && job.status === "SERVICE_PENDING" ? job.shopId : this.state.user.currentShopifyConfig.shopId,

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Updated the condition.

},
'shopifyConfigId': job.status === "SERVICE_PENDING" ? job.runtimeData?.shopifyConfigId : this.state.user.currentShopifyConfigId,
'shopifyConfigId': job.status === "SERVICE_PENDING" ? job.runtimeData?.shopifyConfigId : this.state.user.currentShopifyConfig.shopifyConfigId,
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
'shopifyConfigId': job.status === "SERVICE_PENDING" ? job.runtimeData?.shopifyConfigId : this.state.user.currentShopifyConfig.shopifyConfigId,
'shopifyConfigId': job.runtimeData?.shopifyConfigId && job.status === "SERVICE_PENDING" ? job.runtimeData?.shopifyConfigId : this.state.user.currentShopifyConfig.shopifyConfigId,

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Updated the condition.

@adityasharma7 adityasharma7 merged commit c48c397 into hotwax:main Sep 1, 2022
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.

3 participants