Merge lp:~aaronp/software-center/fix-807010 into lp:software-center

[Aaron Peachey]

* softwarecenter/backend/reviews.py,
   softwarecenter/backend/spawn_helper.py:
    - correct modify/delete UI callback behaviour with
      new spawn helper and pagination code (LP: #807010)

Fixes bug lp: #807010 - issue where modified and deleted reviews not replacing in the UI after a successful update by making compatible with recent changes to pagination and spawn_helper.

I believe usefulness votes and report abuse will also not update the UI correctly, which I will work on next but proposing this for merging now since it fixes 807010 without further delay.

[Gary Lasker (gary-lasker)]

Excellent work as always, Aaron, thank you very much!!

review: Approve

=== modified file 'debian/changelog'
--- debian/changelog	2011-07-07 16:04:38 +0000
+++ debian/changelog	2011-07-08 19:13:52 +0000
@@ -9,7 +9,13 @@
     item network state bugs, many thanks to Robert Roth
     (LP: #802919, LP: #802920)
 
- -- Gary Lasker <[e-mail]>  Thu, 07 Jul 2011 12:02:13 -0400
+  [ Aaron Peachey ]
+  * softwarecenter/backend/reviews.py, 
+    softwarecenter/backend/spawn_helper.py:
+    - correct modify/delete UI callback behaviour with
+      new spawn helper and pagination code (LP: #807010)
+
+ -- Aaron Peachey <[e-mail]>  Fri, 08 Jul 2011 23:00:47 +1000
 
 software-center (4.1.8) oneiric; urgency=low
 

=== modified file 'softwarecenter/backend/reviews.py'
--- softwarecenter/backend/reviews.py	2011-07-05 07:09:20 +0000
+++ softwarecenter/backend/reviews.py	2011-07-08 19:13:52 +0000
@@ -469,28 +469,32 @@
         spawn_helper.connect("exited", 
                              self._on_delete_review_finished, 
                              review_id, callback)
+        spawn_helper.connect("error", self._on_delete_review_error,
+                             review_id, callback)
         spawn_helper.run(cmd)
 
-    def _on_delete_review_finished(self, spawn_helper, exitcode, review_id, callback):
+    def _on_delete_review_finished(self, spawn_helper, res, review_id, callback):
         """ called when delete_review finished"""
-        if exitcode == 0:
-            LOG.debug("delete id %s " % review_id)
-            for (app, reviews) in self._reviews.iteritems():
-                for review in reviews:
-                    if str(review.id) == str(review_id):
-                        # remove the one we don't want to see anymore
-                        self._reviews[app].remove(review)
-                        callback(app, self._reviews[app])
-                        break                    
-        else:
-            LOG.debug("delete review id=%s failed with exitcode %s" % (
-                review_id, exitcode))
-            for (app, reviews) in self._reviews.iteritems():
-                for review in reviews:
-                    if str(review.id) == str(review_id):
-                        review.delete_error = exitcode
-                        callback(app, self._reviews[app])
-                        break
+        LOG.debug("delete id %s " % review_id)
+        for (app, reviews) in self._reviews.iteritems():
+            for review in reviews:
+                if str(review.id) == str(review_id):
+                    # remove the one we don't want to see anymore
+                    self._reviews[app].remove(review)
+                    callback(app, self._reviews[app], None, 'remove', review)
+                    break                    
+
+    def _on_delete_review_error(self, spawn_helper, error_str, review_id, callback):
+        """called if delete review errors"""
+        LOG.warn("delete review id=%s failed with error: %s" % (review_id, error_str))
+        for (app, reviews) in self._reviews.iteritems():
+            for review in reviews:
+                if str(review.id) == str(review_id):
+                    review.delete_error = True
+                    callback(app, self._reviews[app], action='replace', 
+                             single_review=review)
+                    break
+
     
     def spawn_modify_review_ui(self, parent_xid, iconname, datadir, review_id, callback):
         """ this spawns the UI for writing a new review and
@@ -502,35 +506,39 @@
                "--review-id", "%s" % review_id,
                ]
         spawn_helper = SpawnHelper(format="json")
-        spawn_helper.connect("exited", 
+        spawn_helper.connect("data-available", 
                              self._on_modify_review_finished, 
                              review_id, callback)
+        spawn_helper.connect("error", self._on_modify_review_error,
+                             review_id, callback)
         spawn_helper.run(cmd)
 
-
-    def _on_modify_review_finished(self, spawn_helper, exitcode, review_id, review_json, callback):
+    def _on_modify_review_finished(self, spawn_helper, review_json, review_id, callback):
         """called when modify_review finished"""
         LOG.debug("_on_modify_review_finished")
-        if exitcode == 0:
-            review_json = spawn_helper._stdout
-            mod_review = ReviewDetails.from_dict(review_json)
-            for (app, reviews) in self._reviews.iteritems():
-                for review in reviews:
-                    if str(review.id) == str(review_id):
-                        # remove the one we don't want to see anymore
-                        self._reviews[app].remove(review)
-                        self._reviews[app].insert(0, Review.from_piston_mini_client(mod_review))
-                        callback(app, self._reviews[app])
-                        break
-        else:
-            LOG.debug("modify review id=%s failed with exitcode %s" % (
-                review_id, exitcode))
-            for (app, reviews) in self._reviews.iteritems():
-                for review in reviews:
-                    if str(review.id) == str(review_id):
-                        review.modify_error = exitcode
-                        callback(app, self._reviews[app])
-                        break
+        #review_json = spawn_helper._stdout
+        mod_review = ReviewDetails.from_dict(review_json)
+        for (app, reviews) in self._reviews.iteritems():
+            for review in reviews:
+                if str(review.id) == str(review_id):
+                    # remove the one we don't want to see anymore
+                    self._reviews[app].remove(review)
+                    new_review = Review.from_piston_mini_client(mod_review)
+                    self._reviews[app].insert(0, new_review)
+                    callback(app, self._reviews[app], action='replace', 
+                             single_review=new_review)
+                    break
+                    
+    def _on_modify_review_error(self, spawn_helper, error_str, review_id, callback):
+        """called if modify review errors"""
+        LOG.debug("modify review id=%s failed with error: %s" % (review_id, error_str))
+        for (app, reviews) in self._reviews.iteritems():
+            for review in reviews:
+                if str(review.id) == str(review_id):
+                    review.modify_error = True
+                    callback(app, self._reviews[app], action='replace', 
+                             single_review=review)
+                    break
 
 
 # this code had several incernations: 

=== modified file 'softwarecenter/backend/spawn_helper.py'
--- softwarecenter/backend/spawn_helper.py	2011-06-21 12:09:26 +0000
+++ softwarecenter/backend/spawn_helper.py	2011-07-08 19:13:52 +0000
@@ -64,16 +64,17 @@
     def _helper_finished(self, pid, status, (stdout, stderr)):
         # get status code
         res = os.WEXITSTATUS(status)
-        if res != 0:
+        if res == 0:
+            self.emit("exited", res)
+        else:
             LOG.warn("exit code %s from helper" % res)
-        # check stderr
-        err = os.read(stderr, 4*1024)
-        self._stderr = err
-        if err:
-            LOG.warn("got error from helper: '%s'" % err)
+            # check stderr
+            err = os.read(stderr, 4*1024)
+            self._stderr = err
+            if err:
+                LOG.warn("got error from helper: '%s'" % err)
             self.emit("error", err)
-        os.close(stderr)
-        self.emit("exited", res)
+            os.close(stderr)
 
     def _helper_io_ready(self, source, condition, (stdout,)):
         # read the raw data