Repository: abh1nav/gnippy

Issue:
New rule format breaks delete example
The examples in the README for deleting rules are broken due to the addition of the id field in the rules returned by GNIP.

    from gnippy import rules
    from gnippy.errors import RuleDeleteFailedException, RulesGetFailedException

    try:
        rules_list = rules.get_rules()
        # Suppose I want to delete the first rule in the list
        rules.delete_rule(rules_list[0])
        # OR ... I want to delete ALL rules
        rules.delete_rules(rules_list)

    except RuleDeleteFailedException, RulesGetFailedException:
        pass


At the moment a user needs to either manually remove the `id` field 

    del rule['id']
    rules.delete_rule(rule)

or rebuild the rule using `rules.build`

     rules.delete_rule(rules.build(rule['value'], rule.get('tag')))



Discussion:

Patch:
diff --git a/gnippy/rules.py b/gnippy/rules.py
index 37877f8..be59fcc 100644
--- a/gnippy/rules.py
+++ b/gnippy/rules.py
@@ -31,7 +31,7 @@ def _check_rules_list(rules_list):
     if not isinstance(rules_list, list):
         fail()
 
-    expected = ("value", "tag")
+    expected = ("value", "tag", "id")
     for r in rules_list:
         if not isinstance(r, dict):
             fail()
@@ -77,6 +77,7 @@ def _post(conf, built_rules):
         error_text = "HTTP Response Code: %s, Text: '%s'" % (str(r.status_code), r.text)
         raise RuleAddFailedException(error_text)
 
+
 def _generate_delete_url(conf):
     """
         Generate the Rules URL for a DELETE request.
@@ -89,6 +90,7 @@ def _generate_delete_url(conf):
     else:
         return rules_url + "?_method=delete"
 
+
 def _delete(conf, built_rules):
     """
         Generate the Delete Rules URL and make a POST request.
@@ -112,6 +114,7 @@ def _delete(conf, built_rules):
         error_text = "HTTP Response Code: %s, Text: '%s'" % (str(r.status_code), r.text)
         raise RuleDeleteFailedException(error_text)
 
