From 3ec2a2902caaf3e70deea650425339299af5d821 Mon Sep 17 00:00:00 2001
From: zlg <zlg@zlg.space>
Date: Thu, 18 Oct 2018 16:36:39 -0700
Subject: Catch when an invalid list filter is passed

Before, vgstash.DB.list_games() would default to 'allgames' and silently
hide it when a filter wasn't found. This commit ensures that the vgstash
package and CLI both indicate when an invalid filter is passed to them:

* vgstash.DB.list_games() will return False on a failure to match;
* vgstash_cli uses Click's Choice object to enforce the constraint
---
 src/vgstash/__init__.py   | 12 ++++++++++--
 src/vgstash_cli.py        |  4 +++-
 tests/test_vgstash.py     |  4 ++++
 tests/test_vgstash_cli.py |  7 +++++++
 4 files changed, 24 insertions(+), 3 deletions(-)

diff --git a/src/vgstash/__init__.py b/src/vgstash/__init__.py
index e3ede03..b3bd9b9 100755
--- a/src/vgstash/__init__.py
+++ b/src/vgstash/__init__.py
@@ -277,9 +277,17 @@ class DB(object):
         return ret
 
     def list_games(self, filter='allgames'):
+        """
+        Return a list of games that meet the filter criteria.
+
+        If the filter exists, it returns a list of sqlite3.Row objects. If the
+        filter does not exist, it returns False. If no filter is specified, it
+        will return a list of all games in the database.
+        """
         if filter not in FILTERS.keys():
-            filter = 'allgames'
-        return self.conn.execute(FILTERS[filter]).fetchall()
+            return False
+        else:
+            return self.conn.execute(FILTERS[filter]).fetchall()
 
     def update_filter(self, filter_name, stmt):
         """
diff --git a/src/vgstash_cli.py b/src/vgstash_cli.py
index 22cfc5a..db5853b 100644
--- a/src/vgstash_cli.py
+++ b/src/vgstash_cli.py
@@ -108,13 +108,15 @@ def add(title, system, ownership, progress, notes):
 
 
 @cli.command('list')
-@click.argument('filter', required=False, default="allgames")
+@click.argument('filter', type=click.Choice(vgstash.FILTERS.keys()), required=False, default="allgames")
 @click.option('--raw', '-r', is_flag=True, show_default=True, default=False, help="Output raw, pipe-delimited lines")
 @click.option('--width', '-w', type=str, required=False, default=get_terminal_size(fallback=(80,24)).columns, help="The width of the table to output, in characters.")
 def list_games(filter, raw, width):
     db = get_db()
     res = db.list_games(filter)
     first_pass = True
+    # res can be False if the filter doesn't exist, but Click should catch it
+    # and spit out an error before this function even starts.
     for r in res:
         if 'notes' in r.keys() and len(r['notes']) > 0:
             notes = r['notes'].replace('\n', '\\n')
diff --git a/tests/test_vgstash.py b/tests/test_vgstash.py
index 74f3c43..43cd639 100644
--- a/tests/test_vgstash.py
+++ b/tests/test_vgstash.py
@@ -129,6 +129,10 @@ def test_db_list_games(vgstash_db):
     assert isinstance(res, list)
     assert isinstance(res[0], sqlite3.Row)
 
+def test_db_list_games_not_found(vgstash_db):
+    res = vgstash_db.list_games("derp")
+    assert res == False
+
 def test_db_add_filter(vgstash_db):
     assert vgstash_db.add_filter("db_add_filter", "SELECT * FROM games WHERE system = 'system2'")
     assert vgstash_db.has_filter("db_add_filter")
diff --git a/tests/test_vgstash_cli.py b/tests/test_vgstash_cli.py
index 02f0d81..f34f2d1 100644
--- a/tests/test_vgstash_cli.py
+++ b/tests/test_vgstash_cli.py
@@ -99,6 +99,13 @@ def test_list_filter():
         'The Legend of Zelda|NES|2|2|\n',
     ))
 
+def test_list_filter_invalid():
+    runner = CliRunner()
+    result = runner.invoke(vgstash_cli.cli, ['list', '-r', 'derp'])
+    if verbose:
+        print(result.output)
+    assert result.exit_code != 0
+
 
 def test_list_pretty():
     runner = CliRunner()
-- 
cgit v1.2.3-54-g00ecf