Fixed bug #5688 New friends are not shown on the map
authorSami Rämö <sami.ramo@ixonos.com>
Wed, 12 May 2010 07:29:50 +0000 (10:29 +0300)
committerSami Rämö <sami.ramo@ixonos.com>
Wed, 12 May 2010 07:29:50 +0000 (10:29 +0300)
 - refactored whole friend items updating code

 - BaseLocationItem: removed position() and setPosition methods,
   class is not depending from MapEngine anymore

 - TestFriendLocationItem: removed position related tests

 - FriendGroupItem::dropFriend() changed to public

12 files changed:
src/map/baselocationitem.cpp
src/map/baselocationitem.h
src/map/friendgroupitem.cpp
src/map/friendgroupitem.h
src/map/frienditemshandler.cpp
src/map/frienditemshandler.h
src/map/friendlocationitem.cpp
src/map/friendlocationitem.h
src/map/mapengine.cpp
src/map/ownlocationitem.cpp
src/map/ownlocationitem.h
tests/map/friendlocationitem/testfriendlocationitem.cpp

index 4e3c46e..229c0f6 100644 (file)
@@ -3,6 +3,7 @@
    Copyright (C) 2010  Ixonos Plc. Authors:
 
        Ville Tiensuu - ville.tiensuu@ixonos.com
+       Sami Rämö - sami.ramo@ixonos.com
 
    Situare is free software; you can redistribute it and/or
    modify it under the terms of the GNU General Public License
@@ -21,7 +22,8 @@
 
 #include <QDebug>
 
-#include "mapengine.h" /// @todo REMOVE METHOD AFTER FRIEND ITEMS HAVE BEEN MODIFIED TO USE setPos() METHOD
+#include "mapcommon.h"
+
 #include "baselocationitem.h"
 
 BaseLocationItem::BaseLocationItem(QObject *parent)
@@ -43,20 +45,6 @@ QRect BaseLocationItem::sceneTransformedBoundingRect(int zoomLevel) const
     return rect.adjusted(-widthDelta, -heightDelta, widthDelta, heightDelta);
 }
 
