Skip to content

Commit

Permalink
Set Entry.{read,important}_modified in Reader.mark_as_...().
Browse files Browse the repository at this point in the history
Add a round-trip test.

For #254.
  • Loading branch information
lemon24 committed Sep 29, 2021
1 parent bcbeb9f commit 81d7919
Show file tree
Hide file tree
Showing 4 changed files with 95 additions and 25 deletions.
29 changes: 23 additions & 6 deletions src/reader/_storage.py
Original file line number Diff line number Diff line change
Expand Up @@ -635,30 +635,47 @@ def mark_as_stale(self, url: str) -> None:
rowcount_exactly_one(cursor, lambda: FeedNotFoundError(url))

@wrap_exceptions(StorageError)
def mark_as_read_unread(self, feed_url: str, entry_id: str, read: bool) -> None:
def mark_as_read_unread(
self, feed_url: str, entry_id: str, read: bool, modified: Optional[datetime]
) -> None:
with self.db:
cursor = self.db.execute(
"""
UPDATE entries
SET read = :read
SET
read = :read,
read_modified = :modified
WHERE feed = :feed_url AND id = :entry_id;
""",
dict(feed_url=feed_url, entry_id=entry_id, read=read),
dict(
feed_url=feed_url, entry_id=entry_id, read=read, modified=modified
),
)
rowcount_exactly_one(cursor, lambda: EntryNotFoundError(feed_url, entry_id))

@wrap_exceptions(StorageError)
def mark_as_important_unimportant(
self, feed_url: str, entry_id: str, important: bool
self,
feed_url: str,
entry_id: str,
important: bool,
modified: Optional[datetime],
) -> None:
with self.db:
cursor = self.db.execute(
"""
UPDATE entries
SET important = :important
SET
important = :important,
important_modified = :modified
WHERE feed = :feed_url AND id = :entry_id;
""",
dict(feed_url=feed_url, entry_id=entry_id, important=important),
dict(
feed_url=feed_url,
entry_id=entry_id,
important=important,
modified=modified,
),
)
rowcount_exactly_one(cursor, lambda: EntryNotFoundError(feed_url, entry_id))

