Fix next_job timer logic

[jayeclark]

Signed-off-by: Jay Clark jay@jayeclark.dev

New contributor here! I've located the error in the next_job logic and think I've implemented a fix, but I need to leave this PR in draft mode for the time being - I'm signing off for the day and have been having trouble getting the tests to run, will try again in the morning. I've verified manually that the fix works.

Fixes #1099

[m3nu]

Welcome to the project @jayeclark and thanks for your contribution!

There are a few related changes in #1096. They deal with the jobs manager and only remotely with the scheduler. So your change is probably still needed.

[jayeclark]

@m3nu Yes, I noticed that PR, but on review it didn't look like anything altered in it was the source of the error - there's actually just a basic error/oversight in the logic for selecting the right next_job from the timers. (The secondary comment in the issue, I'm pretty sure is addressed by the PR you referenced.)

However, I did notice the UI is a bit confusing even after this fix, because the next job timing in the tray menu is displayed only in terms of hours & minutes, no date. So, if my next backup is set to run at 11:00 three days from now, the tray menu will show me 11:00, and as a user I might think that means the backup will run at 11:00 today or, if it's past 11:00 today,
I might assume that it will run at 11:00 tomorrow - both of which would be incorrect assumptions.

Are most users likely to be running backups at least daily? (in which case the situation I described would be fairly rare?) Would it be useful to add a date to the next job display if the start time of the next job is more than 24 hours in the future?

Current PR is good to go and solves the main issue, however I could open an additional one for the date display if that's a good feature to add (or I could raise it as a separate issue and leave some time for discussion.)

[m3nu]

#1096 is now merged. Feel free to add your changes on top, if you still spot an issue, @jayeclark 👏

[m3nu]

(also rebased your branch to match the current master branch.)

[jayeclark]

@m3nu Yes, please take a look at my original commit. As far as I can tell, #1096 doesn't fix or affect the issue, but of course I'm very junior so I could be mistaken.

If I'm reading it correctly (and interpreting my user experience correctly), the issue is that the tray menu is displaying the wrong next job among all future scheduled jobs. That is, it is displaying a valid next job for a particular profile, but the job it is displaying isn't the actual next scheduled job among all the profiles - instead the tray menu is displaying the last next scheduled job.

Sure enough, that's what the code does. As currently written, given a list of jobs that are scheduled for the future (self.timers.items()), next_job (which is what determines what time shows in the tray menu) will always return the job whose start time is furthest from the present time, instead of the one whose start time is the closest to the present time. Whether or not the list of timers is in the correct chronological order doesn't affect the outcome.

[m3nu]

You are totally correct. When I made the scheduler changes, I just didn't properly add this intentionally to get a release out. I'm happy we can improve on this now.

I can also confirm that it now displays the correct closest job. So this resolves #1099. 👍

I did notice one other issue, where jobs aren't removed when disabling the scheduler. This just needs moving a block of code around. So I'll just add that next.

[jayeclark]

@m3nu Awesome, I'm so glad I could help!