Added tests for update_read.

Also modified “update_read” and renamed “is_unread” to
“tracker_needs_update”. Should be a little bit more readable now (I
hope ^^).

flaskbb/forum/models.py

@@ -457,35 +457,47 @@ class Topic(db.Model):
         db.session.commit()
         return self
 
-    def is_unread(self, user, forumsread):
-        """Returns True if the topic is unread for the user.
-
-        :param user: The user whom should be checked if he has read the topic.
+    def tracker_needs_update(self, forumsread, topicsread):
+        """Returns True if the tracker needs an update.
+        TODO: Couldn't think of a better name for this method - ideas?
 
         :param forumsread: The ForumsRead object is needed because we also
-                           need to check if the topic is below or above the
-                           read_cutoff.
+                           need to check if the forum has been cleared
+                           sometime ago.
+
+        :param topicsread: The topicsread object is used to check if there is
+                           a new post in the topic.
         """
-        read_cutoff = datetime.utcnow() - timedelta(
-            days=current_app.config['TRACKER_LENGTH'])
+        read_cutoff = None
+        if current_app.config['TRACKER_LENGTH'] > 0:
+            read_cutoff = datetime.utcnow() - timedelta(
+                days=current_app.config['TRACKER_LENGTH'])
+
+        # The tracker is disabled - abort
+        if read_cutoff is None:
+            return False
 
-        # Anonymous User or the post is too old for inserting it in the
-        # TopicsRead model
-        if not user.is_authenticated() or \
-                read_cutoff > self.last_post.date_created:
+        # Else the topic is still below the read_cutoff
+        elif read_cutoff > self.last_post.date_created:
             return False
 
-        # Can be None if the user has never marked the forum as read. If this
-        # condition is false - we need to update the tracker
+        # Can be None (cleared) if the user has never marked the forum as read.
+        # If this condition is false - we need to update the tracker
         if forumsread and forumsread.cleared is not None and \
                 forumsread.cleared >= self.last_post.date_created:
             return False
 
+        if topicsread and topicsread.last_read >= self.last_post.date_created:
+            return False
+
         return True
 
     def update_read(self, user, forum, forumsread):
-        """Update the topics read status if the user hasn't read the latest
-        post. Returns True if the tracker has been updated.
+        """Updates the topicsread and forumsread tracker for a specified user,
+        if the topic contains new posts or the user hasn't read the topic.
+        Also, if the ``TRACKER_LENGTH`` is configured, it will just recognize
+        topics that are newer than the ``TRACKER_LENGTH`` (days) as unread.
+        Returns True if the tracker has been updated.
 
         :param user: The user for whom the readstracker should be updated.
 
@@ -495,20 +507,24 @@ class Topic(db.Model):
                            is a new post since the forum has been marked as
                            read.
         """
-        if not self.is_unread(user, forumsread):
+        # User is not logged in - abort
+        if not user.is_authenticated():
             return False
 
         topicsread = TopicsRead.query.\
             filter(TopicsRead.user_id == user.id,
                    TopicsRead.topic_id == self.id).first()
 
+        if not self.tracker_needs_update(forumsread, topicsread):
+            return False
+
         # Because we return True/False if the trackers have been
         # updated, we need to store the status in a temporary variable
         updated = False
 
         # A new post has been submitted that the user hasn't read.
         # Updating...
-        if topicsread and (topicsread.last_read < self.last_post.date_created):
+        if topicsread:
             topicsread.last_read = datetime.utcnow()
             topicsread.save()
             updated = True
@@ -528,8 +544,8 @@ class Topic(db.Model):
         else:
             updated = False
 
-        if forum:
-            updated = forum.update_read(user, forumsread, topicsread)
+        # Save True/False if the forums tracker has been updated.
+        updated = forum.update_read(user, forumsread, topicsread)
 
         return updated
 
@@ -606,19 +622,28 @@ class Forum(db.Model):
 
         db.session.commit()
 
