Skip to content

Commit

Permalink
FactController: Fix start date picker modifying date twice (#674)
Browse files Browse the repository at this point in the history
Previously, setting the `date` property would (in addition to setting
the property itself and the `default_day` property of the dayline) also
update the fact's start and end, moving those by the same delta as the
date property was modified.

However, this caused a problem when changing the start date using the
date picker, since `on_start_date_changed()` would already update the
fact's start and end and then also update the `date` property, which
would then update the fact *again*, effectively applying the delta
twice.

This commit fixes this problem by changing the `date` setter to no
longer update the fact. This means that the `date` property now only
tracks the currently selected day of the dayline (and the UI as a
whole), making the setter and getter a bit better matched.

This also means that any changes to the fact's start and end must be
done separately from modifying the `date` property. There are two places
that currently update the `date` property:
 1. The `on_start_date_changed()` method, which already updates the fact.
 2. The `increment_date()`, which is changed by this commit to update
    the fact.

Note that `increment_date()` now uses the `Fact.date` property setter to
modify the fact, which mostly implements the same logic as the code that
it replaces, except there is a small change in behavior when
`fact.start` is `None`.  However, that is a corner case that probably
almost never occurs and has other problems, so this is left for a later
refactor.

This fixes #590.
  • Loading branch information
matthijskooijman committed Mar 16, 2021
1 parent a3c1840 commit b3dc385
Showing 1 changed file with 1 addition and 7 deletions.
8 changes: 1 addition & 7 deletions src/hamster/edit_activity.py
Expand Up @@ -134,15 +134,8 @@ def date(self):

@date.setter
def date(self, value):
delta = value - self._date if self._date else None
self._date = value
self.cmdline.default_day = value
if self.fact and delta:
if self.fact.start_time:
self.fact.start_time += delta
if self.fact.end_time:
self.fact.end_time += delta
# self.update_fields() here would enter an infinite loop

def on_prev_day_clicked(self, button):
self.increment_date(-1)
Expand All @@ -161,6 +154,7 @@ def get_widget(self, name):
def increment_date(self, days):
delta = dt.timedelta(days=days)
self.date += delta
self.fact.date = self.date
self.update_fields()

def show(self):
Expand Down

0 comments on commit b3dc385

Please sign in to comment.