[Sugar-devel] [PATCH sugar] Friendstray: make sure the tray is right on the sharer side OLPC #10817

[Simon Schampijer]

On 07/02/2011 06:50 PM, Sascha Silbe wrote:
> Excerpts from Simon Schampijer's message of Fri Jun 24 11:57:56 +0200 2011:
>> The code path that listens for the 'activity-added' signal is used to
>> track the following use case: 'A' starts an activity
>> (he gets the 'active-activity-changed' signal from the shell), he then
>> at a later point shares the activity. In order to track joining/leaving
>> buddies he needs to get the shared activity. On other machines if we
>> receive the 'activity-added' signal we can ignore it, therefore the
>> check if the current active activity is the shared one.
>
> I understood neither the description nor the patch at first. Let's go
> into detail a bit:
>
>
> Sequence of events:
>
> A					B
> ---------------------------------------------------------------------------
> starts activity
> shell:active-activity-changed fires
> shares activity
> neighborhood:activity-added fires?	neighborhood:activity-added fires
> 					joins activity
> 					shell:active-activity-changed fires
>
>
> According to the HIG, the People side of the Frame shows the individuals
> that the user is collaborating with in the currently active
> activity. [1]

Correct.

> On the "sharer" side shell:active-activity-changed will fire before a
> shared activity object exists. So we need to wait for the corresponding
> neighborhood:activity-added signal in order to access the shared
> activity object and connect to its buddy-added and buddy-received
> signals.
>
> On the "joiner" side the shared activity object is already available
> when shell:active-activity-changed fires, so we can connect to the
> signals right away. neighborhood:activity-added doesn't need to do
> anything in this case.
>
> When switching between different activities, the shared activity object
> also already exists (if the activity is shared at all), so again
> neighborhood:activity-added should be a no-op.

Yes, this all is true.

>> +++ b/src/jarabe/frame/friendstray.py
>> @@ -75,6 +75,10 @@ class FriendsTray(VTray):
>>       def __neighborhood_activity_added_cb(self, neighborhood_model,
>>                                            shared_activity):
>>           logging.debug('FriendsTray.__neighborhood_activity_added_cb')
>> +        active_activity = shell.get_model().get_active_activity()
>> +        if active_activity.get_activity_id() != shared_activity.activity_id:
>> +            return
>> +
>
> With the above understanding, shouldn't we reset self._shared_activity
> to None in __active_activity_changed_cb() and add the following check
> before yours?
>
>          if self._shared_activity is not None:
>              return

So my check prevents on any other machine then the sharer machine the 
neighborhood:activity-added to have any effect. All the rest should work 
fine. AFAIK your addition is not needed, from testing and looking at the 
code. But maybe I do oversee something, can you explain which exact case 
it should fix?

Regards,
    Simon