+
 def build(rule_string, tag=None):
     """
         Takes a rule string and optional tag and turns it into a "built_rule" that looks like:


Test patch:
diff --git a/gnippy/test/test_rules.py b/gnippy/test/test_rules.py
index cfa1ee4..cbb4202 100644
--- a/gnippy/test/test_rules.py
+++ b/gnippy/test/test_rules.py
@@ -42,7 +42,8 @@ def good_get_no_rules(url, auth):
 
 
 def good_get_one_rule(url, auth):
-    return test_utils.GoodResponse(json={"rules":[{"value": "Hello", "tag": "mytag"}]})
+    return test_utils.GoodResponse(
+        json={"rules": [{"value": "Hello", "tag": "mytag"}]})
 
 
 def bad_delete(url, auth, data):
@@ -54,7 +55,6 @@ def good_delete(url, auth, data):
 
 
 class RulesTestCase(unittest.TestCase):
-
     rule_string = "Hello OR World"
     tag = "my_tag"
 
@@ -79,45 +79,53 @@ class RulesTestCase(unittest.TestCase):
 
     def test_check_one_rule_ok(self):
         """ Check list of one rule. """
-        l = [ { "value": "hello" } ]
+        l = [{"value": "hello"}]
         rules._check_rules_list(l)
 
     def test_check_many_rules_ok(self):
         """ Check list of many rules. """
-        l = [ { "value": "hello" }, { "value": "h", "tag": "w" }]
+        l = [
+            {"value": "hello", "id": 3},
+            {"value": "goodbye", "tag": "w", "id": 4},
+            {"value": "hi again", "tag": "x"},
+            {"value": "bye again"}
+        ]
         rules._check_rules_list(l)
 
     def test_check_rule_tag_none(self):
         """ Check list of rules both with tag and without. """
-        l = [ { "value": "hello", "tag": None }, { "value": "h", "tag": "w" }]
+        l = [{"value": "hello", "tag": None}, {"value": "h", "tag": "w"}]
         rules._check_rules_list(l)
 
     def test_check_one_rule_typo_values(self):
         """ Make sure incorectly formatted rule values fail. """
-        l = [ { "values": "hello" } ]
+        l = [{"values": "hello"}]
         try:
             rules._check_rules_list(l)
         except RulesListFormatException:
             return
-        self.fail("_check_rules_list was supposed to throw a RuleFormatException")
+        self.fail(
+            "_check_rules_list was supposed to throw a RuleFormatException")
 
     def test_check_one_rule_typo_tag(self):
         """ Make sure incorrectly formatted rule tags fail. """
-        l = [ { "value": "hello", "tags": "t" } ]
+        l = [{"value": "hello", "tags": "t"}]
         try:
             rules._check_rules_list(l)
         except RulesListFormatException:
             return
-        self.fail("_check_rules_list was supposed to throw a RuleFormatException")
+        self.fail(
+            "_check_rules_list was supposed to throw a RuleFormatException")
 
     def test_check_one_rule_extra_stuff_in_rule(self):
         """ Make sure rules with unexpected keys fail. """
-        l = [ { "value": "hello", "wat": "man" } ]
+        l = [{"value": "hello", "wat": "man"}]
         try:
             rules._check_rules_list(l)
         except RulesListFormatException:
             return
-        self.fail("_check_rules_list was supposed to throw a RuleFormatException")
+        self.fail(
+            "_check_rules_list was supposed to throw a RuleFormatException")
 
     def test_build_rule_bad_args(self):
         """ Make sure building rules with unexpected args fail. """
@@ -125,7 +133,8 @@ class RulesTestCase(unittest.TestCase):
             rules.build(None)
         except BadArgumentException:
             return
-        self.fail("rules.build_rule was supposed to throw a BadArgumentException")
+        self.fail(
+            "rules.build_rule was supposed to throw a BadArgumentException")
 
     def test_build_rule_without_tag(self):
         """ Build rule without tag. """
@@ -149,18 +158,21 @@ class RulesTestCase(unittest.TestCase):
             rules.add_rule(self.rule_string, self.tag)
         except ConfigFileNotFoundException:
             return
-        self.fail("Rule Add was supposed to fail and throw a ConfigFileNotFoundException")
+        self.fail(
+            "Rule Add was supposed to fail and throw a ConfigFileNotFoundException")
 
     @mock.patch('requests.post', good_post)
     def test_add_one_rule_ok(self):
         """Add one rule with config. """
-        rules.add_rule(self.rule_string, self.tag, config_file_path=test_utils.test_config_path)
+        rules.add_rule(self.rule_string, self.tag,
+                       config_file_path=test_utils.test_config_path)
 
     @mock.patch('requests.post', bad_post)
     def test_add_one_rule_not_ok(self):
         """Add one rule with exception thrown. """
         try:
-            rules.add_rule(self.rule_string, self.tag, config_file_path=test_utils.test_config_path)
+            rules.add_rule(self.rule_string, self.tag,
+                           config_file_path=test_utils.test_config_path)
         except RuleAddFailedException:
             return
         self.fail("Rule Add was supposed to fail and throw a RuleAddException")
@@ -173,20 +185,23 @@ class RulesTestCase(unittest.TestCase):
             rules.add_rule(self.rule_string, self.tag)
         except ConfigFileNotFoundException:
             return
-        self.fail("Rule Add was supposed to fail and throw a ConfigFileNotFoundException")
+        self.fail(
+            "Rule Add was supposed to fail and throw a ConfigFileNotFoundException")
 
     @mock.patch('requests.post', good_post)
     def test_add_many_rules_ok(self):
         """ Add many rules. """
         rules_list = self._generate_rules_list()
-        rules.add_rules(rules_list, config_file_path=test_utils.test_config_path)
+        rules.add_rules(rules_list,
+                        config_file_path=test_utils.test_config_path)
 
     @mock.patch('requests.post', bad_post)
     def test_add_many_rules_not_ok(self):
         """ Add many rules with exception thrown. """
         try:
             rules_list = self._generate_rules_list()
-            rules.add_rules(rules_list, config_file_path=test_utils.test_config_path)
+            rules.add_rules(rules_list,
+                            config_file_path=test_utils.test_config_path)
         except RuleAddFailedException:
             return
         self.fail("Rule Add was supposed to fail and throw a RuleAddException")
@@ -226,7 +241,8 @@ class RulesTestCase(unittest.TestCase):
         try:
             r = rules.get_rules(config_file_path=test_utils.test_config_path)
         except RulesGetFailedException as e:
-            self.assertTrue("GNIP API response did not return a rules object" in str(e))
+            self.assertTrue(
+                "GNIP API response did not return a rules object" in str(e))
             return
         self.fail("rules.get() was supposed to throw a RulesGetFailedException")
 
@@ -245,13 +261,15 @@ class RulesTestCase(unittest.TestCase):
     @mock.patch('requests.post', good_delete)
     def test_delete_rules_single(self):
         """ Delete one rule. """
-        rules.delete_rule({"value": "Hello World"}, config_file_path=test_utils.test_config_path)
+        rules.delete_rule({"value": "Hello World"},
+                          config_file_path=test_utils.test_config_path)
 
     @mock.patch('requests.post', good_delete)
     def test_delete_rules_multiple(self):
         """ Delete multiple rules. """
         rules_list = [
-            { "value": "Hello World" },
-            { "value": "Hello", "tag": "mytag" }
+            {"value": "Hello World"},
+            {"value": "Hello", "tag": "mytag"}
         ]
-        rules.delete_rules(rules_list, config_file_path=test_utils.test_config_path)
\ No newline at end of file
+        rules.delete_rules(rules_list,
+                           config_file_path=test_utils.test_config_path)