Merge lp:~aaronp/software-center/modify-reviews into lp:software-center

[Aaron Peachey]

implements edit and delete review functionality

production rnr server currently does not support the edit functionality (achuni advised ~3 weeks before next release) but the changes to USC are designed to fail gracefully on errors from the server

=== added symlink 'data/delete_review.py'
=== target is u'../utils/delete_review.py'
=== added symlink 'data/modify_review.py'
=== target is u'../utils/modify_review.py'
=== modified file 'setup.py'
--- setup.py	2011-05-31 21:12:53 +0000
+++ setup.py	2011-06-07 12:26:16 +0000
@@ -79,6 +79,8 @@
                "utils/submit_review.py",
                "utils/report_review.py",
                "utils/submit_usefulness.py",
+               "utils/delete_review.py",
+               "utils/modify_review.py",
                "utils/get_reviews_helper.py",
                "utils/get_review_stats_helper.py",
                "utils/get_useful_votes_helper.py",

=== modified file 'softwarecenter/backend/reviews.py'
--- softwarecenter/backend/reviews.py	2011-06-01 08:04:32 +0000
+++ softwarecenter/backend/reviews.py	2011-06-07 12:26:16 +0000
@@ -47,6 +47,8 @@
                                   SUBMIT_REVIEW_APP,
                                   REPORT_REVIEW_APP,
                                   SUBMIT_USEFULNESS_APP,
+                                  DELETE_REVIEW_APP,
+                                  MODIFY_REVIEW_APP,
                                   GET_REVIEWS_HELPER,
                                   GET_REVIEW_STATS_HELPER,
                                   GET_USEFUL_VOTES_HELPER,
@@ -184,6 +186,8 @@
         self.usefulness_favorable = 0
         # this will be set if tryint to submit usefulness for this review failed
         self.usefulness_submit_error = False
+        self.delete_error = False
+        self.modify_error = False
     def __repr__(self):
         return "[Review id=%s review_text='%s' reviewer_username='%s']" % (
             self.id, self.review_text, self.reviewer_username)
@@ -328,6 +332,29 @@
         (pid, stdin, stdout, stderr) = glib.spawn_async(
             cmd, flags=glib.SPAWN_DO_NOT_REAP_CHILD, standard_output=True)
         glib.child_watch_add(pid, self._on_submit_usefulness_finished, (review_id, is_useful, stdout, callback))
+    
+    def spawn_delete_review_ui(self, review_id, parent_xid, datadir, callback):
+        cmd = [os.path.join(datadir, DELETE_REVIEW_APP), 
+               "--review-id", "%s" % review_id,
+               "--parent-xid", "%s" % parent_xid,
+               "--datadir", datadir,
+              ]
+        (pid, stdin, stdout, stderr) = glib.spawn_async(
+            cmd, flags=glib.SPAWN_DO_NOT_REAP_CHILD, standard_output=True)
+        glib.child_watch_add(pid, self._on_delete_review_finished, (review_id, callback))
+    
+    def spawn_modify_review_ui(self, parent_xid, iconname, datadir, review_id, callback):
+        """ this spawns the UI for writing a new review and
+            adds it automatically to the reviews DB """
+        cmd = [os.path.join(datadir, MODIFY_REVIEW_APP), 
+               "--parent-xid", "%s" % parent_xid,
+               "--iconname", iconname,
+               "--datadir", "%s" % datadir,
+               "--review-id", "%s" % review_id,
+               ]
+        (pid, stdin, stdout, stderr) = glib.spawn_async(
+            cmd, flags=glib.SPAWN_DO_NOT_REAP_CHILD, standard_output=True)
+        glib.child_watch_add(pid, self._on_modify_review_finished, (review_id, stdout, callback))
 
     # internal callbacks/helpers
     def _on_submit_review_finished(self, pid, status, (app, stdout_fd, callback)):
@@ -369,6 +396,64 @@
                         callback(app, self._reviews[app])
                         break
 
+    def _on_delete_review_finished(self, pid, status, (review_id, callback)):
+        """ called when delete_review finished"""
+        exitcode = os.WEXITSTATUS(status)
+        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
+
+    def _on_modify_review_finished(self, pid, status, (review_id, stdout_fd, callback)):
+        """called when modify_review finished"""
+        exitcode = os.WEXITSTATUS(status)
+        LOG.debug("_on_modify_review_finished")
+        stdout = ""
+        while True:
+            s = os.read(stdout_fd, 1024)
+            if not s: break
+            stdout += s
+        LOG.debug("stdout from modify_review: '%s'" % stdout)
+        if exitcode == 0:
+            try:
+                review_json = simplejson.loads(stdout)
+            except simplejson.decoder.JSONDecodeError:
+                LOG.error("failed to parse '%s'" % stdout)
+                return
+            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
+
     def _on_submit_usefulness_finished(self, pid, status, (review_id, is_useful, stdout_fd, callback)):
         """ called when report_usefulness finished """
         exitcode = os.WEXITSTATUS(status)
