Bug 410948

Summary: RSIBreak should not reset timers when on configuration OK with no changes
Product: [Applications] rsibreak Reporter: sanketh.ind
Component: generalAssignee: Albert Astals Cid <aacid>
Status: RESOLVED FIXED    
Severity: wishlist CC: sanketh.ind
Priority: NOR    
Version First Reported In: 0.12.8   
Target Milestone: ---   
Platform: Other   
OS: Linux   
Latest Commit: https://invent.kde.org/utilities/rsibreak/commit/44a7c608beffa95deccec48b83d90b8868b26ed5 Version Fixed/Implemented In:
Sentry Crash Report:

Description sanketh.ind 2019-08-15 17:38:44 UTC 
SUMMARY
RSI Break resets timers on hitting OK on settings dialog whether or not changes have been made.

STEPS TO REPRODUCE
1. RSI Break -> Configure RSI Break
2. Do not make any changes. Switch tabs if needed
3. Hit OK.

OBSERVED RESULT
Notice that both long and short break timers are now reset even though no config change happened.

EXPECTED RESULT
Timers should not be reset because no config changes happened.

SOFTWARE/OS VERSIONS
Windows: 
macOS: 
Linux/KDE Plasma: Ubuntu 18.04.3 LTS 
(available in About System)
KDE Plasma Version: 
KDE Frameworks Version: 5.44.0
Qt 5.9.5 (built against 5.9.5)

ADDITIONAL INFORMATION

It appears similar to https://bugs.kde.org/show_bug.cgi?id=200026 - I will look into why that fix does not work.
Comment 1 Bug Janitor Service 2022-01-06 21:42:18 UTC 
A possibly relevant merge request was started @ https://invent.kde.org/utilities/rsibreak/-/merge_requests/10
Comment 2 Albert Astals Cid 2022-01-10 21:55:53 UTC 
Git commit 44a7c608beffa95deccec48b83d90b8868b26ed5 by Albert Astals Cid, on behalf of Roman Azami.
Committed on 10/01/2022 at 21:55.
Pushed by aacid into branch 'master'.

Fix timers behaviour when config was[n't] updated

Original bug report detailed that when no changes to config were
made and OK was pressed the timers still reset everytime.

Method for resetting timers is RSITimer::updateConfig. Upon clicking
OK button event "configChanged" is emitted. This event has two
subscribers through which we arrive at RSITimer::updateConfig:
 - Three functions deep passageway
   RSIObject::readConfig ->
     RSIObject::configureTimer ->
       m_timer->updateConfig()
 - Direct connect to RSITimer::updateConfig

The second one is problematic, as when "configChanged" is being emitted
the argument passed to RSITimer::updateConfig is essentially always
true when app is not suspended. Passing true forces reset of timers.

So delete the second one, as it is redundant and will be called
through other subscriber without argument and wont force reset.

Upon removing second subscriber resets when config hasn't changed
went away. But another issue came up - legitimate timer config changes
weren't being applied when clicking OK the first time. Opening
config second time and again clicking OK seemed to set the new config
values.

After further debugging it seems that another subscriber of
"configChanged" - RSIGlobals::slotReadConfig sets
RSITimer::m_intervals's new values only after the call to
RSITimer::updateConfig was already made.

The fix for this seems to be changing sequence of connect()'s to the
event. Although I do not know enough about this observer mechanism
to be completely sure that sequence of subcribing is always being
respected when events are emitted :)

M  +1    -1    src/rsidock.cpp
M  +1    -4    src/rsidock.h
M  +1    -2    src/rsiwidget.cpp

https://invent.kde.org/utilities/rsibreak/commit/44a7c608beffa95deccec48b83d90b8868b26ed5