Appended comments on unit test.

Added 'valid' property in QStateAction.

8 files changed, 157 insertions, 31 deletions

diff --git a/src/common/qdbusactiongroup.cpp b/src/common/qdbusactiongroup.cpp
index 2b3528e..1038014 100644
--- a/src/common/qdbusactiongroup.cpp
+++ b/src/common/qdbusactiongroup.cpp
@@ -71,13 +71,22 @@ QDBusActionGroup::~QDBusActionGroup()
 */
 QStateAction *QDBusActionGroup::action(const QString &name)
 {
+    QStateAction *act = actionImpl(name);
+    if (act == 0) {
+        return addAction(name.toLatin1(), true);
+    } else {
+        return act;
+    }
+}
+
+QStateAction *QDBusActionGroup::actionImpl(const QString &name)
+{
     Q_FOREACH(QStateAction *act, m_actions) {
         if (act->text() == name) {
             return act;
         }
     }
-
-    return NULL;
+    return 0;
 }
 
 /*!
@@ -162,48 +171,66 @@ void QDBusActionGroup::setActionGroup(GDBusActionGroup *ag)
 
         gchar **actionNames = g_action_group_list_actions(m_actionGroup);
         for(int i=0; actionNames[i] != NULL; i++) {
-            addAction(actionNames[i]);
+            addAction(actionNames[i], true);
         }
         g_strfreev(actionNames);
     }
 }
 
 /*! \internal */