@@ -421,6 +506,7 @@
         self.rnrclient = RatingsAndReviewsAPI(cachedir=cachedir)
         self._reviews = {}
 
+
     def _update_rnrclient_offline_state(self):
         # this needs the lp:~mvo/piston-mini-client/offline-mode branch
         self.rnrclient._offline_mode = not network_state_is_connected()
@@ -506,6 +592,7 @@
     # stats
     def refresh_review_stats(self, callback):
         """ public api, refresh the available statistics """
+ 
         try:
             mtime = os.path.getmtime(self.REVIEW_STATS_CACHE_FILE)
             days_delta = int((time.time() - mtime) // (24*60*60))

=== modified file 'softwarecenter/backend/rnrclient_pristine.py'
--- softwarecenter/backend/rnrclient_pristine.py	2011-04-11 14:09:54 +0000
+++ softwarecenter/backend/rnrclient_pristine.py	2011-06-07 12:26:16 +0000
@@ -167,3 +167,21 @@
 
         return self._get('usefulness/', args=data,
             scheme=PUBLIC_API_SCHEME)
+
+    @validate('review_id', int)
+    @returns_json
+    def delete_review(self, review_id):
+        """Delete a review"""
+        return self._post('/reviews/delete/%s/' % review_id, data={},
+            scheme=AUTHENTICATED_API_SCHEME)
+
+    @validate('review_id', int)
+    @validate('rating', int)
+    @validate_pattern('summary', r'[^\n]+')
+    @validate_pattern('review_text', r'[^\n]+')
+    @returns(ReviewDetails)
+    def modify_review(self, review_id, rating, summary, review_text):
+        """Modify an existing review"""
+        data = {'rating':rating, 'summary':summary, 'review_text':review_text}
+        return self._put('/reviews/modify/%s/' % review_id, data=data,
+            scheme=AUTHENTICATED_API_SCHEME)

=== modified file 'softwarecenter/paths.py'
--- softwarecenter/paths.py	2011-05-31 07:26:24 +0000
+++ softwarecenter/paths.py	2011-06-07 12:26:16 +0000
@@ -58,6 +58,8 @@
 SUBMIT_REVIEW_APP = "submit_review.py"
 REPORT_REVIEW_APP = "report_review.py"
 SUBMIT_USEFULNESS_APP = "submit_usefulness.py"
+MODIFY_REVIEW_APP = "modify_review.py"
+DELETE_REVIEW_APP = "delete_review.py"
 GET_REVIEWS_HELPER = "get_reviews_helper.py"
 GET_REVIEW_STATS_HELPER = "get_review_stats_helper.py"
 GET_USEFUL_VOTES_HELPER = "get_useful_votes_helper.py"

=== modified file 'softwarecenter/ui/gtk/appdetailsview.py'
--- softwarecenter/ui/gtk/appdetailsview.py	2011-06-05 22:50:04 +0000
+++ softwarecenter/ui/gtk/appdetailsview.py	2011-06-07 12:26:16 +0000
@@ -131,6 +131,17 @@
             review_id, is_useful, parent_xid, self.datadir,
             self._reviews_ready_callback)
 
+    def _review_modify(self, review_id):
+        parent_xid = get_parent_xid(self)
+        self.review_loader.spawn_modify_review_ui(
+            parent_xid, self.appdetails.icon, self.datadir, review_id,
+            self._reviews_ready_callback)
+
+    def _review_delete(self, review_id):
+        parent_xid = get_parent_xid(self)
+        self.review_loader.spawn_delete_review_ui(
+            review_id, parent_xid, self.datadir, self._reviews_ready_callback)
+
     # public interface
     def reload(self):
         """ reload the package cache, this goes straight to the backend """

=== modified file 'softwarecenter/ui/gtk/appdetailsview_gtk.py'
--- softwarecenter/ui/gtk/appdetailsview_gtk.py	2011-06-05 22:50:04 +0000
+++ softwarecenter/ui/gtk/appdetailsview_gtk.py	2011-06-07 12:26:16 +0000
@@ -850,6 +850,7 @@
         if my_votes:
             self.reviews.update_useful_votes(my_votes)
         
+        self.reviews.clear()
         for review in reviews_data:
             self.reviews.add_review(review)
         self.reviews.configure_reviews_ui()
@@ -1150,6 +1151,8 @@
         self.reviews.connect("new-review", self._on_review_new)
         self.reviews.connect("report-abuse", self._on_review_report_abuse)
         self.reviews.connect("submit-usefulness", self._on_review_submit_usefulness)
+        self.reviews.connect("modify-review", self._on_review_modify)
+        self.reviews.connect("delete-review", self._on_review_delete)
         self.reviews.connect("more-reviews-clicked", self._on_more_reviews_clicked)
         self.reviews.connect("different-review-language-clicked", self._on_reviews_in_different_language_clicked)
         vb.pack_start(self.reviews, False)
