fix bug when iterating over all existing posts in a thread (#23)

* fix get posts

* remove tail/head and fix posts command

.pylintrc

@@ -54,6 +54,7 @@ confidence=
 
 disable=
     attribute-defined-outside-init,
+    bad-option-value,
     duplicate-code,
     fixme,
     invalid-name,

requirements_dev.txt

@@ -1,5 +1,5 @@
-black==19.3b0
+black
-pylint==2.3.1
+pylint
 pytest==5.1.2
-rope==0.14.0
+pytest-sugar
-rope==0.14.0
+rope

rfd/__init__.py

@@ -2,7 +2,7 @@
 
 from __future__ import unicode_literals
 
-__title__ = 'RFD CLI'
+__title__ = "RFD CLI"
-__author__ = 'Dave Gallant'
+__author__ = "Dave Gallant"
-__license__ = 'Apache 2.0'
+__license__ = "Apache 2.0"
-__copyright__ = '(c) 2018 Dave Gallant'
+__copyright__ = "(c) 2018 Dave Gallant"

rfd/__version__.py

@@ -1,4 +1,4 @@
 # -*- coding: utf-8 -*-
 from __future__ import unicode_literals
 
-version = '0.2.0'
+version = "0.2.1"

rfd/api.py

@@ -5,54 +5,20 @@ try:
 except ImportError:
     JSONDecodeError = ValueError
 import logging
-from math import ceil
 import requests
-from bs4 import BeautifulSoup
+from .constants import API_BASE_URL
-from rfd.constants import API_BASE_URL
+from .format import strip_html, is_valid_url
-
+from .models import Post
-try:
+from .scores import calculate_score
-    from urllib.parse import urlparse  # python 3
+from .utils import is_int
-except ImportError:
-    from urlparse import urlparse  # python 2
-
-
-def build_web_path(slug):
-    return "{}{}".format(API_BASE_URL, slug)
 
 
 def extract_post_id(url):
     return url.split("/")[3].split("-")[-1]
 
 
-def is_int(number):
-    try:
-        int(number)
-        return True
-    except ValueError:
-        return False
-
-
-def calculate_score(post):
-    """Calculate either topic or post score. If votes cannot be retrieved, the score is 0.
-
-    Arguments:
-        post {dict} -- pass in the topic/post object
-
-    Returns:
-        int -- score
-    """
-    score = 0
-    try:
-        score = int(post.get("votes").get("total_up")) - int(
-            post.get("votes").get("total_down")
-        )
-    except AttributeError:
-        pass
-
-    return score
-
-
 def get_safe_per_page(limit):
+    """Ensure that per page limit is between 5-40"""
     if limit < 5:
         return 5
     if limit > 40:
@@ -61,21 +27,13 @@ def get_safe_per_page(limit):
 
 
 def users_to_dict(users):
+    """Create a dictionary of user ids to usernames."""
     users_dict = {}
     for user in users:
         users_dict[user.get("user_id")] = user.get("username")
     return users_dict
 
 
-def strip_html(text):
-    return BeautifulSoup(text, "html.parser").get_text()
-
-
-def is_valid_url(url):
-    result = urlparse(url)
-    return all([result.scheme, result.netloc, result.path])
-
-
 def get_threads(forum_id, limit):
     """Get threads from rfd api
 
@@ -100,40 +58,14 @@ def get_threads(forum_id, limit):
     return None
 
 
-def parse_threads(api_response, limit):
+def get_posts(post):
-    """parse topics list api response into digestible list.
-
-    Arguments:
-        api_response {dict} -- topics response from rfd api
-        limit {int} -- limit number of threads returned
-
-    Returns:
-        list(dict) -- digestible list of threads
-    """
-    threads = []
-    if api_response is None:
-        return threads
-    for topic in api_response.get("topics"):
-        threads.append(
-            {
-                "title": topic.get("title"),
-                "dealer_name": topic["offer"].get("dealer_name"),
-                "score": calculate_score(topic),
-                "url": build_web_path(topic.get("web_path")),
-            }
-        )
-    return threads[:limit]
-
-
-def get_posts(post, count=5, tail=False, per_page=40):
     """Retrieve posts from a thread.
 
     Args:
-        post (str): either post id or full url
+        post (str): either full url or postid
-        count (int, optional): Description
 
     Yields:
-        list(dict): body, score, and user
+        list(Post): Posts
     """
     if is_valid_url(post):
         post_id = extract_post_id(post)
@@ -145,59 +77,27 @@ def get_posts(post, count=5, tail=False, per_page=40):
     response = requests.get(
         "{}/api/topics/{}/posts?per_page=40&page=1".format(API_BASE_URL, post_id)
     )
-    total_posts = response.json().get("pager").get("total")
+
     total_pages = response.json().get("pager").get("total_pages")
 
-    if count == 0:
+    for page in range(0, total_pages + 1):
-        pages = total_pages
-    if count > per_page:
-        if count > total_posts:
-            count = total_posts
-        pages = ceil(count / per_page)
-    else:
-        if tail:
-            pages = total_pages
-        else:
-            pages = 1
-
-    if tail:
-        start_page = ceil((total_posts + 1 - count) / per_page)
-        start_post = (total_posts + 1 - count) % per_page
-        if start_post == 0:
-            start_post = per_page
-    else:
-        start_page, start_post = 0, 0
-
-    # Go through as many pages as necessary
-    for page in range(start_page, pages + 1):
         response = requests.get(
             "{}/api/topics/{}/posts?per_page={}&page={}".format(
-                API_BASE_URL, post_id, get_safe_per_page(per_page), page
+                API_BASE_URL, post_id, 40, page
             )
         )
-
         users = users_to_dict(response.json().get("users"))
 
-        _posts = response.json().get("posts")
+        posts = response.json().get("posts")
 
-        # Determine which post to start with (for --tail)
+        for i in posts:
-        if page == start_page and start_post != 0:
-            if tail:
-                _posts = _posts[start_post - 1 :]
-            else:
-                _posts = _posts[:start_post]
-
-        for _post in _posts:
-            count -= 1
-            if count < 0:
-                return
             # Sometimes votes is null
-            if _post.get("votes") is not None:
+            if i.get("votes") is not None:
-                calculated_score = calculate_score(_post)
+                calculated_score = calculate_score(i)
             else:
                 calculated_score = 0
-            yield {
+            yield Post(
-                "body": strip_html(_post.get("body")),
+                body=strip_html(i.get("body")),
-                "score": calculated_score,
+                score=calculated_score,
-                "user": users[_post.get("author_id")],
+                user=users[i.get("author_id")],
-            }
+            )

rfd/cli.py

@@ -6,8 +6,9 @@ import os
 import sys
 import click
 from colorama import init, Fore, Style
-from rfd.api import parse_threads, get_threads, get_posts
+from .api import get_threads, get_posts
-from rfd.__version__ import version as current_version
+from .parsing import parse_threads
+from .__version__ import version as current_version
 
 init()
 print()
@@ -38,7 +39,7 @@ def get_vote_color(score):
 @click.option("--version/--no-version", default=False)
 @click.pass_context
 def cli(ctx, version):
-    """Welcome to the RFD CLI. (RedFlagDeals.com)"""
+    """CLI for https://forums.redflagdeals.com"""
     if version:
         click.echo(get_version())
     elif not ctx.invoked_subcommand:
@@ -51,16 +52,8 @@ def display_version():
 
 
 @cli.command(short_help="Displays posts in a specific thread.")
-@click.option(
-    "--head", default=0, help="Number of topics. Default is 0, for all topics"
-)
-@click.option(
-    "--tail",
-    default=0,
-    help="Number of topics. Default is disabled. This will override head.",
-)
 @click.argument("post_id")
-def posts(post_id, head, tail):
+def posts(post_id):
     """Displays posts in a specific thread.
 
     post_id can be a full url or post id only
@@ -71,30 +64,17 @@ def posts(post_id, head, tail):
     url: https://forums.redflagdeals.com/koodo-targeted-public-mobile-12-120-koodo-5gb-40-no-referrals-2173603
     post_id: 2173603
     """
-    if head < 0:
-        click.echo("Invalid head.")
-        sys.exit(1)
-
-    if tail < 0:
-        click.echo("Invalid tail.")
-        sys.exit(1)
-
-    # Tail overrides head
-    if tail > 0:
-        count = tail
-    else:
-        count = head
 
     try:
         click.echo("-" * get_terminal_width())
-        for post in get_posts(post=post_id, count=count, tail=tail > 0):
+        for post in get_posts(post=post_id):
             click.echo(
                 " -"
-                + get_vote_color(post.get("score"))
+                + get_vote_color(post.score)
                 + Fore.RESET
-                + post.get("body")
+                + post.body
                 + Fore.YELLOW
-                + " ({})".format(post.get("user"))
+                + " ({})".format(post.user)
             )
             click.echo(Style.RESET_ALL)
             click.echo("-" * get_terminal_width())
@@ -102,7 +82,8 @@ def posts(post_id, head, tail):
         click.echo("Invalid post id.")
         sys.exit(1)
     except AttributeError:
-        click.echo("AttributeError: RFD API did not return expected data.")
+        click.echo("The RFD API did not return the expected data.")
+        sys.exit(1)
 
 
 @cli.command(short_help="Displays threads in the specified forum.")
@@ -131,11 +112,11 @@ def threads(limit, forum_id):
             " "
             + str(i)
             + "."
-            + get_vote_color(thread.get("score"))
+            + get_vote_color(thread.score)
             + Fore.RESET
-            + "[%s] %s" % (thread.get("dealer_name"), thread.get("title"))
+            + "[%s] %s" % (thread.dealer_name, thread.title)
         )