Expand Down
14 changes: 10 additions & 4 deletions src/reader/core.py
Original file line number Diff line number Diff line change
Expand Up @@ -1049,6 +1049,8 @@ def get_entries(
'updated',
'published',
'last_updated',
'read_modified',
'important_modified',
feed=fix_datetime_tzinfo(
rv_entry.feed, 'updated', 'added', 'last_updated'
),
Expand Down Expand Up @@ -1145,8 +1147,9 @@ def mark_entry_as_read(self, entry: EntryInput) -> None:
Renamed from :meth:`mark_as_read`.
"""
modified = self._now()
feed_url, entry_id = _entry_argument(entry)
self._storage.mark_as_read_unread(feed_url, entry_id, True)
self._storage.mark_as_read_unread(feed_url, entry_id, True, modified)

def mark_entry_as_unread(self, entry: EntryInput) -> None:
"""Mark an entry as unread.
Expand All @@ -1162,8 +1165,9 @@ def mark_entry_as_unread(self, entry: EntryInput) -> None:
Renamed from :meth:`mark_as_unread`.
"""
modified = self._now()
feed_url, entry_id = _entry_argument(entry)
self._storage.mark_as_read_unread(feed_url, entry_id, False)
self._storage.mark_as_read_unread(feed_url, entry_id, False, modified)

def mark_entry_as_important(self, entry: EntryInput) -> None:
"""Mark an entry as important.
Expand All @@ -1179,8 +1183,9 @@ def mark_entry_as_important(self, entry: EntryInput) -> None:
Renamed from :meth:`mark_as_important`.
"""
modified = self._now()
feed_url, entry_id = _entry_argument(entry)
self._storage.mark_as_important_unimportant(feed_url, entry_id, True)
self._storage.mark_as_important_unimportant(feed_url, entry_id, True, modified)

def mark_entry_as_unimportant(self, entry: EntryInput) -> None:
"""Mark an entry as unimportant.
Expand All @@ -1196,8 +1201,9 @@ def mark_entry_as_unimportant(self, entry: EntryInput) -> None:
Renamed from :meth:`mark_as_unimportant`.
"""
modified = self._now()
feed_url, entry_id = _entry_argument(entry)
self._storage.mark_as_important_unimportant(feed_url, entry_id, False)
self._storage.mark_as_important_unimportant(feed_url, entry_id, False, modified)

def get_feed_metadata(
self,
Expand Down
52 changes: 47 additions & 5 deletions tests/test_reader.py
Original file line number Diff line number Diff line change
Expand Up @@ -1705,9 +1705,9 @@ def __init__(self, exc=None):
def close(self):
self.calls.append(('close',))

def mark_as_important_unimportant(self, feed_url, entry_id, important):
def mark_as_important_unimportant(self, feed_url, entry_id, important, modified):
self.calls.append(
('mark_as_important_unimportant', feed_url, entry_id, important)
('mark_as_important_unimportant', feed_url, entry_id, important, modified)
)
if self.exc:
raise self.exc
Expand All @@ -1724,21 +1724,34 @@ def get_entries(self, *args):


def test_mark_as_important(reader, entry_arg):

reader._storage = FakeStorage()
reader._now = lambda: naive_datetime(2010, 1, 1)
entry = Entry('entry', None, feed=Feed('feed'))
reader.mark_entry_as_important(entry_arg(entry))
assert reader._storage.calls == [
('mark_as_important_unimportant', 'feed', 'entry', True)
(
'mark_as_important_unimportant',
'feed',
'entry',
True,
naive_datetime(2010, 1, 1),
)
]


def test_mark_as_unimportant(reader, entry_arg):
reader._storage = FakeStorage()
reader._now = lambda: naive_datetime(2010, 1, 1)
entry = Entry('entry', None, feed=Feed('feed'))
reader.mark_entry_as_unimportant(entry_arg(entry))
assert reader._storage.calls == [
('mark_as_important_unimportant', 'feed', 'entry', False)
(
'mark_as_important_unimportant',
'feed',
'entry',
False,
naive_datetime(2010, 1, 1),
)
]


Expand Down Expand Up @@ -3078,3 +3091,32 @@ def test_reserved_names(make_reader):

with pytest.raises(TypeError):
reader.reserved_name_scheme['separator'] = '.'


@pytest.mark.parametrize('flag', ['read', 'important'])
@rename_argument('reader', 'reader_with_one_feed')
def test_entry_read_important_modified(reader, flag):
reader.update_feeds()

entry = next(reader.get_entries())
assert not getattr(entry, flag)
assert getattr(entry, f'{flag}_modified') is None

reader._now = lambda: naive_datetime(2010, 1, 1)
getattr(reader, f'mark_entry_as_{flag}')(entry)
entry = next(reader.get_entries())
assert getattr(entry, flag)
assert getattr(entry, f'{flag}_modified') == datetime(2010, 1, 1)

reader._now = lambda: naive_datetime(2010, 1, 2)
getattr(reader, f'mark_entry_as_{flag}')(entry)
entry = next(reader.get_entries())
assert getattr(entry, f'{flag}_modified') == datetime(2010, 1, 2)

# TODO: set to specific datetime, set to None

reader._now = lambda: naive_datetime(2010, 1, 3)
getattr(reader, f'mark_entry_as_un{flag}')(entry)
entry = next(reader.get_entries())
assert not getattr(entry, flag)
assert getattr(entry, f'{flag}_modified') == datetime(2010, 1, 3)
25 changes: 15 additions & 10 deletions tests/test_storage.py
Original file line number Diff line number Diff line change
Expand Up @@ -198,7 +198,11 @@ def mark_as_stale(storage, feed, __):


def mark_as_read_unread(storage, feed, entry):
storage.mark_as_read_unread(feed.url, entry.id, 1)
storage.mark_as_read_unread(feed.url, entry.id, 1, None)


def mark_as_important_unimportant(storage, feed, entry):
storage.mark_as_important_unimportant(feed.url, entry.id, 1, None)


def update_feed(storage, feed, entry):
Expand Down Expand Up @@ -283,6 +287,7 @@ def get_entry_last(storage, feed, entry):
set_feed_updates_enabled,
mark_as_stale,
mark_as_read_unread,
mark_as_important_unimportant,
update_feed,
update_feed_last_updated,
add_or_update_entry,
Expand Down Expand Up @@ -435,9 +440,9 @@ def check_iter_locked(db_path, pre_stuff, iter_stuff):

# shouldn't raise an exception
storage = Storage(db_path, timeout=0, wal_enabled=False)
storage.mark_as_read_unread(feed.url, entry.id, 1)
storage.mark_as_read_unread(feed.url, entry.id, 1, None)
storage = Storage(db_path, timeout=0)
storage.mark_as_read_unread(feed.url, entry.id, 0)
storage.mark_as_read_unread(feed.url, entry.id, 0, None)


def test_update_feed_last_updated_not_found(db_path):
Expand Down Expand Up @@ -525,7 +530,7 @@ def storage():

def test_entry_remains_read_after_update(storage_with_two_entries):
storage = storage_with_two_entries
storage.mark_as_read_unread('feed', 'one', True)
storage.mark_as_read_unread('feed', 'one', True, None)

storage.add_or_update_entry(
EntryUpdateIntent(
Expand Down Expand Up @@ -581,7 +586,7 @@ def test_important_unimportant_by_default(storage):

@rename_argument('storage', 'storage_with_two_entries')
def test_important_get_entries(storage):
storage.mark_as_important_unimportant('feed', 'one', True)
storage.mark_as_important_unimportant('feed', 'one', True, datetime(2010, 1, 2))

assert {e.id for e in storage.get_entries(now=datetime(2010, 1, 1))} == {
'one',
Expand Down Expand Up @@ -609,7 +614,7 @@ def test_important_get_entries(storage):

@rename_argument('storage', 'storage_with_two_entries')
def test_important_entry_remains_important_after_update(storage):
storage.mark_as_important_unimportant('feed', 'one', True)
storage.mark_as_important_unimportant('feed', 'one', True, None)

storage.add_or_update_entry(
EntryUpdateIntent(
Expand All @@ -631,7 +636,7 @@ def test_important_entry_remains_important_after_update(storage):

@rename_argument('storage', 'storage_with_two_entries')
def test_important_entry_important(storage):
storage.mark_as_important_unimportant('feed', 'one', True)
storage.mark_as_important_unimportant('feed', 'one', True, None)

assert {e.id: e.important for e in storage.get_entries(datetime(2010, 1, 1))} == {
'one': True,
Expand All @@ -641,8 +646,8 @@ def test_important_entry_important(storage):

@rename_argument('storage', 'storage_with_two_entries')
def test_important_mark_as_unimportant(storage):
storage.mark_as_important_unimportant('feed', 'one', True)
storage.mark_as_important_unimportant('feed', 'one', False)
storage.mark_as_important_unimportant('feed', 'one', True, None)
storage.mark_as_important_unimportant('feed', 'one', False, None)

assert {
e.id
Expand All @@ -654,7 +659,7 @@ def test_important_mark_as_unimportant(storage):

def test_important_mark_entry_not_found(storage):
with pytest.raises(EntryNotFoundError):
storage.mark_as_important_unimportant('feed', 'one', True)
storage.mark_as_important_unimportant('feed', 'one', True, None)


def test_minimum_sqlite_version(db_path, monkeypatch):
Expand Down

0 comments on commit 81d7919

Please sign in to comment.