@@ -1197,7 +1200,13 @@
         return False
 
     def _on_review_new(self, button):
-        self._review_write_new()
+            self._review_write_new()
+    
+    def _on_review_modify(self, button, review_id):
+        self._review_modify(review_id)
+    
+    def _on_review_delete(self, button, review_id):
+        self._review_delete(review_id)
 
     def _on_review_report_abuse(self, button, review_id):
         self._review_report_abuse(str(review_id))

=== modified file 'softwarecenter/ui/gtk/widgets/reviews.py'
--- softwarecenter/ui/gtk/widgets/reviews.py	2011-06-05 22:50:04 +0000
+++ softwarecenter/ui/gtk/widgets/reviews.py	2011-06-07 12:26:16 +0000
@@ -714,6 +714,12 @@
         'submit-usefulness':(gobject.SIGNAL_RUN_FIRST,
                     gobject.TYPE_NONE,
                     (gobject.TYPE_PYOBJECT, bool)),
+        'modify-review':(gobject.SIGNAL_RUN_FIRST,
+                    gobject.TYPE_NONE,
+                    (gobject.TYPE_PYOBJECT,)),
+        'delete-review':(gobject.SIGNAL_RUN_FIRST,
+                    gobject.TYPE_NONE,
+                    (gobject.TYPE_PYOBJECT,)),
         'more-reviews-clicked':(gobject.SIGNAL_RUN_FIRST,
                                 gobject.TYPE_NONE,
                                 () ),
@@ -957,14 +963,24 @@
         self.yes_like = None
         self.no_like = None
         self.status_box = gtk.HBox()
+        self.delete_status_box = gtk.HBox()
+        self.delete_error_img = gtk.Image()
+        self.delete_error_img.set_from_stock(gtk.STOCK_DIALOG_ERROR, gtk.ICON_SIZE_SMALL_TOOLBAR)        
         self.submit_error_img = gtk.Image()
         self.submit_error_img.set_from_stock(gtk.STOCK_DIALOG_ERROR, gtk.ICON_SIZE_SMALL_TOOLBAR)
         self.submit_status_spinner = gtk.Spinner()
         self.submit_status_spinner.set_size_request(12,12)
+        self.delete_status_spinner = gtk.Spinner()
+        self.delete_status_spinner.set_size_request(12,12)
         self.acknowledge_error = mkit.VLinkButton(_("<small>OK</small>"))
         self.acknowledge_error.set_underline(True)
         self.acknowledge_error.set_subdued(True)
+        self.delete_acknowledge_error = mkit.VLinkButton(_("<small>OK</small>"))
+        self.delete_acknowledge_error.set_underline(True)
+        self.delete_acknowledge_error.set_subdued(True)
         self.usefulness_error = False
+        self.delete_error = False
+        self.modify_error = False
 
         self.pack_start(self.header, False)
         self.pack_start(self.body, False)
@@ -1011,6 +1027,11 @@
         if reviews:
             reviews.emit("report-abuse", self.id)
     
+    def _on_modify_clicked(self, button):
+        reviews = self.get_ancestor(UIReviewsList)
+        if reviews:
+            reviews.emit("modify-review", self.id)
+    
     def _on_useful_clicked(self, btn, is_useful):
         reviews = self.get_ancestor(UIReviewsList)
         if reviews:
@@ -1066,6 +1087,56 @@
         # example raw_date str format: 2011-01-28 19:15:21
         return datetime.datetime.strptime(raw_date_str, '%Y-%m-%d %H:%M:%S')
 
+    def _delete_ui_update(self, type, current_user_reviewer=False, action=None):
+        self._hide_delete_elements()
+        if type == 'renew':
+            self._build_delete_flag_ui(current_user_reviewer)
+            return
+        if type == 'progress':
+            self.delete_status_spinner.start()
+            self.delete_status_spinner.show()
+            self.delete_status_label = gtk.Label("<small><b>%s</b></small>" % _(u"Deleting now\u2026"))
+            self.delete_status_box.pack_start(self.delete_status_spinner, False)
+            self.delete_status_label.set_use_markup(True)
+            self.delete_status_label.set_padding(2,0)
+            self.delete_status_box.pack_start(self.delete_status_label,False)
+            self.delete_status_label.show()
+        if type == 'error':
+            self.delete_error_img.show()
+            self.delete_status_label = gtk.Label("<small><b>%s</b></small>" % _("Error %s review" % action))
+            self.delete_status_box.pack_start(self.delete_error_img, False)
+            self.delete_status_label.set_use_markup(True)
+            self.delete_status_label.set_padding(2,0)
+            self.delete_status_box.pack_start(self.delete_status_label,False)
+            self.delete_status_label.show()
+            self.delete_acknowledge_error.show()
+            self.delete_status_box.pack_start(self.delete_acknowledge_error,False)
+            self.delete_acknowledge_error.connect('clicked', self._on_delete_error_acknowledged, current_user_reviewer)
+        self.delete_status_box.show()
+        self.footer.pack_end(self.delete_status_box, False)
+        return
+    
+    def _on_delete_clicked(self, btn):
+        reviews = self.get_ancestor(UIReviewsList)
+        if reviews:
+            self._delete_ui_update('progress')
+            reviews.emit("delete-review", self.id)
+    
+    def _on_delete_error_acknowledged(self, button, current_user_reviewer):
+        self.delete_error = False
+        self._delete_ui_update('renew', current_user_reviewer)
+        
+    def _hide_delete_elements(self):
+        """ hide all delete elements """
+        for attr in ["complain", "edit", "delete", "delete_status_spinner",
+                     "delete_error_img", "delete_status_box", "delete_status_label",
+                     "delete_acknowledge_error", "flagbox"
+                     ]:
+            o = getattr(self, attr, None)
+            if o:
+                o.hide()
+        return
+    
     def _build(self, review_data, app_version, logged_in_person, useful_votes):
 
         # all the attributes of review_data may need markup escape, 