-void BaseLocationItem::setPosition(const QPointF & newPosition)
-{
-    /// @todo REMOVE METHOD AFTER FRIEND ITEMS HAVE BEEN MODIFIED TO USE setPos() METHOD
-    qDebug() << __PRETTY_FUNCTION__;
-    setPos(MapEngine::convertLatLonToSceneCoordinate(newPosition));
-}
-
-QPoint BaseLocationItem::position() const
-{
-    /// @todo REMOVE METHOD AFTER FRIEND ITEMS HAVE BEEN MODIFIED TO USE setPos() METHOD
-    qDebug() << __PRETTY_FUNCTION__;
-    return pos().toPoint();
-}
-
 void BaseLocationItem::hideLocation()
 {
     qDebug() << __PRETTY_FUNCTION__;
index ae20156..aa14dc7 100644 (file)
@@ -3,6 +3,7 @@
    Copyright (C) 2010  Ixonos Plc. Authors:
 
        Ville Tiensuu - ville.tiensuu@ixonos.com
+       Sami Rämö - sami.ramo@ixonos.com
 
    Situare is free software; you can redistribute it and/or
    modify it under the terms of the GNU General Public License
@@ -31,6 +32,7 @@
 *
 * @class BaseLocationItem baselocationitem.h "map/baselocationitem.h"
 * @author Ville Tiensuu
+* @author Sami Rämö - sami.ramo@ixonos.com
 */
 class BaseLocationItem : public QGraphicsPixmapItem, public QObject
 {    
@@ -57,13 +59,6 @@ public:
     void hideLocation();
 
     /**
-    * @brief returns position of item in scene coordinates.
-    *
-    * @return QPoint position of OwnLocationItem
-    */
-    QPoint position() const;
-
-    /**
       *@brief Return item sceneBoundingRect transformed to given zoom level
       *
       * Because of using ItemIgnoresTransformations, and not scaling the item, the default
@@ -76,14 +71,6 @@ public:
     QRect sceneTransformedBoundingRect(int zoomLevel) const;
 
     /**
-    * @brief sets position of item as specified in parameter.
-    *        Parameter can be given in georaphical coordinates
-    *
-    * @param newPosition Parameter that specifies new position.
-    */
-    void setPosition(const QPointF & newPosition);
-
-    /**
     * @brief Sets item to be visible on the map.
     *        Item is visible by default, function is needed only when own location
     *        needs to be set back visible after hideLocation function call.
index 8ae761e..d64558d 100644 (file)
@@ -47,8 +47,8 @@ void FriendGroupItem::dropFriend(FriendLocationItem *item)
 {
     qDebug() << __PRETTY_FUNCTION__;
 
-    m_friends.removeOne(item);
-    item->setPartOfGroup(false);
+    if (m_friends.removeOne(item))
+        item->setPartOfGroup(false);
 }
 
 bool FriendGroupItem::dropFriends(int zoomLevel)
index d9f8f22..c8add0c 100644 (file)
@@ -72,6 +72,13 @@ public:
     bool dropFriends(int zoomLevel);
 
     /**
+      * @brief Drop single FriendLocationItem from this group
+      *
+      * @param item FriendLocationItem to be dropped
+      */
+    void dropFriend(FriendLocationItem *item);
+
+    /**
       * @brief Join new FriendLocationItem to this group.
       *
       * Given item is also hidden.
@@ -90,14 +97,9 @@ public:
       */
     void mergeWithGroup(FriendGroupItem *group);
 
-private:
-    /**
-      * @brief Drop single FriendLocationItem from this group
-      *
-      * @param item FriendLocationItem to be dropped
-      */
-    void dropFriend(FriendLocationItem *item);
-
+/*******************************************************************************
+ * DATA MEMBERS
+ ******************************************************************************/
 private:
     QList<FriendLocationItem *> m_friends; ///< List of joined FriendLocationItem items
 };
index 9b61023..9c39c4b 100644 (file)
@@ -38,6 +38,19 @@ FriendItemsHandler::FriendItemsHandler(MapScene *mapScene, QObject *parent)
     qDebug() << __PRETTY_FUNCTION__;
 }
 
+void FriendItemsHandler::addFriendItem(User *friendData)
+{
+    qDebug() << __PRETTY_FUNCTION__;
+
+    FriendLocationItem *item = new FriendLocationItem(friendData->profileImage());
+
+    item->setUserId(friendData->userId());
+    item->setProfileImageUrl(friendData->profileImageUrl());
+    item->setPos(MapEngine::convertLatLonToSceneCoordinate(friendData->coordinates()));
+    m_friendItems.append(item);
+    m_mapScene->addItem(item);
+}
+
 void FriendItemsHandler::checkAllFriendsForCollidingFriends()
 {
     qDebug() << __PRETTY_FUNCTION__;
@@ -150,32 +163,21 @@ void FriendItemsHandler::mergeCollidingGroups()
     }
 }
 
-void FriendItemsHandler::cleanOldFriendData(const QList<User *> &friendsList)
+void FriendItemsHandler::deleteFriendItem(FriendLocationItem *item)
 {
     qDebug() << __PRETTY_FUNCTION__;
 
-    // loop through all m_friendItem items
-    QLinkedList<FriendLocationItem *>::iterator iter = m_friendItems.begin();
-    while (iter != m_friendItems.end()) {
-        bool friendFound = false;
+    dropFriendFromAllGroups(item);
+    m_mapScene->removeItem(item);
+    delete item;
+}
 
-        // check against each item in the friendsList
-        QList<User *>::const_iterator j;
-        for (j = friendsList.begin(); j != friendsList.end(); j++) {
-            if ((**iter).userId() == (**j).userId()) {
-                friendFound = true;
-                break;
-            }
-        }
+void FriendItemsHandler::dropFriendFromAllGroups(FriendLocationItem *item)
+{
+    qDebug() << __PRETTY_FUNCTION__;
 
-        if (!friendFound) {
-            m_mapScene->removeItem(*iter);
-            delete *iter;
-            iter = m_friendItems.erase(iter);
-        }
-        else {
-            iter++;
-        }
+    foreach (FriendGroupItem *group, m_friendGroupItems) {
+        group->dropFriend(item);
     }
 }
 
@@ -194,22 +196,47 @@ void FriendItemsHandler::dropOutOfGroupFriends()
     }
 }
 
