Opened 14 years ago

Closed 13 years ago

Last modified 11 years ago

#1742 closed defect (fixed)

palettes of buddies in the neighborhood don't update

Reported by: sascha_silbe Owned by: tomeu
Priority: Unspecified by Maintainer Milestone:
Component: Sugar Version: Git as of bugdate
Severity: Blocker Keywords: olpc-0.90
Cc: sdz, timclicks, smparrish, alsroot, rgs Distribution/OS:
Bug Status: New

Description

How to reproduce:

  1. Switch to neighborhood.
  2. Right click on someone who isn't a "friend" yet.
  3. Choose "Make friend".
  4. Right click again on the same person.

Actual results:
The palette still shows "Make friend".

Expected results:
Show "Remove friend".

Even switching somewhere else and back to the neighborhood doesn't change the entry.

Change History (22)

comment:1 Changed 14 years ago by walter

I've confirmed this. It seems to be a regression.

comment:2 Changed 14 years ago by sascha_silbe

  • Distribution/OS Unspecified deleted
  • Keywords sugar-love added

A quick check of the source suggests that jarabe.view.buddymenu.BuddyMenu needs to listen to the friend-added and friend-removed signals of jarabe.model.friends.Friends.

comment:3 Changed 14 years ago by sdz

  • Cc sdz added

comment:4 follow-up: Changed 14 years ago by timclicks

  • Cc timclicks added

Looking at jarabe.view.buddymenu.BuddyMenu, there is already a check to see whether the person is a current friend. If the method is checking for the state every time and doesn't have its own list of buddies, does it need to listen to signals also?

This seems to be the relevant method:

def  _add_buddy_items(self):
    if friends.get_model().has_buddy(self._buddy):
        menu_item = MenuItem(_('Remove friend'), 'list-remove')
        menu_item.connect('activate', self._remove_friend_cb)
    else:
        menu_item = MenuItem(_('Make friend'), 'list-add')
        menu_item.connect('activate', self._make_friend_cb)

If this check isn't working, it could mean that jarabe.model.friends.get_model isn't working properly... I don't see a __call__() method in jarabe.model.friends.Friends, but I don't know enough about gobject.GObject to know if this is a problem.

_model =  None

def get_model():
    global _model
    if _model is None:
        _model = Friends()
    return _model

comment:5 in reply to: ↑ 4 Changed 14 years ago by bernie

Replying to timclicks:

If this check isn't working, it could mean that jarabe.model.friends.get_model isn't working properly... I don't see a __call__() method in jarabe.model.friends.Friends, but I don't know enough about gobject.GObject to know if this is a problem.

_model =  None

def get_model():
    global _model
    if _model is None:
        _model = Friends()
    return _model

get_model() implements a Singleton pattern. The first time you invoke it, it builds a Friends object through the constructor and stores a reference to it in the global variable _model, which is used in future calls.

comment:6 Changed 13 years ago by smparrish

  • Cc smparrish added

comment:7 Changed 13 years ago by anurag_seeta

What i noticed regarding this bug was that after adding a friend you will still get add friend option instead of remove friend , but once you restart your system then next time you will get remove friend option but any more subsequent changes will again have the same issues. So what i gather is there must be an issue with the entire refresh criterion which is done only on system restart, are there any set of files in sugar that update only on system restart and which could possibly be the reason behind this bug because if it would have been a codind error then even a system restart would not of have updated the pallete.

comment:8 Changed 13 years ago by smparrish

  • seeta_dev set to Anurag

comment:9 Changed 13 years ago by anurag_seeta

What I figured out for this bug is that the bug could be solved only if the shell service has a D-Bus API for buddies. Untill then even the present scenario of one time update during system restart has also been applied through a hack place in sync_friends () in the jarabe.model.friends.py module
this bug will get wrapped itself when the said api for shell service will get designed
then even this sync_friends function would be removed.

But I would need some pointers on how to create this dbus api for the present scenario and how will that work through the process to solve this defect.

comment:10 Changed 13 years ago by tomeu

Why D-Bus? We have only one process involved, so there's no need for any IPC.

Sascha and Tim are right in their analysis, but the solution I would prefer is to make sure that the palette gets disposed when it pops down. That way, when it pops up again, it will check the current status of the buddy.

comment:11 Changed 13 years ago by tomeu

  • Summary changed from after adding a friend the palette doesn't change to palettes of buddies in the neighborhood don't update

This also affect other parts of the palette such as the nick name.

comment:12 follow-up: Changed 13 years ago by alsroot

  • Severity changed from Minor to Blocker

I've set Severity to Blocker because commit (it is in 0.90.2)
http://git.sugarlabs.org/projects/sugar/repos/mainline/commits/356641c332d6cc889b265dfc29598004cf37763c
introduced a regression.

|TestCase|

  • switch to journal
  • open entry palette
  • choose "Vew Details"
  • nothing happened

The problem part is:

diff --git a/src/sugar/graphics/palettewindow.py b/src/sugar/graphics/palettewindow.py
index f51c938..449c77f 100644
--- a/src/sugar/graphics/palettewindow.py
+++ b/src/sugar/graphics/palettewindow.py
@@ -642,7 +642,6 @@ class Invoker(gobject.GObject):
         if self._palette is not None:
             self._palette.popdown(immediate=True)
             self._palette.props.invoker = None
-            self._palette.destroy()

         self._palette = palette

This is because on popdown, palette is being destroyed but Journal sends envet for Palette object, thus signal never be processed (maybe it works in some cases but I guess it is indeterminate behaviour).

In fact, it might be fixed in Journal but do we need destroy() call. afaik, gc will do this work but after that all signals will be delivered.

comment:13 Changed 13 years ago by alsroot

  • Milestone changed from 0.88 to 0.90

comment:15 Changed 13 years ago by alsroot

  • Component changed from sugar to sugar-toolkit

comment:16 Changed 13 years ago by alsroot

  • Cc alsroot added

comment:17 Changed 13 years ago by erikos

Sorry to be so late on this. Yes this is bad indeed. Another occurrence is, that you can not delete activities from the list view anymore. And yes the 'destroy' that Aleksey has been pointing out, is the issue. Need a closer look as to what is the appropriate fix here.

comment:18 Changed 13 years ago by rgs

  • Cc rgs added

comment:19 Changed 13 years ago by erikos

  • Keywords olpc-0.90 added; sugar-love removed

comment:20 Changed 13 years ago by erikos

  • Resolution set to fixed
  • Status changed from new to closed

comment:21 Changed 11 years ago by dnarvaez

  • Component changed from sugar-toolkit to Sugar

comment:22 Changed 11 years ago by dnarvaez

  • Milestone 0.90 deleted

Milestone 0.90 deleted

Note: See TracTickets for help on using tickets.