@@ -1081,6 +1152,8 @@
         self.useful_total = useful_total = review_data.usefulness_total
         useful_favorable = review_data.usefulness_favorable
         useful_submit_error = review_data.usefulness_submit_error
+        delete_error = review_data.delete_error
+        modify_error = review_data.modify_error
 
         dark_color = self.style.dark[gtk.STATE_NORMAL]
         m = self._whom_when_markup(self.person, displayname, cur_t, dark_color)
@@ -1133,19 +1206,12 @@
 
         self._build_usefulness_ui(current_user_reviewer, useful_total,
                                   useful_favorable, useful_votes, useful_submit_error)
-
-        # Translators: This link is for flagging a review as inappropriate.
-        # To minimize repetition, if at all possible, keep it to a single word.
-        # If your language has an obvious verb, it won't need a question mark.
-        self.complain = mkit.VLinkButton('<small>%s</small>' % _('Inappropriate?'))
-        self.complain.set_subdued(True)
-        self.complain.set_underline(True)
-        self.footer.pack_end(self.complain, False)
-        self.complain.connect('clicked', self._on_report_abuse_clicked)
-        # FIXME: dynamically update this on network changes
-        self.complain.set_sensitive(network_state_is_connected())
-        self.body.connect('size-allocate', self._on_allocate, stars, 
-                          summary, text, who_when, version_lbl, self.complain)
+            
+        self.flagbox = gtk.HBox()
+        self._build_delete_flag_ui(current_user_reviewer, delete_error, modify_error)
+        self.footer.pack_end(self.flagbox,False)
+        self.body.connect('size-allocate', self._on_allocate, stars, summary, text, who_when, version_lbl, self.flagbox)
+            
         return
     
     def _build_usefulness_ui(self, current_user_reviewer, useful_total, 
@@ -1264,6 +1330,36 @@
         
         return gtk.Label('<small>%s</small>' % s)
 
+    def _build_delete_flag_ui(self, current_user_reviewer, delete_error=False, modify_error=False):
+        if delete_error:
+            self._delete_ui_update('error', current_user_reviewer, 'deleting')
+        elif modify_error:
+            self._delete_ui_update('error', current_user_reviewer, 'modifying')
+        else:
+            if current_user_reviewer:
+                self.edit = mkit.VLinkButton('<small>%s</small>' %_('Edit'))
+                self.delete = mkit.VLinkButton('<small>%s</small>' %_('Delete'))
+                self.edit.set_underline(True)
+                self.delete.set_underline(True)
+                self.edit.set_subdued(True)
+                self.delete.set_subdued(True)
+                self.flagbox.pack_start(self.edit, False)
+                self.flagbox.pack_start(self.delete, False)
+                self.edit.connect('clicked', self._on_modify_clicked)
+                self.delete.connect('clicked', self._on_delete_clicked)
+            else:
+                # Translators: This link is for flagging a review as inappropriate.
+                # To minimize repetition, if at all possible, keep it to a single word.
+                # If your language has an obvious verb, it won't need a question mark.
+                self.complain = mkit.VLinkButton('<small>%s</small>' % _('Inappropriate?'))
+                self.complain.set_subdued(True)
+                self.complain.set_underline(True)
+                self.complain.set_sensitive(network_state_is_connected())
+                self.flagbox.pack_start(self.complain, False)
+                self.complain.connect('clicked', self._on_report_abuse_clicked)
+            self.flagbox.show_all()
+            return
+
     def _whom_when_markup(self, person, displayname, cur_t, dark_color):
         nice_date = get_nice_date_string(cur_t)
         #dt = datetime.datetime.utcnow() - cur_t

=== added symlink 'utils/delete_review.py'
=== target is u'submit_review.py'
=== added symlink 'utils/modify_review.py'
=== target is u'submit_review.py'
=== modified file 'utils/submit_review.py'
--- utils/submit_review.py	2011-06-06 08:49:12 +0000
+++ utils/submit_review.py	2011-06-07 12:26:16 +0000
@@ -69,6 +69,7 @@
 distro = get_distro()
 SERVER_ROOT=distro.REVIEWS_SERVER
 
+
 # server status URL
 SERVER_STATUS_URL = SERVER_ROOT+"/server-status/"
 
@@ -117,6 +118,12 @@
     def submit_usefulness(self, review_id, is_useful):
         self.emit("transmit-start", review_id)
         self.worker_thread.pending_usefulness.put((int(review_id), is_useful))
+    def modify_review(self, review_id, review):
+        self.emit("transmit-start", review_id)
+        self.worker_thread.pending_modify.put((int(review_id), review))
+    def delete_review(self, review_id):
+        self.emit("transmit-start", review_id)
+        self.worker_thread.pending_delete.put(int(review_id))
     def server_status(self):
         self.worker_thread.pending_server_status()
     def shutdown(self):
@@ -140,6 +147,8 @@
         self.pending_reviews = Queue()
         self.pending_reports = Queue()
         self.pending_usefulness = Queue()
+        self.pending_modify = Queue()
+        self.pending_delete = Queue()
         self.pending_server_status = Queue()
         self._shutdown = False
         # FIXME: instead of a binary value we need the state associated
@@ -176,11 +185,15 @@
             self._submit_reviews_if_pending()
             self._submit_reports_if_pending()
             self._submit_usefulness_if_pending()
+            self._submit_modify_if_pending()
+            self._submit_delete_if_pending()
             time.sleep(0.2)
             if (self._shutdown and
                 self.pending_reviews.empty() and
                 self.pending_usefulness.empty() and
-                self.pending_reports.empty()):
+                self.pending_reports.empty() and
+                self.pending_modify.empty() and
+                self.pending_delete.empty()):
                 return
 
     # usefulness