-void QDBusActionGroup::addAction(const char *actionName)
+QStateAction *QDBusActionGroup::addAction(const char *actionName, bool create)
 {
-    QStateAction *act = new QStateAction(actionName, this);
-
-    act->setEnabled(g_action_group_get_action_enabled(m_actionGroup, actionName));
-
-    GVariant *actState = g_action_group_get_action_state(m_actionGroup, actionName);
-    if (actState) {
-        act->setState(Converter::parseGVariant(actState));
+    bool isNew = false;
+    QStateAction *act = actionImpl(actionName);
+    if (act == 0) {
+        if (create) {
+            act = new QStateAction(actionName, this);
+            isNew = true;
+        } else {
+            return 0;
+        }
     }
 
-    QObject::connect(act, SIGNAL(triggered()), this, SLOT(onActionTriggered()));
+    if (g_action_group_has_action(m_actionGroup, actionName)) {
+        act->setEnabled(g_action_group_get_action_enabled(m_actionGroup, actionName));
 
-    // remove any older action with the same name
-    removeAction(actionName);
+        GVariant *actState = g_action_group_get_action_state(m_actionGroup, actionName);
+        if (actState) {
+            act->setState(Converter::parseGVariant(actState));
+        }
+        act->setValid(true);
+    } else {
+        act->setValid(false);
+    }
 
-    m_actions.insert(act);
-    Q_EMIT countChanged(m_actions.count());
+    if (isNew) {
+        QObject::connect(act, SIGNAL(triggered()), this, SLOT(onActionTriggered()));
+        m_actions.insert(act);
+        Q_EMIT countChanged(m_actions.count());
+    }
+    return act;
 }
 
 /*! \internal */
 void QDBusActionGroup::onActionTriggered()
 {
-    QAction *act = qobject_cast<QAction*>(QObject::sender());
-    g_action_group_activate_action(m_actionGroup, act->text().toLatin1(), NULL);
+    QStateAction *act = qobject_cast<QStateAction*>(QObject::sender());
+    if (act->isValid()) {
+        g_action_group_activate_action(m_actionGroup, act->text().toLatin1(), NULL);
+    }
 }
 
 /*! \internal */
-void QDBusActionGroup::removeAction(const char *actionName)
+void QDBusActionGroup::removeAction(const char *actionName, bool erase)
 {
     Q_FOREACH(QStateAction *act, m_actions) {
         if (act->text() == actionName) {
-            m_actions.remove(act);
-            delete act;
-            Q_EMIT countChanged(m_actions.count());
+            if (erase) {
+                m_actions.remove(act);
+                delete act;
+            } else {
+                act->setValid(false);
+            }
             break;
         }
     }
@@ -236,15 +263,14 @@ void QDBusActionGroup::clear()
 void QDBusActionGroup::onActionAdded(GDBusActionGroup *, gchar *actionName, gpointer data)
 {
     QDBusActionGroup *self = reinterpret_cast<QDBusActionGroup*>(data);
-    self->addAction(actionName);
+    self->addAction(actionName, true);
 }
 
 /*! \internal */
 void QDBusActionGroup::onActionRemoved(GDBusActionGroup *, gchar *actionName, gpointer data)
 {
     QDBusActionGroup *self = reinterpret_cast<QDBusActionGroup*>(data);
-    self->removeAction(actionName);
-
+    self->removeAction(actionName, false);
 }
 
 /*! \internal */
diff --git a/src/common/qdbusactiongroup.h b/src/common/qdbusactiongroup.h
index 28e4f25..f2b2033 100644
--- a/src/common/qdbusactiongroup.h
+++ b/src/common/qdbusactiongroup.h
@@ -43,6 +43,8 @@ public:
 
     int count() const;
 
+    Q_INVOKABLE QStateAction *action(const QString &actionName);
+
 Q_SIGNALS:
     void busTypeChanged(BusType type);
     void busNameChanged(const QString &busNameChanged);
@@ -50,10 +52,11 @@ Q_SIGNALS:
     void statusChanged(ConnectionStatus status);
     void countChanged(int count);
 
+
 public Q_SLOTS:
     void start();
     void stop();
-    QStateAction *action(const QString &actionName);
+
 
 protected:
     virtual void serviceAppear(GDBusConnection *connection);
@@ -74,10 +77,11 @@ private:
     void setIntBusType(int busType);
 
     void setActionGroup(GDBusActionGroup *ag);
-    void addAction(const char *actionName);
-    void removeAction(const char *actionName);
+    QStateAction *addAction(const char *actionName, bool create);
+    void removeAction(const char *actionName, bool erase);
     void updateAction(const char *actionName, GVariant *state);
     void clear();
+    QStateAction *actionImpl(const QString &actionName);
 
     // glib slots
     static void onActionAdded(GDBusActionGroup *ag, gchar *actionName, gpointer data);
diff --git a/src/common/qmenumodel.cpp b/src/common/qmenumodel.cpp
index accc47d..2321f1c 100644
--- a/src/common/qmenumodel.cpp
+++ b/src/common/qmenumodel.cpp
@@ -70,7 +70,7 @@ void QMenuModel::setMenuModel(GMenuModel *other)
         g_object_unref(m_menuModel);
     }
 
-    m_menuModel = other;    
+    m_menuModel = other;
 
     if (m_menuModel) {
         // this will trigger the menu load
diff --git a/src/common/qstateaction.cpp b/src/common/qstateaction.cpp
index a9ecabc..8e3c7c3 100644
--- a/src/common/qstateaction.cpp
+++ b/src/common/qstateaction.cpp
@@ -58,6 +58,25 @@ QVariant QStateAction::state() const
     return m_state;
 }
 
+/*!
+    \qmlproperty int QStateAction::isValid
+    This property return if the current Action is valid or not
+    A valid Action is a action which has a DBus action linked
+*/
+bool QStateAction::isValid() const
+{
+    return m_valid;
+}
+
+/*! \internal */
+void QStateAction::setValid(bool valid)
+{
+    if (m_valid != valid) {
+        m_valid = valid;
+        Q_EMIT validChanged(m_valid);
+    }
+}
+
 /*! \internal */
 void QStateAction::setState(const QVariant &state)
 {
diff --git a/src/common/qstateaction.h b/src/common/qstateaction.h
index 3ed6bf3..4019c64 100644
--- a/src/common/qstateaction.h
+++ b/src/common/qstateaction.h
@@ -27,17 +27,26 @@ class QStateAction : public QAction
 {
     Q_OBJECT
     Q_PROPERTY(QVariant state READ state WRITE setState NOTIFY stateChanged)
+    Q_PROPERTY(bool valid READ isValid NOTIFY validChanged)
 public:
     QStateAction(const QString &text="", QObject *parent=0);
 
     QVariant state() const;
     void setState(const QVariant &state);
 
+    bool isValid() const;
+
 Q_SIGNALS:
     void stateChanged(QVariant state);
+    void validChanged(bool valid);
 
 private:
-    QVariant m_state;
+    QVariant m_state;    
+    bool m_valid;
+
+    void setValid(bool valid);
+
+    friend class QDBusActionGroup;
 };
 
 #endif
diff --git a/tests/client/actiongrouptest.cpp b/tests/client/actiongrouptest.cpp
index 6fc2f82..fa2dca5 100644
--- a/tests/client/actiongrouptest.cpp
+++ b/tests/client/actiongrouptest.cpp
@@ -64,6 +64,9 @@ private Q_SLOTS:
         m_script.unpublishMenu();
     }
 
+    /*
+     * Test if the propety busType handle correct integer values
+     */
     void testBusTypeProperty()
     {
         m_actionGroup.setProperty("busType", 1);
@@ -79,6 +82,10 @@ private Q_SLOTS:
         QCOMPARE(m_actionGroup.busType(), QDBusObject::SystemBus);
     }
 
+    /*
+     * Test if QDBusActionGroup change to correct state after DBus
+     * ervice appear
+     */
     void testServiceAppear()
     {
         m_model.start();
@@ -93,6 +100,10 @@ private Q_SLOTS:
         QCOMPARE(m_actionGroup.count(), 0);
     }
 
+    /*
+     * Test if QDBusActionGroup change to correct state after DBus
+     * service disappear
+     */
     void testServiceDisappear()
     {
         m_model.start();
@@ -116,6 +127,9 @@ private Q_SLOTS:
         QCOMPARE(m_actionGroup.status(), QDBusObject::Disconnected);
     }
 
+    /*
+     * Test if Action::trigger active the action over DBus
+     */
     void testActiveAction()
     {
         // start model
@@ -139,6 +153,9 @@ private Q_SLOTS:
         QCOMPARE(m_script.popActivatedAction(), QString("Menu1Act"));
     }
 
+    /*
+     * Test if Action became invalid after desappear from DBus
+     */
     void testRemoveAction()
     {
         // start model
@@ -150,9 +167,42 @@ private Q_SLOTS:
         m_script.walk(2);
         QCOMPARE(m_actionGroup.count(), 2);
 
+        // Get Action
+        QStateAction *act = m_actionGroup.action(QString("Menu1Act"));
+        QVERIFY(act);
+        QVERIFY(act->isValid());
+
         // Remove 1 menu
         m_script.walk(1);
-        QCOMPARE(m_actionGroup.count(), 1);
+        QCOMPARE(m_actionGroup.count(), 2);
+
+        //Check if action is invalid
+        QVERIFY(!act->isValid());
+    }
+
+    /*
+     * Test if Action became valid after service appears
+     */
+    void testActionIsValid()
+    {
+        // start model
+        m_model.start();
+        m_actionGroup.start();
+
+        // Make menu available and append 2 menus
+        m_script.publishMenu();
+
+        // Get invalid Action
+        QStateAction *act = m_actionGroup.action(QString("Menu1Act"));
+        QVERIFY(act);
+        QVERIFY(!act->isValid());
+        QVERIFY(!act->state().isValid());
+
+        // Append menus
+        m_script.walk(2);
+
+        // Action appear
+        QVERIFY(act->isValid());
     }
 };
 