-void FriendItemsHandler::receiveFriendLocations(QList<User *> &friendsList)
+void FriendItemsHandler::friendListUpdated(QList<User *> &friendsList)
 {
     qDebug() << __PRETTY_FUNCTION__;
 
-    static int receiveFriendsLocationsCounter = 0;
-
-    if (receiveFriendsLocationsCounter == 0) {
-        receiveFriendsLocationsCounter++;
-        updateFriendItemList(friendsList);
+    // loop through new friend data, find matching friend items and update them, or add new items
+    // if old items are not found
+    foreach (User * friendData, friendsList) {
+        bool found = false;
+        foreach (FriendLocationItem *friendItem, m_friendItems) {
+            if (friendData->userId() == friendItem->userId()) {
+                // friend item was found so update the data
+                updateFriendItem(friendItem, friendData);
+                found = true;
+                break;
+            }
+        }
+        if (!found) {
+            // friend item was not found so new item must be added
+            addFriendItem(friendData);
+        }
     }
 
-    else {
-        /// @todo VILLE: New friends are never added to list & scene
-        receiveFriendsLocationsCounter++;
-        updateFriendLocationsAndImages(friendsList);
-        cleanOldFriendData(friendsList);
+    // loop through friend items and find matching friend data. If matching data
+    // is not found then remove item
+    QLinkedList<FriendLocationItem *>::iterator iter = m_friendItems.begin();
+    while (iter != m_friendItems.end()) {
+        bool found = false;
+        foreach (User * friendData, friendsList) {
+            if (friendData->userId() == (*iter)->userId()) {
+                found = true;
+                break;
+            }
+        }
+        if (!found) {
+            // data for friend item was not found so item must be deleted
+            deleteFriendItem(*iter);
+            iter = m_friendItems.erase(iter);
+        }
+        else {
+            iter++;
+        }
     }
 
     refactorFriendItems(m_zoomLevel);
@@ -227,48 +254,18 @@ void FriendItemsHandler::refactorFriendItems(int zoomLevel)
     checkAllFriendsForCollidingFriends();
 }
 
-void FriendItemsHandler::updateFriendItemList(const QList<User *> &friendsList)
+void FriendItemsHandler::updateFriendItem(FriendLocationItem *friendItem, User *friendData)
 {
     qDebug() << __PRETTY_FUNCTION__;
 
-    qDeleteAll(m_friendItems.begin(),m_friendItems.end());
-    m_friendItems.clear();
-
+    // update position
+    QPoint newPosition = MapEngine::convertLatLonToSceneCoordinate(friendData->coordinates());
+    if (friendItem->pos().toPoint() != newPosition)
+        friendItem->setPos(newPosition);
 
-    for (int i=0; i<friendsList.count(); i++) {
-        FriendLocationItem *friendLocation
-                = new FriendLocationItem(friendsList.at(i)->profileImage(),
-                                         friendsList.at(i)->coordinates(),0);
-
-        friendLocation->setUserId(friendsList.at(i)->userId());
-        m_friendItems.append(friendLocation);
-        m_mapScene->addItem(friendLocation);
-
-        refactorFriendItems(m_zoomLevel);
-    }
-}
-
-void FriendItemsHandler::updateFriendLocationsAndImages(const QList<User *> &friendsList)
-{
-    qDebug() << __PRETTY_FUNCTION__;
-
-    for (int i=0; i<friendsList.count(); i++) {
-
-        QLinkedList<FriendLocationItem *>::iterator iter;
-        for (iter = m_friendItems.begin(); iter!= m_friendItems.end(); iter++) {
-
-            if ((*iter)->userId() == friendsList.at(i)->userId()) {
-
-                if ((*iter)->position()
-                    != MapEngine::convertLatLonToSceneCoordinate(friendsList.at(i)->coordinates()))
-                    (*iter)->setPosition(friendsList.at(i)->coordinates());
-
-                if ((*iter)->profileImageUrl()
-                    != friendsList.at(i)->profileImageUrl()) {
-                    (*iter)->setPixmap(friendsList.at(i)->profileImage());
-                    (*iter)->setProfileImageUrl(friendsList.at(i)->profileImageUrl());
-                }
-            }
-        }
+    // update image
+    if (friendItem->profileImageUrl() != friendData->profileImageUrl()) {
+        friendItem->setPixmap(friendData->profileImage());
+        friendItem->setProfileImageUrl(friendData->profileImageUrl());
     }
 }
index b6fa04e..e35ff60 100644 (file)
@@ -59,6 +59,16 @@ public:
  ******************************************************************************/
 private:
     /**
+      * @brief Add new FriendLocationItem
+      *
+      * New FriendLocationItem is created, values are set and item is added to map.
+      * Item is also added to m_friendItems list.
+      *
+      * @param friendData Data for new friend
+      */
+    void addFriendItem(User *friendData);
+
+    /**
       * @brief Group colliding friends
       *
       * Call checkFriendForCollidingFriends() for all visible FriendLocationItem items.
@@ -111,6 +121,22 @@ private:
     void cleanOldFriendData(const QList<User *> &friendsList);
 
     /**
+      * @brief Delete FriendLocationItem
+      *
+      * Drops item from all groups, removes it from scene and deletes the item.
+      *
+      * @param item Item to be deleted
+      */
+    void deleteFriendItem(FriendLocationItem *item);
+
+    /**
+      * @brief Drop FriendLocationItem from all FriendGroupItem groups
+      *
+      * @param item Item to be dropped.
+      */
+    void dropFriendFromAllGroups(FriendLocationItem *item);
+
+    /**
       * @brief Drop all FriendLocationItem items, which are out of FriendGroupItem boundaries,
       * from all FriendGroupItem items.
       */
@@ -124,6 +150,16 @@ private:
     void mergeCollidingGroups();
 
     /**
+      * @brief Update FriendLocationItem data
+      *
+      * Position and image are updated.
+      *
+      * @param friendItem Item to be updated
+      * @param friendData New data for the item
+      */
+    void updateFriendItem(FriendLocationItem *friendItem, User *friendData);
+
+    /**
     * @brief updates data member m_friendItems from given parameter
     *
     * @param friendsList QList item of friend information
@@ -139,11 +175,14 @@ private:
 
 private slots:
     /**
-    * @brief Slot to catch friends location data
+    * @brief Slot for upgrading friend items and groups
+    *
+    * Does add and/or remove FriendLocationItem items if there are new friends
+    * or old ones deleted in the given list. Remaining FriensLocationItems are updated.
     *
     * @param friendsList QList item of friend information
     */
-    void receiveFriendLocations(QList<User *> &friendsList);
+    void friendListUpdated(QList<User *> &friendsList);
 
     /**
       * @brief Re-factors all FriendLocationItem and FriendGroupItem items.
index b72e637..60e6bbd 100644 (file)
@@ -3,6 +3,7 @@
    Copyright (C) 2010  Ixonos Plc. Authors:
 
        Ville Tiensuu - ville.tiensuu@ixonos.com
+       Sami Rämö - sami.ramo@ixonos.com
 
    Situare is free software; you can redistribute it and/or
    modify it under the terms of the GNU General Public License
 
 #include <QDebug>
 #include "friendlocationitem.h"
-#include "mapengine.h"
 #include "mapcommon.h"
 
 FriendLocationItem::FriendLocationItem(const QPixmap &icon,
-                                       const QPointF &friendsLocation,
                                        QObject *parent)
     : BaseLocationItem(parent),
       m_partOfGroup(false)
-
 {
     qDebug() << __PRETTY_FUNCTION__;
 
     setPixmap(icon);
-    setPosition(QPointF( friendsLocation.x(), friendsLocation.y() ));
+    setPos(UNDEFINED, UNDEFINED);
     setZValue(FRIEND_LOCATION_ICON_Z_LEVEL);
     setOffset(-icon.width()/2, -icon.height()/2);
 }
@@ -48,7 +46,6 @@ void FriendLocationItem::mousePressEvent(QGraphicsSceneMouseEvent *event)
 {
     qDebug() << __PRETTY_FUNCTION__;
     Q_UNUSED(event);
-    qDebug() << "Friends Location is " << position().x() << "  " << position().y();
     qDebug() << "Friends user ID is " << m_userId.toAscii();
     emit iconClicked(m_userId);
 }
index d3767e4..be57256 100644 (file)
@@ -3,6 +3,7 @@
    Copyright (C) 2010  Ixonos Plc. Authors:
 
        Ville Tiensuu - ville.tiensuu@ixonos.com
+       Sami Rämö - sami.ramo@ixonos.com
 
    Situare is free software; you can redistribute it and/or
    modify it under the terms of the GNU General Public License
@@ -33,6 +34,7 @@
 *
 * @class FriendLocationItem friendlocationitem.h "map/friendlocationitem.h"
 * @author Ville Tiensuu.
+* @author Sami Rämö - sami.ramo@ixonos.com
 */
 class FriendLocationItem : public BaseLocationItem
 {
@@ -42,7 +44,7 @@ public:
 
     /**
     * @brief Constructor of FriendLocationItem.
-    *        Sets position to specified location.
+    *        Sets position to UNDEFINED.
     *        Loads and sets default pixmap that is show on the map.
     *        Sets default Z-value.
     *        Sets offset so that achor of the picture is at the origin. this feature is
@@ -50,11 +52,10 @@ public:
     *        Sets item to ignore transformations. this feature is needed to make icon on the map
     *        immune to scaling
     *
-    * @param icon Friends Facebook profile picture, friendsLocation Position in QPoinF, parent
-    * @param friendsLocation Location of friend
+    * @param icon Friends Facebook profile picture
     * @param parent Parent
     */
-    FriendLocationItem(const QPixmap &icon, const QPointF &friendsLocation, QObject *parent = 0);
+    FriendLocationItem(const QPixmap &icon, QObject *parent = 0);
 
 /*******************************************************************************
 * MEMBER FUNCTIONS AND SLOTS
index 8cd40d2..6d221c2 100644 (file)
@@ -57,7 +57,7 @@ MapEngine::MapEngine(QObject *parent)
     connect(m_mapZoomPanel, SIGNAL(zoomInPressed()), this, SLOT(zoomIn()));
     connect(m_mapZoomPanel, SIGNAL(zoomOutPressed()), this, SLOT(zoomOut()));
 
-    m_ownLocation = new OwnLocationItem(QPoint(UNDEFINED, UNDEFINED));
+    m_ownLocation = new OwnLocationItem();
     m_ownLocation->hide(); // hide until first location info is received
     m_mapScene->addItem(m_ownLocation);
 
@@ -66,7 +66,7 @@ MapEngine::MapEngine(QObject *parent)
             m_friendItemsHandler, SLOT(refactorFriendItems(int)));
 
     connect(this, SIGNAL(friendsLocationsReady(QList<User*>&)),
-            m_friendItemsHandler, SLOT(receiveFriendLocations(QList<User*>&)));
+            m_friendItemsHandler, SLOT(friendListUpdated(QList<User*>&)));
 }
 
 void MapEngine::init()
index f645d7d..8abe941 100644 (file)
 #include "mapengine.h"
 #include "mapcommon.h"
 
-OwnLocationItem::OwnLocationItem(const QPointF & ownPosition)
+OwnLocationItem::OwnLocationItem()
 {
     qDebug() << __PRETTY_FUNCTION__;
     QPixmap ownLocationIcon(":/res/images/led_red.png");
     setPixmap(ownLocationIcon);
 
-    setPosition(ownPosition);
+    setPos(QPoint(UNDEFINED, UNDEFINED));
     setZValue(OWN_LOCATION_ICON_Z_LEVEL);
     setOffset(-ownLocationIcon.width()/2, -ownLocationIcon.height()/2);
 }
index 3216c86..636ad13 100644 (file)
@@ -38,17 +38,14 @@ class OwnLocationItem : public BaseLocationItem
 public:
     /**
     * @brief Constructor of OwnLocationItem.
-    *        Sets position to specified location.
     *        Loads and sets default pixmap that is show on the map.
     *        Sets default Z-value.
     *        Sets offset so that achor of the picture is at the origin. this feature is
     *        needed to center icon on the middle of the location.
     *        Sets item to ignore transformations. this feature is needed to make icon on the map
     *        immune to scaling
-    *
-    * @param ownPosition Position in QPoinF format
     */
-    OwnLocationItem(const QPointF & ownPosition);
+    OwnLocationItem();
 };
 
 #endif // OWNLOCATIONITEM_H
index f368e15..989d715 100644 (file)
@@ -60,8 +60,6 @@ class TestFriendLocationItem: public QObject
     /**
     * @brief Test method for constructors.
     *        Creates instance of FriendLocationItem
-    *        Tests that position is set correctly.
-    *        Tests that location is set correctly.
     *        Tests that Z-values are set correctly.
     *        Tests that offses are set correctly.
     *        Tests that ItemIgnoresTransformations flag is set.
@@ -69,11 +67,6 @@ class TestFriendLocationItem: public QObject
      void testConstructor();
 
      /**
-     * @brief Test that position of friend location item is set and get correctly
-     */
-     void testSetAndGetPosition();
-
-     /**
      * @brief Tests that friend location item is possible to set visible and unvisible
      */
      void testHideAndShowPosition();
@@ -87,7 +80,7 @@ class TestFriendLocationItem: public QObject
  void TestFriendLocationItem::testConstructor()
  {    
      QPixmap friendIcon("situare_user.gif");
-     FriendLocationItem friendLocation(friendIcon,defaultLocationPoint);
+     FriendLocationItem friendLocation(friendIcon);
 
      // Test Pixmap
      QPixmap pixmap;
@@ -96,10 +89,6 @@ class TestFriendLocationItem: public QObject
      pixmap = friendLocation.pixmap();
      QCOMPARE (pixmap.isNull(), false);
 
-     // Test Position
-     QCOMPARE(friendLocation.position(),
-              MapEngine::convertLatLonToSceneCoordinate(defaultLocationPoint));
-
      // Test Z-value
      QCOMPARE(static_cast<int>(friendLocation.zValue()), FRIEND_LOCATION_ICON_Z_LEVEL);
 
@@ -113,25 +102,10 @@ class TestFriendLocationItem: public QObject
 
  }
 
- void TestFriendLocationItem::testSetAndGetPosition()
- {
-     QPixmap friendIcon(":/res/images/situare_user.jpg");
-     FriendLocationItem friendLocation(friendIcon, defaultLocationPoint);
-
-     QCOMPARE(friendLocation.position(),
-              MapEngine::convertLatLonToSceneCoordinate(defaultLocationPoint));
-
-     friendLocation.setPosition(testLocationPoint);
-
-     QCOMPARE(friendLocation.position(),
-              MapEngine::convertLatLonToSceneCoordinate(testLocationPoint));
-
- }
-
  void TestFriendLocationItem::testHideAndShowPosition()
  {
      QPixmap friendIcon(":/res/images/situare_user.jpg");
-     FriendLocationItem friendLocation(friendIcon, defaultLocationPoint);
+     FriendLocationItem friendLocation(friendIcon);
      QCOMPARE(friendLocation.isVisible(), true);
 
      friendLocation.hideLocation();
@@ -143,8 +117,8 @@ class TestFriendLocationItem: public QObject
 
  void TestFriendLocationItem::testSetUserId()
  {
-     QPixmap friendIcon(":/res/images/situare_user.jpg");
-     FriendLocationItem friendLocation(friendIcon, defaultLocationPoint);
+     QPixmap friendIcon("situare_user.jpg");
+     FriendLocationItem friendLocation(friendIcon);
 
      QString userIdentity = "110022";
      friendLocation.setUserId(userIdentity);