@@ -207,6 +220,59 @@
                 self._write_exception_html_log_if_needed(e)
                 self._transmit_state = TRANSMIT_STATE_ERROR
             self.pending_usefulness.task_done()
+    
+    #modify
+    def queue_modify(self, modification):
+        """ queue a new review modification request for sending to LP """
+        logging.debug("queue_modify %s %s" % modification)
+        self.pending_modify.put(modification)
+
+    def _submit_modify_if_pending(self):
+        """ the actual modify function """
+        while not self.pending_modify.empty():
+            logging.debug("_modify_review")
+            self._transmit_state = TRANSMIT_STATE_INPROGRESS
+            (review_id, review) = self.pending_modify.get()
+            summary = review['summary']
+            review_text = review['review_text']
+            rating = review['rating']
+            try:
+                res = self.rnrclient.modify_review(review_id=review_id, 
+                                                   summary=summary,
+                                                   review_text=review_text,
+                                                   rating=rating)
+                self._transmit_state = TRANSMIT_STATE_DONE
+                sys.stdout.write(simplejson.dumps(vars(res)))
+            except Exception as e:
+                logging.exception("modify_review")
+                err_str = self._get_error_messages(e)
+                self._write_exception_html_log_if_needed(e)
+                self._transmit_state = TRANSMIT_STATE_ERROR
+                self._transmit_error_str = err_str
+            self.pending_modify.task_done() 
+                       
+    #delete
+    def queue_delete(self, deletion):
+        """ queue a new deletion request for sending to LP """
+        logging.debug("queue_delete review id: %s" % deletion)
+        self.pending_delete.put(deletion)
+    
+    def _submit_delete_if_pending(self):
+        """ the actual deletion """
+        while not self.pending_delete.empty():
+            logging.debug("POST delete")
+            self._transmit_state = TRANSMIT_STATE_INPROGRESS
+            review_id = self.pending_delete.get()
+            try:
+                res = self.rnrclient.delete_review(review_id=review_id)
+                self._transmit_state = TRANSMIT_STATE_DONE
+                sys.stdout.write(simplejson.dumps(res))
+            except Exception as e:
+                logging.exception("delete_review failed")
+                self._write_exception_html_log_if_needed(e)
+                self._transmit_error_str = _("Failed to delete review")
+                self._transmit_state = TRANSMIT_STATE_ERROR
+            self.pending_delete.task_done()
 
     # reports
     def queue_report(self, report):
@@ -350,6 +416,9 @@
         #submit success image
         self.submit_success_img = gtk.Image()
         self.submit_success_img.set_from_stock(gtk.STOCK_APPLY,  gtk.ICON_SIZE_SMALL_TOOLBAR)