diff --git a/tests/client/menuchangestest.cpp b/tests/client/menuchangestest.cpp
index c2dc66a..64c1729 100644
--- a/tests/client/menuchangestest.cpp
+++ b/tests/client/menuchangestest.cpp
@@ -56,6 +56,9 @@ private Q_SLOTS:
         m_script.unpublishMenu();
     }
 
+    /*
+     * Test it the model updates correct after remove or add a new menu
+     */
     void testMenuItemAppend()
     {
         m_script.publishMenu();
diff --git a/tests/client/modeltest.cpp b/tests/client/modeltest.cpp
index 347676e..ee3e0f4 100644
--- a/tests/client/modeltest.cpp
+++ b/tests/client/modeltest.cpp
@@ -56,16 +56,25 @@ private Q_SLOTS:
         m_script.unpublishMenu();
     }
 
+    /*
+     * Test if columnCount is always 1
+     */
     void testColumnCount()
     {
         QCOMPARE(m_model.columnCount(), 1);
     }
 
+    /*
+     * Test if parent function always return a empty QModelIndex
+     */
     void testParent()
     {
         QCOMPARE(m_model.parent(QModelIndex()), QModelIndex());
     }
 
+    /*
+     * Test if the propety busType handle correct integer values
+     */
     void testBusTypeProperty()
     {
         m_model.setProperty("busType", 1);
@@ -82,6 +91,9 @@ private Q_SLOTS:
 
     }
 
+    /*
+     * Test if model return the correct values for standard properties
+     */
     void testData()
     {
         // Make menu available
@@ -127,6 +139,9 @@ private Q_SLOTS:
         QCOMPARE(submenu->rowCount(), 2);
     }
 
+    /*
+     * Test if the model parse correct GVariant values types
+     */
     void testExtraData()
     {
         // Make menu available