-        click.echo(Fore.BLUE + " {}".format(thread.get("url")))
+        click.echo(Fore.BLUE + " {}".format(thread.url))
         click.echo(Style.RESET_ALL)
 
 

rfd/format.py

@@ -0,0 +1,16 @@
+"""Formatting utils"""
+
+try:
+    from urllib.parse import urlparse  # python 3
+except ImportError:
+    from urlparse import urlparse  # python 2
+from bs4 import BeautifulSoup
+
+
+def strip_html(text):
+    return BeautifulSoup(text, "html.parser").get_text()
+
+
+def is_valid_url(url):
+    result = urlparse(url)
+    return all([result.scheme, result.netloc, result.path])

rfd/models.py

@@ -0,0 +1,14 @@
+# pylint: disable=old-style-class
+class Thread:
+    def __init__(self, title, dealer_name, score, url):
+        self.dealer_name = dealer_name
+        self.score = score
+        self.title = title
+        self.url = url
+
+
+class Post:
+    def __init__(self, body, score, user):
+        self.body = body
+        self.score = score
+        self.user = user

rfd/parsing.py

@@ -0,0 +1,34 @@
+from .constants import API_BASE_URL
+from .scores import calculate_score
+from .models import Thread
+
+
+def build_web_path(slug):
+    return "{}{}".format(API_BASE_URL, slug)
+
+
+def parse_threads(threads, limit):
+    """parse topics list api response into digestible list.
+
+    Arguments:
+        threads {dict} -- topics response from rfd api
+        limit {int} -- limit number of threads returned
+
+    Returns:
+        list(dict) -- digestible list of threads
+    """
+    parsed_threads = []
+    if threads is None:
+        return []
+    for count, topic in enumerate(threads.get("topics"), start=1):
+        if count > limit:
+            break
+        parsed_threads.append(
+            Thread(
+                title=topic.get("title"),
+                dealer_name=topic["offer"].get("dealer_name"),
+                score=calculate_score(topic),
+                url=build_web_path(topic.get("web_path")),
+            )
+        )
+    return parsed_threads