+        #submit warn image
+        self.submit_warn_img = gtk.Image()
+        self.submit_warn_img.set_from_stock(gtk.STOCK_DIALOG_INFO,  gtk.ICON_SIZE_SMALL_TOOLBAR)
         #label size to prevent image or spinner from resizing
         self.label_transmit_status.set_size_request(-1, gtk.icon_size_lookup(gtk.ICON_SIZE_SMALL_TOOLBAR)[1])
 
@@ -472,6 +541,8 @@
         """method to separate the updating of status icon/spinner and message in the submit review window,
          takes a type (progress, fail, success) as a string and a message string then updates status area accordingly"""
         self._clear_status_imagery()
+        self.label_transmit_status.set_text("")
+        
         if type == "progress":
             self.status_hbox.pack_start(self.submit_spinner, False)
             self.status_hbox.reorder_child(self.submit_spinner, 0)
@@ -490,6 +561,11 @@
             self.status_hbox.reorder_child(self.submit_success_img, 0)
             self.submit_success_img.show()
             self.label_transmit_status.set_text(message)
+        elif type == "warning":
+            self.status_hbox.pack_start(self.submit_warn_img, False)
+            self.status_hbox.reorder_child(self.submit_warn_img, 0)
+            self.submit_warn_img.show()
+            self.label_transmit_status.set_text(message)
 
     def _clear_status_imagery(self):
         self.detail_expander.hide()
@@ -514,13 +590,18 @@
         except TypeError:
             pass
         
+        try: 
+            self.status_hbox.query_child_packing(self.submit_warn_img)
+            self.status_hbox.remove(self.submit_warn_img)
+        except TypeError:
+            pass
+        
         return
-        
+
 
 class SubmitReviewsApp(BaseApp):
     """ review a given application or package """
 
-
     STAR_SIZE = (32, 32)
     APP_ICON_SIZE = 48
     #character limits for text boxes and hurdles for indicator changes 
@@ -532,8 +613,9 @@
     ERROR_COLOUR = "FF0000"
     SUBMIT_MESSAGE = _("Submitting Review")
     FAILURE_MESSAGE = _("Failed to submit review")
+    SUCCESS_MESSAGE = _("Review submitted")
 
-    def __init__(self, app, version, iconname, origin, parent_xid, datadir):
+    def __init__(self, app, version, iconname, origin, parent_xid, datadir, action="submit", review_id=0):
         BaseApp.__init__(self, datadir, "submit_review.ui")
         self.datadir = datadir
         # legal fineprint, do not change without consulting a lawyer
@@ -546,20 +628,6 @@
         self.submit_window.connect("destroy", self.on_button_cancel_clicked)
         self._add_spellcheck_to_textview(self.textview_review)
 
-        # gwibber stuff
-        self.gwibber_combo = gtk.combo_box_new_text()
-        #cells = self.gwibber_combo.get_cells()
-        #cells[0].set_property("ellipsize", pango.ELLIPSIZE_END)
-        self.gwibber_hbox.pack_start(self.gwibber_combo, True)
-        if "SOFTWARE_CENTER_GWIBBER_MOCK_USERS" in os.environ:
-            self.gwibber_helper = GwibberHelperMock()
-        else:
-            self.gwibber_helper = GwibberHelper()
-        
-        #get a dict with a saved gwibber_send (boolean) and gwibber account_id for persistent state
-        self.gwibber_prefs = self._get_gwibber_prefs()
-
-        # interactive star rating
         self.star_rating = StarRatingSelector(0, star_size=self.STAR_SIZE)
         self.star_caption = StarCaption()
 
@@ -573,33 +641,85 @@
         self.rating_hbox.reorder_child(self.star_caption, 1)
 
         self.review_buffer = self.textview_review.get_buffer()
-        
+
         self.detail_expander.hide()
+        
+        self.retrieve_api = RatingsAndReviewsAPI()
 
+        
         # data
         self.app = app
         self.version = version
         self.origin = origin
         self.iconname = iconname
+        self.action = action
+        self.review_id = int(review_id)
         
-        # title
-        self.submit_window.set_title(_("Review %s") % self.app.name)
+        # parent xid
+        if parent_xid:
+            win = gtk.gdk.window_foreign_new(int(parent_xid))
+            if win:
+                self.submit_window.realize()
+                self.submit_window.window.set_transient_for(win)
 
+        self.submit_window.set_position(gtk.WIN_POS_MOUSE)
+        
         self.review_summary_entry.connect('changed', self._on_mandatory_text_entry_changed)
         self.star_rating.connect('changed', self._on_mandatory_fields_changed)
         self.review_buffer.connect('changed', self._on_text_entry_changed)
         
         # gwibber stuff
