Fix #2382: Only return latest version when 24 hours has lapsed

[ADTC]

WHY are these changes introduced?

Fixes #2382

Version check happens on every execution of any command, instead of happening once every 24 hours.

WHAT is this pull request doing?

Make sure that the version check happens only once every 24 hours.

How to test your changes?

1. Install older version and integrate this change.
2. Run a command; observe version check.
3. Run a command again within 24 hours; observe no version check.
4. Run a command again after 24 hours; observe version check.

Note: You may test using a shorter interval by changing the value of VERSION_CHECK_INTERVAL.

Post-release steps

None.

Update checklist

• I've added a CHANGELOG entry for this PR (if the change is public-facing)
• I've considered possible cross-platform impacts (Mac, Linux, Windows).
• I've left the version number as is (we'll handle incrementing this when releasing).
• I've included any post-release steps in the section above (if needed).

[amcaplan]

Thanks, this is a great idea! Agreed it's not a good experience to have pushes to upgrade all the time. Once a day is enough.

I left one suggestion to quiet Rubocop so CI can run; please fix!

[ADTC]

Thanks, this is a great idea! Agreed it's not a good experience to have pushes to upgrade all the time. Once a day is enough.

To be perfectly clear, there's nothing new here. The "once a day" check was already part of Shopify CLI several versions prior. But a recent change broke the logic, spitting the message out on every command (even if the actual version check happens only once a day!).

This PR just fixes the broken logic 🙂

Update: Yehay! All tests passing! 🎉

[ADTC]

Hi @gonzaloriestra & @karreiro it will be nice if I can request your review as well. Thanks 🙂

[karreiro]

Thank you, @ADTC! Great stuff :)

Also, I'm approving as it fixes a regression.

However, ideally we should add a test case on test/shopify-cli/context_test.rb to validate this change. Do you plan to include a unit test @ADTC?

[ADTC]

@karreiro sorry I had not thought of it, and Ruby isn't a language I'm intimately familiar with. I can only make these simple changes as of now, though I understand the code's logic due to my CS background 😄

Any help with the unit test is appreciated! (Feel free to push to this branch.) I will try to do one myself in my free time if someone didn't already do it. (Not right now though.)

Thank you so much for the approval! Yes, this is a fix for regression. Good catch there!

[ADTC]

@karreiro when I think about this further, the change is so simple I feel it shouldn't be necessary to wait for a unit test. It's also urgent to merge this before the release of next version, as the recent older versions are annoying the users with the update prompt on every command.

Hope this can be merged before next release.

[ADTC]

Thank you @karreiro :)