fix: Delay updating schedule until after fields have been initialized

[cincodenada]

Description

This is a fix for #2316. It changes the schedule field handling so that updates to the schedule fields don't propagate to the profile until after the fields are initialized (that is, loaded from the profile).

This resolves a bug where we briefly reset schedule fields to 0 because we copied all field values to the profile after only updating the first (or second) field from the profile. This in turn got in a race with the initial schedule check, and caused spurious "catchup" runs where Vorta calculated the next run to be at midnight (00:00) regardless of the user's settings, and thus always generating a "catchup" run, since midnight is the very beginning of the day.

I also modified the init_db fixture in tests slightly to support deferring the window load to the test itself. Unless a test uses the newly-introduced window_load fixture, init_db functions as it did before.

Related Issue

Issue #2316

Motivation and Context

Solves a bug where fixed-time schedules would always generate a catch-up run if the application was started before the scheduled time, which also caused the scheduled runs to not take place.

How Has This Been Tested?

I added a unit test specifically to test this bug, initially failing and then passing after the changes. I also manually tested as follows:

• Set a fixed-time schedule to run at 6pm
• Ran a backup to start the schedule
• Used the Linux utility faketime to simulate re-opening the app the next morning
• Used faketime to simulate re-opening the app again at 10pm that same day

As expected, there was no catch-up run when opening the app in the morning, and it scheduled the 6pm run as anticipated. Opening the app at (simulated) 10pm did do a catch-up run, as it was past the scheduled run time.

I also ran all other tests as well, which remain passing, other than the test_scheduler_create_backup test that fails in main on my machine too.

Types of changes

• Bug fix (non-breaking change which fixes an issue)
• New feature (non-breaking change which adds functionality)
• Breaking change (fix or feature that would cause existing functionality to change)

Checklist:

• I have read the CONTRIBUTING guide.
• My code follows the code style of this project.
• My change requires a change to the documentation.
• I have updated the documentation accordingly.
• I have added tests to cover my changes.
• All new and existing tests passed.

I provide my contribution under the terms of the license of this repository and I affirm the Developer Certificate of Origin.

[m3nu]

🔍 Code Review

PR: #2317 - fix: Delay updating schedule until after fields have been initialized
Branch: debug-scheduler-race → master
Changes: +106 / -12 lines across 6 files

📋 Summary

This PR correctly fixes a race condition where schedule fields were being reset to 0 during UI initialization, causing spurious catchup runs. The fix uses a flag to defer writing to the profile until after all fields have been populated from the profile.

✅ What's Good

• Root cause correctly identified and addressed
• Clean implementation of loaded signal in MainWindow for coordinating initialization
• Good test coverage with parameterized test cases covering multiple scenarios
• Test fixture design (window_load) allows fine-grained control over initialization order

🔍 Review Details

7 inline comment(s) added.

Severity	Count
🔴 Error	0
🟡 Warning	2
🔵 Info	2
💡 Suggestion	2
✅ Good	1

📊 Verdict: COMMENT 💬

Overall this is a solid fix for a tricky race condition. The suggestions below are minor and mostly about code style and cleanup.

---

🤖 Reviewed by Claude Code

[m3nu]

🔵 [debug-logging] Debug statement for review

This debug logging statement appears to be added for debugging purposes. Consider removing it before merging or ensure it's at the appropriate log level for production.

Note: If you want to keep debug logging, this is fine - just wanted to confirm it's intentional for ongoing diagnostics rather than leftover debug code.

[m3nu]

🔵 [debug-logging] Debug statement for review

Same as above - another debug logging statement. If these are intended to help with future schedule debugging, that's perfectly fine. Just flagging in case they were temporary.

[m3nu]

🟡 [naming-convention] Non-PEP8 variable name

The codebase uses snake_case for instance variables. Consider renaming to _has_populated_schedule_fields for consistency with Python conventions and to indicate it's a private implementation detail.

Suggested fix:

self._has_populated_schedule_fields = False

[m3nu]

💡 [code-clarity] Consider adding a brief comment

Good fix! The logic correctly moves the profile updates outside the for-loop so all fields are saved regardless of which radio button is checked. A brief comment explaining why the flag check is needed could help future maintainers understand the race condition being prevented.

Example:

def on_scheduler_change(self, _):
    # Prevent writing default values back to profile during initial field population
    if not self._has_populated_schedule_fields:
        return

[m3nu]

🟡 [documentation] Missing docstring

The load_window function should have a docstring explaining its purpose since it's used as a fixture helper. The existing comment above is good but could be converted to a proper docstring.

Suggested fix:

def load_window(qapp: vorta.application.VortaApp):
    """
    Reload the main window of the given application.
    
    Used to repopulate fields after loading mock data in tests.
    """
    qapp.main_window.deleteLater()
    del qapp.main_window
    qapp.main_window = MainWindow(qapp)

[m3nu]

💡 [resource-management] Consider cleaning up original scheduler

Replacing qapp.scheduler creates a new VortaScheduler instance, but the original scheduler may still have active timers or connections. Consider explicitly cleaning up the original scheduler first, or verify that the garbage collector handles this correctly.

Potential improvement:

# Clean up existing scheduler resources
old_scheduler = qapp.scheduler
# ... any cleanup if needed ...
qapp.scheduler = VortaScheduler()

This might already be handled by the test teardown, just flagging for consideration.

[m3nu]

✅ [good-practice] Clean signal implementation

Nice addition of the loaded signal! This provides a clean way to coordinate actions that need to happen after the window is fully initialized. The placement at the end of __init__ after set_icons() ensures all setup is complete before the signal fires.