+        self.gwibber_combo = gtk.combo_box_new_text()
+        #cells = self.gwibber_combo.get_cells()
+        #cells[0].set_property("ellipsize", pango.ELLIPSIZE_END)
+        self.gwibber_hbox.pack_start(self.gwibber_combo, True)
+        if "SOFTWARE_CENTER_GWIBBER_MOCK_USERS" in os.environ:
+            self.gwibber_helper = GwibberHelperMock()
+        else:
+            self.gwibber_helper = GwibberHelper()
+        
+        #get a dict with a saved gwibber_send (boolean) and gwibber account_id for persistent state
+        self.gwibber_prefs = self._get_gwibber_prefs()
+        
+        # gwibber stuff
         self._setup_gwibber_gui()
 
-        # parent xid
-        if parent_xid:
-            win = gtk.gdk.window_foreign_new(int(parent_xid))
-            if win:
-                self.submit_window.realize()
-                self.submit_window.window.set_transient_for(win)
+        #now setup rest of app based on whether submit or modify
+        if self.action == "submit":
+            self._init_submit()
+        elif self.action == "modify":
+            self._init_modify()
 
-        self.submit_window.set_position(gtk.WIN_POS_MOUSE)
+    def _init_submit(self):
+        self.submit_window.set_title(_("Review %s" % self.app.name))
+    
+    def _init_modify(self):
+        self._populate_review()
+        self.submit_window.set_title(_("Modify Your %s Review" % self.app.name))
+        self.button_post.set_label(_("Modify"))
+        self.SUBMIT_MESSAGE = _("Updating your review")
+        self.FAILURE_MESSAGE = _("Failed to edit review")
+        self.SUCCESS_MESSAGE = _("Review updated")
+        self._enable_or_disable_post_button()
+    
+    def _populate_review(self):
+        try:
+            review_data = self.retrieve_api.get_review(review_id=self.review_id)
+            app = Application(pkgname=review_data.package_name)
+            self.app = app
+            self.review_summary_entry.set_text(review_data.summary)
+            self.star_rating.set_rating(review_data.rating)
+            self.review_buffer.set_text(review_data.review_text)
+            #save original review field data, for comparison purposes when user makes changes to fields
+            self.orig_summary_text = review_data.summary
+            self.orig_star_rating = review_data.rating
+            self.orig_review_text = review_data.review_text
+            self.version = review_data.version
+            self.origin = review_data.origin
+            return  
+        except piston_mini_client.APIError:
+            logging.warn('Unable to retrieve review id %s for editing. Exiting' % self.review_id)
+            self.quit(2)
 
     def _setup_details(self, widget, app, iconname, version, display_name):
         # icon shazam
@@ -654,8 +774,41 @@
             review_chars and review_chars <= self.REVIEW_CHAR_LIMITS[0] and
             self.star_rating.get_rating()):
             self.button_post.set_sensitive(True)
+            self._change_status("clear", "")
         else:
             self.button_post.set_sensitive(False)
+            self._change_status("clear", "")
+        
+        #set post button insensitive, if review being modified is the same as what is currently in the UI fields
+        #checks if 'original' review attributes exist to avoid exceptions when this method has been called prior to review being retrieved  
+        if self.action == 'modify' and hasattr(self, "orig_star_rating"):
+            if self._modify_review_is_the_same():
+                self.button_post.set_sensitive(False)
+                self._change_status("warning", _("Can't submit unmodified"))
+            else:
+                self._change_status("clear", "")
+    
+    def _modify_review_is_the_same(self):
+        '''checks if review fields are the same as the review being modified and returns true if so'''
+        
+        #perform an initial check on character counts to return False if any don't match, avoids doing unnecessary string comparisons
+        if (self.review_summary_entry.get_text_length() != len(self.orig_summary_text) or
+           self.review_buffer.get_char_count() != len(self.orig_review_text)):
+            return False
+        #compare rating
+        if self.star_rating.get_rating() != self.orig_star_rating:
+            return False
+        #compare summary text
+        if self.review_summary_entry.get_text() != self.orig_summary_text:
+            return False
+        #compare review text
+        if self.review_buffer.get_text(self.review_buffer.get_start_iter(),
+                                           self.review_buffer.get_end_iter()) != self.orig_review_text:
+            return False
+        return True
+        
+            
+            
     
     def _check_summary_character_count(self):
         summary_chars = self.review_summary_entry.get_text_length()
@@ -737,7 +890,14 @@
         review.rating = self.star_rating.get_rating()
         review.package_version = self.version
         review.origin = self.origin
-        self.api.submit_review(review)
+        
+        if self.action == "submit":
+            self.api.submit_review(review)
+        elif self.action == "modify":
+            changes = {'review_text':review.text,
+                       'summary':review.summary,
+                       'rating':review.rating}
+            self.api.modify_review(self.review_id, changes)
 
     def login_successful(self, display_name):
         self.main_notebook.set_current_page(1)
@@ -887,7 +1047,7 @@
     
     def _success_status(self):
         """Updates status area to show success for 2 seconds then allows window to proceed"""
-        self._change_status("success", _("Review submitted."))
+        self._change_status("success", self.SUCCESS_MESSAGE)
         while gtk.events_pending():
             gtk.main_iteration(False)
         time.sleep(2)