-    def is_unread(self, user, forumsread, topicsread):
-        """Returns True if the forum is unread for the specified user.
+    def update_read(self, user, forumsread, topicsread):
+        """Updates the ForumsRead status for the user. In order to work
+        correctly, be sure that `topicsread is **not** `None`.
 
-        :param user: The user who should be checked if he has read the forum.
+        :param user: The user for whom we should check if he has read the
+                     forum.
 
-        :param forumsread: The forumsread object is needed because we are
-                           going to check if the forum is unread.
+        :param forumsread: The forumsread object. It is needed to check if
+                           if the forum is unread. If `forumsread` is `None`
+                           and the forum is unread, it will create a new entry
+                           in the `ForumsRead` relation, else (and the forum
+                           is still unread) we are just going to update the
+                           entry in the `ForumsRead` relation.
 
         :param topicsread: The topicsread object is used in combination
                            with the forumsread object to check if the
                            forumsread relation should be updated and
                            therefore is unread.
         """
+        if not user.is_authenticated() or topicsread is None:
+            return False
+
         # fetch the unread posts in the forum
         unread_count = Topic.query.\
             outerjoin(TopicsRead,
@@ -632,43 +657,29 @@ class Forum(db.Model):
                           TopicsRead.last_read < Topic.last_updated)).\
             count()
 
-
         # No unread topics available - trying to mark the forum as read
         if unread_count == 0:
-            # ForumsRead is already up-to-date.
-            # This condition is used to prevent updating the forumsread
-            # everytime you visit a topic in the forum
+
             if forumsread and forumsread.last_read > topicsread.last_read:
                 return False
 
-            # else the forum is unread
-            return True
-
-        # if > 0 unread topics - the forum is unread
-        return True
-
-    def update_read(self, user, forumsread, topicsread):
-        """Updates the ForumsRead status for the user
-
-        :param user: The user whom should be checked if he has read the forum.
-        """
-        if not self.is_unread(user, forumsread, topicsread):
-            return False
-
-        # ForumRead Entry exists - Updating it because a new post
-        # has been submitted that the user hasn't read.
-        if forumsread:
+            # ForumRead Entry exists - Updating it because a new post
+            # has been submitted that the user hasn't read.
+            elif forumsread:
+                forumsread.last_read = datetime.utcnow()
+                forumsread.save()
+                return True
+
+            # No ForumRead Entry existing - creating one.
+            forumsread = ForumsRead()
+            forumsread.user_id = user.id
+            forumsread.forum_id = self.id
             forumsread.last_read = datetime.utcnow()
             forumsread.save()
             return True
 
-        # No ForumRead Entry existing - creating one.
-        forumsread = ForumsRead()
-        forumsread.user_id = user.id
-        forumsread.forum_id = self.id
-        forumsread.last_read = datetime.utcnow()
-        forumsread.save()
-        return True
+        # Nothing updated, because there are still more than 0 unread topics
+        return False
 
     def save(self, moderators=None):
         """Saves a forum"""

tests/unit/test_forum_models.py

@@ -1,4 +1,10 @@
-from flaskbb.forum.models import Category, Forum, Topic, Post
+from datetime import datetime
+
+from flask import current_app
+from flask.ext.login import login_user, current_user, logout_user
+
+from flaskbb.forum.models import Category, Forum, Topic, Post, ForumsRead, \
+    TopicsRead
 from flaskbb.user.models import User
 
 
@@ -97,6 +103,81 @@ def test_forum_update_last_post(topic_normal, normal_user):
     assert topic_normal.forum.last_post == topic_normal.first_post
 
 
+def test_forum_update_read(database, normal_user, topic_normal):
+    forumsread = ForumsRead.query.\
+        filter(ForumsRead.user_id == normal_user.id,
+               ForumsRead.forum_id == topic_normal.forum_id).first()
+
+    topicsread = TopicsRead.query.\
+        filter(TopicsRead.user_id == normal_user.id,
+               TopicsRead.topic_id == topic_normal.id).first()
+
+    forum = topic_normal.forum
+
+    with current_app.test_request_context():
+        # Test with logged in user
+        login_user(normal_user)
+
+        # Should return False because topicsread is None
+        assert not forum.update_read(current_user, forumsread, topicsread)
+
+        # This is the first time the user visits the topic
+        topicsread = TopicsRead()
+        topicsread.user_id = normal_user.id
+        topicsread.topic_id = topic_normal.id
+        topicsread.forum_id = topic_normal.forum_id
+        topicsread.last_read = datetime.utcnow()
+        topicsread.save()
+
+        # hence, we also need to create a new entry
+        assert forum.update_read(current_user, forumsread, topicsread)
+
+        forumsread = ForumsRead.query.\
+            filter(ForumsRead.user_id == normal_user.id,
+                   ForumsRead.forum_id == topic_normal.forum_id).first()
+
+        # everything should be up-to-date now
+        assert not forum.update_read(current_user, forumsread, topicsread)
+
+        post = Post(content="Test Content")
+        post.save(user=normal_user, topic=topic_normal)
+
+        # Updating the topicsread tracker
+        topicsread.last_read = datetime.utcnow()
+        topicsread.save()
+
+        # now the forumsread tracker should also need an update
+        assert forum.update_read(current_user, forumsread, topicsread)
+
+        logout_user()
+        # should fail because the user is logged out
+        assert not forum.update_read(current_user, forumsread, topicsread)
+
+
+def test_forum_update_read_two_topics(database, normal_user, topic_normal,
+                                      topic_moderator):
+    forumsread = ForumsRead.query.\
+        filter(ForumsRead.user_id == normal_user.id,
+               ForumsRead.forum_id == topic_normal.forum_id).first()
+
+    forum = topic_normal.forum
+
+    with current_app.test_request_context():
+        # Test with logged in user
+        login_user(normal_user)
+
+        # This is the first time the user visits the topic
+        topicsread = TopicsRead()
+        topicsread.user_id = normal_user.id
+        topicsread.topic_id = topic_normal.id
+        topicsread.forum_id = topic_normal.forum_id
+        topicsread.last_read = datetime.utcnow()
+        topicsread.save()
+
+        # will not create a entry because there is still one unread topic
+        assert not forum.update_read(current_user, forumsread, topicsread)
+
+
 def test_forum_url(forum):
     assert forum.url == "http://localhost:5000/forum/1-test-forum"
 
@@ -204,9 +285,104 @@ def test_topic_move_same_forum(topic_normal):
     assert not topic_normal.move(topic_normal.forum)
 
 
-def test_topic_update_read():
-    # TODO: Refactor it, to make it easier to test it
-    pass
+def test_topic_tracker_needs_update(database, normal_user, topic_normal):
+    forumsread = ForumsRead.query.\
+        filter(ForumsRead.user_id == normal_user.id,
+               ForumsRead.forum_id == topic_normal.forum_id).first()
+
+    topicsread = TopicsRead.query.\
+        filter(TopicsRead.user_id == normal_user.id,
+               TopicsRead.topic_id == topic_normal.id).first()
+
+    with current_app.test_request_context():
+        assert topic_normal.tracker_needs_update(forumsread, topicsread)
+
+        # Update the tracker
+        topicsread = TopicsRead()
+        topicsread.user_id = normal_user.id
+        topicsread.topic_id = topic_normal.id
+        topicsread.forum_id = topic_normal.forum_id
+        topicsread.last_read = datetime.utcnow()
+        topicsread.save()
+
+        forumsread = ForumsRead()
+        forumsread.user_id = normal_user.id
+        forumsread.forum_id = topic_normal.forum_id
+        forumsread.last_read = datetime.utcnow()
+        forumsread.save()
+
+        # Now the topic should be read
+        assert not topic_normal.tracker_needs_update(forumsread, topicsread)
+
+        post = Post(content="Test Content")
+        post.save(topic=topic_normal, user=normal_user)
+
+        assert topic_normal.tracker_needs_update(forumsread, topicsread)
+
+
+def test_topic_tracker_needs_update_cleared(database, normal_user, topic_normal):
+    forumsread = ForumsRead.query.\
+        filter(ForumsRead.user_id == normal_user.id,
+               ForumsRead.forum_id == topic_normal.forum_id).first()
+
+    topicsread = TopicsRead.query.\
+        filter(TopicsRead.user_id == normal_user.id,
+               TopicsRead.topic_id == topic_normal.id).first()
+
+    with current_app.test_request_context():
+        assert topic_normal.tracker_needs_update(forumsread, topicsread)
+
+        # Update the tracker
+        forumsread = ForumsRead()
+        forumsread.user_id = normal_user.id
+        forumsread.forum_id = topic_normal.forum_id
+        forumsread.last_read = datetime.utcnow()
+        forumsread.cleared = datetime.utcnow()
+        forumsread.save()
+
+        # Now the topic should be read
+        assert not topic_normal.tracker_needs_update(forumsread, topicsread)
+
+
+def test_topic_update_read(database, normal_user, topic_normal):
+    forumsread = ForumsRead.query.\
+        filter(ForumsRead.user_id == normal_user.id,
+               ForumsRead.forum_id == topic_normal.forum_id).first()
+
+    with current_app.test_request_context():
+        # Test with logged in user
+        login_user(normal_user)
+        assert current_user.is_authenticated()
+
+        # Update the tracker
+        assert topic_normal.update_read(current_user, topic_normal.forum,
+                                        forumsread)
+        # Because the tracker is already up-to-date, it shouldn't update it
+        # again.
+        assert not topic_normal.update_read(current_user, topic_normal.forum,
+                                            forumsread)
+
+        # Adding a new post - now the tracker shouldn't be up-to-date anymore.
+        post = Post(content="Test Content")
+        post.save(topic=topic_normal, user=normal_user)
+
+        forumsread = ForumsRead.query.\
+            filter(ForumsRead.user_id == normal_user.id,
+                   ForumsRead.forum_id == topic_normal.forum_id).first()
+
+        # Test tracker length
+        current_app.config["TRACKER_LENGTH"] = 0
+        assert not topic_normal.update_read(current_user, topic_normal.forum,
+                                            forumsread)
+        current_app.config["TRACKER_LENGTH"] = 1
+        assert topic_normal.update_read(current_user, topic_normal.forum,
+                                        forumsread)
+
+        # Test with logged out user
+        logout_user()
+        assert not current_user.is_authenticated()
+        assert not topic_normal.update_read(current_user, topic_normal.forum,
+                                            forumsread)
 
 
 def test_topic_url(topic_normal):