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

[Aleksey Lim]

On Sat, Oct 23, 2010 at 01:11:18AM +0530, anubhav at seeta.in wrote:
> 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.
> 
> Co-authored-by: Anubhav Aggarwal <anubhav at seeta.in>, Shanjit Singh Jajmann <shanjit at seeta.in>
> ---
> 
> v1 -> v2. Named according to the nomenclature suggested,inline function used
> ,signal emission condition revised,global variables removed.
> 
> v2 -> v3. Taking care of all calling of misc.resume.
> 
> v3 -> v4. Changes in the copyright of the new file
> 
> v4 -> v5. showing the alert in the same window as the journal


To be hones, I'm not happy with this way. It seems to be needless
tricky, i.e, keeping journal window in alert class and storing it
all time. What about more clean way, i.e., using journal window
only on purpose and keep it out of journal activity logic, so any
code in Journal that needs main window to to do something (e.g.,
popping up an alert) can call journalwindow.get_activity_window(),
use it and do not keep in static variable.

>  src/jarabe/journal/journalactivity.py |    1 +
>  src/jarabe/journal/misc.py            |   56 ++++++++++++++++++++++++++------
>  src/jarabe/model/bundleregistry.py    |    7 +++-
>  3 files changed, 51 insertions(+), 13 deletions(-)
> 
> diff --git a/src/jarabe/journal/journalactivity.py b/src/jarabe/journal/journalactivity.py
> index 44cc018..8ab6a2e 100644
> --- a/src/jarabe/journal/journalactivity.py
> +++ b/src/jarabe/journal/journalactivity.py
> @@ -106,6 +106,7 @@ class JournalActivity(Window):
>      def __init__(self):
>          logging.debug("STARTUP: Loading the journal")
>          Window.__init__(self)
> +        misc.get_journal_window(self)
>  
>          self.set_title(_('Journal'))
>  
> diff --git a/src/jarabe/journal/misc.py b/src/jarabe/journal/misc.py
> index 32a2847..b8d8d3a 100644
> --- a/src/jarabe/journal/misc.py
> +++ b/src/jarabe/journal/misc.py
> @@ -27,8 +27,10 @@ from sugar.activity import activityfactory
>  from sugar.activity.activityhandle import ActivityHandle
>  from sugar.graphics.icon import get_icon_file_name
>  from sugar.graphics.xocolor import XoColor
> +from sugar.graphics.alert import ConfirmationAlert
>  from sugar import mime
>  from sugar.bundle.activitybundle import ActivityBundle
> +from sugar.bundle.bundle import AlreadyInstalledException
>  from sugar.bundle.contentbundle import ContentBundle
>  from sugar import util
>  
> @@ -148,8 +150,9 @@ def get_activities(metadata):
>  
>      return activities
>  

> -def resume(metadata, bundle_id=None):
> +def resume(metadata, bundle_id=None, force_to_downgrade=False):
I think better to have force_to_downgrade hidden because it is being
used only from resume() itlsef...

>      registry = bundleregistry.get_registry()
> +    version_downgrade = False
>  
>      if is_activity_bundle(metadata) and bundle_id is None:
>  
> @@ -159,19 +162,26 @@ def resume(metadata, bundle_id=None):
>          bundle = ActivityBundle(file_path)
>          if not registry.is_installed(bundle):
>              logging.debug('Installing activity bundle')
> -            registry.install(bundle)
> +            if not force_to_downgrade:
> +                try:
> +                    registry.install(bundle)
> +                except AlreadyInstalledException:
> +                    older_version_clicked(metadata)
> +                    version_downgrade = True
> +            if force_to_downgrade:
> +                registry.install(bundle, force_downgrade=True)
>          else:
>              logging.debug('Upgrading activity bundle')
>              registry.upgrade(bundle)

...it could be done, eg, by moving this part to separate function and
call it from here and from downgrade alert callback 
> -
> -        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())
> +        if not version_downgrade:
> +            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:
>  
> @@ -239,6 +249,30 @@ def launch(bundle, activity_id=None, object_id=None, uri=None, color=None,
>              object_id=object_id, uri=uri, invited=invited)
>      activityfactory.create(bundle, activity_handle)
>  
> +def get_journal_window(journal_window_instance):
> +    VersionAlert.journal_window = journal_window_instance
> +
> +class VersionAlert():
> +    journal_window = None
> +

> +def older_version_clicked(metadata):
> +    alert = ConfirmationAlert()
> +    alert.props.title = _('Newer Version Found')
> +    alert.props.msg = _('Newer version of the chosen activity is available \
> +    do you still want to continue with the installation? \
> +                         If Yes click Ok and the activity icon of the older \
> +    .xo file in the Journal')
> +    alert.connect('response', downgrade_alert_response_cb, metadata)
> +    VersionAlert.journal_window.add_alert(alert)
> +    alert.show()
> +
> +def downgrade_alert_response_cb(alert, response_id, metadata):
> +    if response_id is gtk.RESPONSE_OK:
> +        VersionAlert.journal_window.remove_alert(alert)
> +        resume(metadata, force_to_downgrade=True)
> +    elif response_id is gtk.RESPONSE_CANCEL:
> +        VersionAlert.journal_window.remove_alert(alert)
these methoids seem to be used only from misc, so their names should be
prefixed by "_". Also "older_version_clicked" seems irrelevant
(misc.resume is not tied to any upper UI workflows).

>  def is_activity_bundle(metadata):
>      mime_type = metadata.get('mime_type', '')
>      return mime_type == ActivityBundle.MIME_TYPE or \
> diff --git a/src/jarabe/model/bundleregistry.py b/src/jarabe/model/bundleregistry.py
> index 699e339..06a1d3a 100644
> --- a/src/jarabe/model/bundleregistry.py
> +++ b/src/jarabe/model/bundleregistry.py
> @@ -373,14 +373,17 @@ class BundleRegistry(gobject.GObject):
>                  return True
>          return False
>  
> -    def install(self, bundle, uid=None):
> +    def install(self, bundle, uid=None, force_downgrade=False):
>          activities_path = env.get_user_activities_path()
>  
>          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 not force_downgrade:
> +                    raise AlreadyInstalledException
> +                if force_downgrade:
> +                    self.uninstall(installed_bundle, force=True)
>              elif bundle.get_bundle_id() == installed_bundle.get_bundle_id():
>                  self.uninstall(installed_bundle, force=True)
>  
> -- 
> 1.7.0.4
> 

-- 
Aleksey