[Sugar-devel] [PATCH] Downgrading activities not allowed. (#2164)

[Aleksey Lim]

On Thu, Oct 07, 2010 at 07:30:36PM +0530, shanjit at seeta.in wrote:
> From: Shanjit Singh Jajmann <shanjit at seeta.in>, Anubhav Aggarwal <anubhav at seeta.in>
> 
> Downgrading an activity is now made possible. When a .xo file of a version older than the currently installed version is clicked, a downgrading option is made available, by popping up of a confirmation alert. Depending upton the choice selected you can downgrade the activity.
> ---
>  src/jarabe/journal/journalactivity.py |   36 +++++++++++++++++++--
>  src/jarabe/journal/listview.py        |    7 +++-
>  src/jarabe/journal/misc.py            |   56 ++++++++++++++++++++++++---------
>  src/jarabe/model/bundleregistry.py    |   18 +++++++---
>  4 files changed, 91 insertions(+), 26 deletions(-)
>  mode change 100644 => 100755 src/jarabe/journal/misc.py
>  mode change 100644 => 100755 src/jarabe/model/bundleregistry.py
> 
> diff --git a/src/jarabe/journal/journalactivity.py b/src/jarabe/journal/journalactivity.py
> index 44cc018..2af55d3 100644
> --- a/src/jarabe/journal/journalactivity.py
> +++ b/src/jarabe/journal/journalactivity.py
> @@ -28,6 +28,7 @@ import os
>  
>  from sugar.graphics.window import Window
>  from sugar.graphics.alert import ErrorAlert
> +from sugar.graphics.alert import ConfirmationAlert
>  
>  from sugar.bundle.bundle import ZipExtractException, RegistrationException
>  from sugar import env
> @@ -128,7 +129,7 @@ class JournalActivity(Window):
>          self.connect('window-state-event', self.__window_state_event_cb)
>          self.connect('key-press-event', self._key_press_event_cb)
>          self.connect('focus-in-event', self._focus_in_event_cb)
> -
> +        
>          model.created.connect(self.__model_created_cb)
>          model.updated.connect(self.__model_updated_cb)
>          model.deleted.connect(self.__model_deleted_cb)
> @@ -136,7 +137,6 @@ class JournalActivity(Window):
>          self._dbus_service = JournalActivityDBusService(self)
>  
>          self.iconify()
> -
>          self._critical_space_alert = None
>          self._check_available_space()
>  
> @@ -145,7 +145,29 @@ class JournalActivity(Window):
>          alert.connect('response', self.__alert_response_cb)
>          self.add_alert(alert)
>          alert.show()
> -
> +        
> +    def __activity_alert1_cb(self):
> +	    if misc.check_previous_install() == 1 and misc.return_checked()==0:
> +             alert1 = ConfirmationAlert()
> +             logging.debug('value of misc is %d', misc.check_previous_install())
> +             alert1.props.title=_('Previous Version Found')
> +             alert1.props.msg = _('A previous version of an installed activity was found. Are you sure you want to continue with installation ?                                                                      If Yes click Ok and the activity icon of the older .xo file in the Journal')
> +             alert1.connect('response', self.__alert1_response_cb)
> +             self.add_alert(alert1)
> +             alert1.show()
> +        
> +    def __alert1_response_cb(self, alert1, response_id):
> +        if response_id is gtk.RESPONSE_OK:
> +            logging.debug('value of checked initial %d', misc.return_checked())
> +            logging.debug('Installing previous version')
> +            self.remove_alert(alert1)
> +            misc.checked = 1
> +            logging.debug('value of checked final %d', misc.return_checked())
> +            
> +        elif response_id is gtk.RESPONSE_CANCEL:
> +            logging.debug('Cancelled by user')
> +            self.remove_alert(alert1)
> +            
>      def __alert_response_cb(self, alert, response_id):
>          self.remove_alert(alert)
>  
> @@ -166,6 +188,8 @@ class JournalActivity(Window):
>          self._list_view = ListView()
>          self._list_view.connect('detail-clicked', self.__detail_clicked_cb)
>          self._list_view.connect('clear-clicked', self.__clear_clicked_cb)
> +        self._list_view.connect('icon-clicked', self.__icon_clicked_cb)
> +        logging.debug('icon clicked in main')
>          self._main_view.pack_start(self._list_view)
>          self._list_view.show()
>  
> @@ -195,7 +219,11 @@ class JournalActivity(Window):
>          keyname = gtk.gdk.keyval_name(event.keyval)
>          if keyname == 'Escape':
>              self.show_main_view()
> -
> +            
> +    def __icon_clicked_cb(self,a):
> +		logging.debug('value of misc is %d', misc.check_previous_install())
> +		self.__activity_alert1_cb()
> +		
>      def __detail_clicked_cb(self, list_view, object_id):
>          self._show_secondary_view(object_id)
>  
> diff --git a/src/jarabe/journal/listview.py b/src/jarabe/journal/listview.py
> index 3d6281a..3dbcc2d 100644
> --- a/src/jarabe/journal/listview.py
> +++ b/src/jarabe/journal/listview.py
> @@ -466,8 +466,12 @@ class ListView(BaseListView):
>      __gsignals__ = {
>          'detail-clicked': (gobject.SIGNAL_RUN_FIRST,
>                             gobject.TYPE_NONE,
> -                           ([object]))
> +                           ([object])),
> +        'icon-clicked': (gobject.SIGNAL_RUN_FIRST,
> +                          gobject.TYPE_NONE,
> +                          ([]))
>      }
> +    
>  
>      def __init__(self):
>          BaseListView.__init__(self)
> @@ -527,6 +531,7 @@ class ListView(BaseListView):
>          row = self.tree_view.get_model()[path]
>          metadata = model.get(row[ListModel.COLUMN_UID])
>          misc.resume(metadata)
> +        self.emit('icon-clicked')
>  
>      def __cell_title_edited_cb(self, cell, path, new_text):
>          row = self._model[path]
> diff --git a/src/jarabe/journal/misc.py b/src/jarabe/journal/misc.py
> old mode 100644
> new mode 100755
> index 32a2847..2b550b9
> --- a/src/jarabe/journal/misc.py
> +++ b/src/jarabe/journal/misc.py
> @@ -31,12 +31,16 @@ from sugar import mime
>  from sugar.bundle.activitybundle import ActivityBundle
>  from sugar.bundle.contentbundle import ContentBundle
>  from sugar import util
> +from sugar.bundle.bundle import AlreadyInstalledException
>  
>  from jarabe.view import launcher
>  from jarabe.model import bundleregistry, shell
>  from jarabe.journal.journalentrybundle import JournalEntryBundle
>  from jarabe.journal import model
>  
> +checker = 0
> +checked = 0
> +
>  def _get_icon_for_mime(mime_type):
>      generic_types = mime.get_all_generic_types()
>      for generic_type in generic_types:
> @@ -148,31 +152,53 @@ def get_activities(metadata):
>  
>      return activities
>  
> +def check_previous_install():
> +    return checker
> +
> +def change_checked(a):
> +    checked = a 
> +    
> +def return_checked():
> +    return checked
> +
>  def resume(metadata, bundle_id=None):
>      registry = bundleregistry.get_registry()
> -
> +    global checker
> +    global checked
>      if is_activity_bundle(metadata) and bundle_id is None:
>  
> -        logging.debug('Creating activity bundle')
> -
> +        logging.debug('Creating activity bundle %d', checker)
> +     
>          file_path = model.get_file(metadata['uid'])
>          bundle = ActivityBundle(file_path)
> -        if not registry.is_installed(bundle):
> +        
> +        if (not registry.is_installed(bundle)) and checked==0 :
>              logging.debug('Installing activity bundle')
> -            registry.install(bundle)
> -        else:
> +            try:    
> +                registry.install(bundle,flag=0)
> +            except AlreadyInstalledException :
> +				checker = 1
> +				
> +        elif (not registry.is_installed(bundle)) and checked==1:
> +                registry.install(bundle,flag=1)
> +                logging.debug('installed older version')
> +                checker = 0
> +                checked = 0
> +                
> +        else :    
>              logging.debug('Upgrading activity bundle')
>              registry.upgrade(bundle)
> -
> -        logging.debug('activityfactory.creating bundle with id %r',
> -                        bundle.get_bundle_id())
> -        installed_bundle = registry.get_bundle(bundle.get_bundle_id())
> -        if installed_bundle:
> -            launch(installed_bundle)
> -        else:
> -            logging.error('Bundle %r is not installed.',
> +            
> +        if checker == 0 : 
> +            logging.debug('activityfactory.creating bundle with id %r',
> +                            bundle.get_bundle_id())
> +            installed_bundle = registry.get_bundle(bundle.get_bundle_id())
> +            if installed_bundle:
> +                launch(installed_bundle)
> +            else:
> +                logging.error('Bundle %r is not installed.',
>                            bundle.get_bundle_id())
> -
> +        
>      elif is_content_bundle(metadata) and bundle_id is None:
>  
>          logging.debug('Creating content bundle')
> diff --git a/src/jarabe/model/bundleregistry.py b/src/jarabe/model/bundleregistry.py
> old mode 100644
> new mode 100755
> index 699e339..28f0ca3

What about less invasive implementation, ie, instead of changing three
files, it could be only popping up an alert in AlreadyInstalledException
try..except block, misc.py is a part of Journal and raising an alert
from it, sounds reasonable.

> --- a/src/jarabe/model/bundleregistry.py
> +++ b/src/jarabe/model/bundleregistry.py
> @@ -53,6 +53,7 @@ class BundleRegistry(gobject.GObject):
>          self._mime_defaults = self._load_mime_defaults()
>  
>          self._bundles = []

> +        self.check_previous_install = 0
* seems remains from previous implementation

>          # hold a reference to the monitors so they don't get disposed
>          self._gio_monitors = []
>  
> @@ -373,14 +374,19 @@ class BundleRegistry(gobject.GObject):
>                  return True
>          return False
>  

> -    def install(self, bundle, uid=None):
> +    def install(self,bundle,flag=0,uid=None):
>          activities_path = env.get_user_activities_path()
* better to put new argument to the end of args list, otherwise existed
  code that relies on old args list could fail
* "flag" sounds too common, something like "force_downgrade" might be
  more appropriate
* boolean sounds more useful for "flag", it allows only two variants

> +        logging.debug('value of flag is %d', flag)
> +   
>          for installed_bundle in self._bundles:
> -            if bundle.get_bundle_id() == installed_bundle.get_bundle_id() and \
> -                    bundle.get_activity_version() <= \
> -                        installed_bundle.get_activity_version():
> -                raise AlreadyInstalledException
> +            if bundle.get_bundle_id() == installed_bundle.get_bundle_id() and bundle.get_activity_version() <= installed_bundle.get_activity_version():
> +                if flag==0 :
> +                    raise AlreadyInstalledException
> +                    
> +                elif flag==1 : 
> +                    logging.debug('uninstalling previous')

> +                    self.uninstall(installed_bundle, force=True)
> +                

>              elif bundle.get_bundle_id() == installed_bundle.get_bundle_id():
>                  self.uninstall(installed_bundle, force=True)
* two "uninstall" might be error-prone, what about "if" instead of
  "elif"

> -- 
> 1.7.2.2

-- 
Aleksey