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

Fixed action scheduler time issue with old method #1050

Merged
merged 5 commits into from
Jan 10, 2025

Conversation

girishpanchal30
Copy link
Contributor

Summary

In this PR, I've fixed the Action Scheduler issue by addressing the older method. I've updated the condition to prioritize checking for the older method before evaluating other conditions.

Will affect visual aspect of the product

Yes

Check before Pull Request is ready:

Closes https://github.com/Codeinwp/feedzy-rss-feeds-pro/issues/815

@girishpanchal30 girishpanchal30 added the pr-checklist-skip Allow this Pull Request to skip checklist. label Jan 9, 2025
@pirate-bot pirate-bot added the pr-checklist-complete The Pull Request checklist is complete. (automatic label) label Jan 9, 2025
@vytisbulkevicius
Copy link
Contributor

Hey @girishpanchal30,

Seems like some checks failed and build was not created here

@pirate-bot
Copy link
Contributor

pirate-bot commented Jan 9, 2025

Plugin build for 770b335 is ready 🛎️!

@vytisbulkevicius
Copy link
Contributor

Looks like the build is here now after re-triggering the job but some checks still failed

@selul
Copy link
Contributor

selul commented Jan 9, 2025

@girishpanchal30 can you explain a bit how this fixes the issue? If it was a function missing issue it would have thrown a fatal error not a wrong timestamp?

@girishpanchal30
Copy link
Contributor Author

@selul Sure!

as_has_scheduled_action function returns true or false if a scheduled action is found, while the as_next_scheduled_action function returns a timestamp or false.
So, I first check the next scheduled action using as_next_scheduled_action function and then check if the action is pending or in progress using as_has_scheduled_action.

@selul
Copy link
Contributor

selul commented Jan 9, 2025

@girishpanchal30 thanks for the explanation, makes sense now. When the as_has_scheduled_action returns true what's the timestamp that the user will use? Maybe in this case we can show in progress or something instead of the timestamp.

Great findings btw.

@girishpanchal30
Copy link
Contributor Author

@selul

Maybe in this case we can show in progress or something instead of the timestamp.

I’ve added the in progress status text.

@vytisbulkevicius Now all checklist are passed.

@selul
Copy link
Contributor

selul commented Jan 10, 2025

lgtm

Copy link
Member

@HardeepAsrani HardeepAsrani left a comment

Choose a reason for hiding this comment

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

LGTM, thanks for the explanation in the thread as I was also confused why having the older method is better than the newer one.

@vytisbulkevicius vytisbulkevicius merged commit 698bede into development Jan 10, 2025
8 checks passed
@vytisbulkevicius vytisbulkevicius deleted the bugfix/pro/815 branch January 10, 2025 09:07
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
pr-checklist-complete The Pull Request checklist is complete. (automatic label) pr-checklist-skip Allow this Pull Request to skip checklist.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants