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

[Bernie Innocenti]

On Tue, 2010-10-12 at 01:15 +0530, shanjit at seeta.in wrote:

> @@ -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)

Here (and elsewhere) you're adding some invisible blanks.


> @@ -145,7 +145,30 @@ class JournalActivity(Window):
>          alert.connect('response', self.__alert_response_cb)
>          self.add_alert(alert)
>          alert.show()
> -
> +        
> +    def __activity_alert1_cb(self):

The name alert1 does not suggest much about what this alert is about.
How about version_alert?


> +             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)

The message to the user isn't very clear: aren't we supposed to complain
only in the case of a downgrade?


> +             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)

Why invoke __activity_alert1_cb synchronously? Since this function isn't
really a callback, you would move its entire body inline here.


> @@ -527,6 +531,11 @@ class LyistView(BaseListView):
>          row = self.tree_view.get_model()[path]
>          metadata = model.get(row[ListModel.COLUMN_UID])
>          misc.resume(metadata)
> +        self.emit('icon-clicked')

Isn't this signal too generic for what we need to accomplish?
icon-clicked would trigger while clicking on *any* item in the Journal,
not just installable bundles, isn't it?


> @@ -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
> +

Hmm... a global flag is probably going to have unintended effects. Also,
the name is too broad: checked *what*?

Thanks,

-- 
   // Bernie Innocenti - http://codewiz.org/
 \X/  Sugar Labs       - http://sugarlabs.org/