-
Notifications
You must be signed in to change notification settings - Fork 33
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
Implemented: Feature to run scheduled job right away(#364ttgf) #211
Conversation
src/components/JobActionsPopover.vue
Outdated
}, | ||
async runJobNow(job: any) { | ||
if(job) { | ||
await this.store.dispatch('job/runServiceNow', job) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think we should handle setting productStoreId from job instead of state value
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes sir. Understood as we discussed. Fixing it
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Check if we need to handle for shopifyConfigId as well
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sir shopifyConfigId never comes from job, thus I guess it is okay to always set it from userState
src/components/JobActionsPopover.vue
Outdated
}, | ||
async runJobNow(job: any) { | ||
if(job) { | ||
await this.store.dispatch('job/runServiceNow', job) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Check if we need to handle for shopifyConfigId as well
We should also add a confirmation alert that asks the user if they are sure they want run now. |
Done sir. Added an alert to confirm to run job immediately. |
src/store/modules/job/actions.ts
Outdated
'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 | ||
}, | ||
'shopifyConfigId': this.state.user.shopifyConfigId, | ||
'shopifyConfigId': job.shopifyConfigId ? job.shopifyConfigId : this.state.user.shopifyConfigId, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
'shopifyConfigId': job.shopifyConfigId ? job.shopifyConfigId : this.state.user.shopifyConfigId, | |
'shopifyConfigId': job.status === "SERVICE_PENDING" ? job.shopifyConfigId : this.state.user.shopifyConfigId, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ok sir. Fixed it.
Related Issues
Closes #
Short Description and Why It's Useful
Screenshots of Visual Changes before/after (If There Are Any)
Screen.Recording.2022-08-08.at.12.48.28.PM.mov
IMPORTANT NOTICE - Remember to add changelog entry
Contribution and Currently Important Rules Acceptance