rfd/scores.py

@@ -0,0 +1,18 @@
+def calculate_score(post):
+    """Calculate either topic or post score. If votes cannot be retrieved, the score is 0.
+
+    Arguments:
+        post {dict} -- pass in the topic/post object
+
+    Returns:
+        int -- score
+    """
+    score = 0
+    try:
+        score = int(post.get("votes").get("total_up")) - int(
+            post.get("votes").get("total_down")
+        )
+    except AttributeError:
+        pass
+
+    return score

rfd/utils.py

@@ -0,0 +1,8 @@
+"""This module provides utility functions that are used within rfd"""
+
+def is_int(number):
+    try:
+        int(number)
+        return True
+    except ValueError:
+        return False

setup.py

@@ -23,7 +23,7 @@ with io.open(path.join(WORKING_DIR, "README.md"), encoding="utf-8") as f:
 setup(
     author="Dave Gallant",
     description="CLI for RedFlagDeals.com",
-    entry_points={"console_scripts": ["rfd = rfd.rfd_cli:cli"]},
+    entry_points={"console_scripts": ["rfd = rfd.cli:cli"]},
     install_requires=REQUIREMENTS,
     keywords="cli redflagdeals",
     license="Apache License, Version 2.0",

tests/test_api.py

@@ -1,5 +1,5 @@
-from rfd.api import build_web_path, extract_post_id, parse_threads
+from rfd.api import extract_post_id
-
+from rfd.parsing import build_web_path, parse_threads
 
 def test_build_web_path():
     assert build_web_path("/test") == "https://forums.redflagdeals.com/test"
@@ -76,4 +76,4 @@ def test_parse_threads(threads_api_response):
         ]
     )
 
-    assert len(parse_threads(None, 10)) == 0
+    assert len(parse_threads(None, 10)) is 0