Merge lp:~adeuring/charmworld/1192544-dont-hide-charms-with-processing-errors into lp:~juju-jitsu/charmworld/trunk

[Abel Deuring]

Do not filter charm with processing errors in search() and find_charms().

This branch removes the filters "don't include charms where charm.error is set" from the functions search() and find_charms(). This should fix bugs 1194554, 1192556, 1192544.

One corner case remains: If an ingest error occurs for a charm that is not yet stored in the MongoDB, it can happen that we now show a charm without having any intersting information about it, like the provided/required interfaces, the description or summary.

[Aaron Bentley (abentley)]

Please change the tests so that instead of testing store errors, they test that processing errors do *not* cause the charms to be hidden. Otherwise, looks fine.

review: Approve

[Abel Deuring (adeuring)]

review: Approve

[Charmworld Lander (charmworld-lander)]

The Jenkins job https://jenkins.qa.ubuntu.com/job/charmworld-merger-trunk/351/console reported an error when processing this lp:~adeuring/charmworld/1192544-dont-hide-charms-with-processing-errors branch.
Not merging it.

[Abel Deuring (adeuring)]

review: Approve

[Charmworld Lander (charmworld-lander)]

There are additional revisions which have not been approved in review. Please seek review and approval of these new revisions.

[Abel Deuring (adeuring)]

review: Approve

=== modified file 'charmworld/models.py'
--- charmworld/models.py	2013-06-26 15:29:55 +0000
+++ charmworld/models.py	2013-06-27 15:43:27 +0000
@@ -1041,7 +1041,6 @@
         if spec is None:
             spec = {}
         spec['store_data.errors'] = {'$exists': False}
-        spec['error'] = {'$exists': False}
     if only_promulgated:
         if spec is None:
             spec = {}

=== modified file 'charmworld/search.py'
--- charmworld/search.py	2013-06-25 13:14:10 +0000
+++ charmworld/search.py	2013-06-27 15:43:27 +0000
@@ -94,9 +94,6 @@
 
     valid_charms_clauses = [
         {'missing': {'field': 'store_data.errors'}},
-        # 'missing' ignores objects/dicts, but 'not': {'exists'}
-        # considers them.
-        {'not': {'exists': {'field': 'error'}}}
     ]
 
     def __init__(self, client, index_name):

=== modified file 'charmworld/tests/test_models.py'
--- charmworld/tests/test_models.py	2013-06-26 15:29:55 +0000
+++ charmworld/tests/test_models.py	2013-06-27 15:43:27 +0000
@@ -986,12 +986,12 @@
         }
         self.assertEqual(expected, result[0])
 
-    def test_hide_with_error(self):
+    def test_dont_hide_with_processing_error(self):
+        # Charms with errors that occured during the ingest script
+        # are returned by find_charms()
         charm_id, charm_data = factory.makeCharm(self.db)
-        self.assertIn(charm_data, find_charms(self.db))
         charm_data['error'] = {}
         self.db.charms.save(charm_data)
-        self.assertNotIn(charm_data, find_charms(self.db))
         self.assertIn(charm_data,
                       find_charms(self.db, valid_charms_only=False))
 

=== modified file 'charmworld/tests/test_search.py'
--- charmworld/tests/test_search.py	2013-06-25 13:14:10 +0000
+++ charmworld/tests/test_search.py	2013-06-27 15:43:27 +0000
@@ -414,30 +414,22 @@
         self.assertEqual(set(('good-charm', 'evil-charm')), names)
 
     def test_search_charm_with_error(self):
-        """searches do not return charms with errors by default."""
+        """searches include charms with processing errors."""
         charm = Charm(self.makeCharm())
         result = self.index_client.search(charm.name)
         self.assertEqual(charm, result['results'][0]['data'])
         charm._representation['error'] = {'error': 'error text'}
         self.index_client.index_charm(charm._representation)
         result = self.index_client.search(charm.name)
-        self.assertEqual([], result['results'])
-        result = self.index_client.search(
-            charm.name, valid_charms_only=False)
         self.assertEqual(charm, result['results'][0]['data'])
 
     def test_api_search_charm_with_error(self):
-        """searches do not return charms with errors by default."""
+        """Search results include charms with processing errors."""
         charm = self.makeCharm()
-        result = self.index_client.api_search(charm['name'])
-        self.assertEqual(charm, result[0])
         charm['error'] = {'error': 'error text'}
         self.index_client.index_charm(charm)
         result = self.index_client.api_search(charm['name'])
-        self.assertEqual([], result)
-        result = self.index_client.api_search(
-            charm['name'], valid_charms_only=False)
-        self.assertEqual(charm, result[0])
+        self.assertEqual([charm], result)
 
     def test_search_charm_sort_most_downloaded(self):
         """Specifying 'downloaded' sorts results appropriately."""