@@ -995,7 +1155,6 @@
         self.textview_report.get_buffer().connect(
             "changed", self._enable_or_disable_report_button)
 
-
         # data
         self.review_id = review_id
 
@@ -1097,27 +1256,67 @@
         self.api.submit_usefulness(self.review_id, self.is_useful)
     
     def on_transmit_failure(self, api, trans, error):
-        print "exiting - error: %s" % error
-        self.api.shutdown()
-        self.quit(2)
-
-    # override parents run to only trigger login (and subsequent
-    # events) but no UI, if this is commented out, there is some
-    # stub ui that can be useful for testing
-    def run(self):
-        self.login()
-    
-    # override UI update methods from BaseApp to prevent them 
-    # causing errors if called when UI is hidden
-    def _clear_status_imagery(self):
-        pass
-    
-    def _change_status(self, type, message):
-        pass
-    
+        logging.warn("exiting - error: %s" % error)
+        self.api.shutdown()
+        self.quit(2)
+
+    # override parents run to only trigger login (and subsequent
+    # events) but no UI, if this is commented out, there is some
+    # stub ui that can be useful for testing
+    def run(self):
+        self.login()
+        
+    # override UI update methods from BaseApp to prevent them 
+    # causing errors if called when UI is hidden
+    def _clear_status_imagery(self):
+        pass
+    
+    def _change_status(self, type, message):
+        pass
+
+class DeleteReviewApp(BaseApp):
+    SUBMIT_MESSAGE = _(u"Deleting review\u2026")
+    FAILURE_MESSAGE = _("Failed to delete review")
+    
+    def __init__(self, review_id, parent_xid, datadir):
+        # uses same UI as submit usefulness because 
+        # (a) it isn't shown and (b) it's similar in usage
+        BaseApp.__init__(self, datadir, "submit_usefulness.ui")
+        # data
+        self.review_id = review_id
+        # no UI except for error conditions
+        self.parent_xid = parent_xid
+
+    # override behavior of baseapp here as we don't actually
+    # have a UI by default
+    def _get_parent_xid_for_login_window(self):
+        return self.parent_xid
+
+    def login_successful(self, display_name):
+        logging.debug("delete review")
+        self.main_notebook.set_current_page(1)
+        self.api.delete_review(self.review_id)    
+    
+    def on_transmit_failure(self, api, trans, error):
+        logging.warn("exiting - error: %s" % error)
+        self.api.shutdown()
+        self.quit(2)
+
+    # override parents run to only trigger login (and subsequent
+    # events) but no UI, if this is commented out, there is some
+    # stub ui that can be useful for testing
+    def run(self):
+        self.login()
+        
+    # override UI update methods from BaseApp to prevent them 
+    # causing errors if called when UI is hidden
+    def _clear_status_imagery(self):
+        pass
+    
+    def _change_status(self, type, message):
+        pass
 
 if __name__ == "__main__":
-
     try:
         locale.setlocale(locale.LC_ALL, "")
     except:
@@ -1219,6 +1418,59 @@
                                          parent_xid=options.parent_xid,
                                          is_useful=int(options.is_useful))
         usefulness_app.run()
-
+    
+    if "delete_review" in sys.argv[0]:
+        #check options
+        parser.add_option("", "--review-id")
+        parser.add_option("", "--parent-xid")
+        parser.add_option("", "--debug",
+                          action="store_true", default=False)
+        (options, args) = parser.parse_args()
+        
+        if not (options.review_id):
+            parser.error(_("Missing review-id argument"))
+    
+        if options.debug:
+            logging.basicConfig(level=logging.DEBUG)
+        
+        logging.debug("delete review mode")
+        
+        delete_app = DeleteReviewApp(datadir=options.datadir,
+                                    review_id=options.review_id,
+                                    parent_xid=options.parent_xid)
+        delete_app.run()
+
+    if "modify_review" in sys.argv[0]:
+            # check options
+        parser.add_option("", "--review-id")
+        parser.add_option("-i", "--iconname")
+        parser.add_option("", "--parent-xid")
+        parser.add_option("", "--debug",
+                          action="store_true", default=False)
+        (options, args) = parser.parse_args()
+
+        if not (options.review_id):
+            parser.error(_("Missing review-id argument"))
+    
+        if options.debug:
+            logging.basicConfig(level=logging.DEBUG)
+
+        # personality
+        logging.debug("modify_review mode")
+
+        # initialize and run
+        modify_app = SubmitReviewsApp(datadir=options.datadir,
+                                      app=None, 
+                                      parent_xid=options.parent_xid,
+                                      iconname=options.iconname,
+                                      origin=None,
+                                      version=None,
+                                    action="modify",
+                                    review_id=options.review_id
+                                    )
+
+        modify_app.run()
+
+        
     # main
     gtk.main()