messaging-framework

Merge lp:~phablet-team/messaging-framework/include-message-when-group-cancelled into lp:messaging-framework

include-message-when-group-cancelled
Merge into trunk

Proposed by Roberto Mier Escandon on 2016-08-25

Status:	Merged
Approved by:	Tiago Salem Herrmann on 2016-08-26
Approved revision:	61
Merged at revision:	59
Proposed branch:	lp:~phablet-team/messaging-framework/include-message-when-group-cancelled
Merge into:	lp:messaging-framework
Diff against target:	304 lines (+63/-89) 6 files modified debian/changelog (+7/-0) include/messaging/group_manager.h (+8/-11) include/messaging/qt/tp/text_channel.h (+9/-14) src/CMakeLists.txt (+1/-0) src/messaging/group_manager.cpp (+4/-17) src/messaging/qt/tp/text_channel.cpp (+34/-47)
To merge this branch:	bzr merge lp:~phablet-team/messaging-framework/include-message-when-group-cancelled
Related bugs:	Link a bug report

Reviewer	Review Type	Date Requested	Status
Tiago Salem Herrmann (community)		2016-08-25	Approve on 2016-08-26
system-apps-ci-bot	continuous-integration		Approve on 2016-08-26
Review via email: mp+304000@code.launchpad.net

Commit message

Simplified Api for having only one callback for all possible group cancelled cases
added more enum cases as cancel reason

NOTE: this branch should be landed along with https://code.launchpad.net/~ningbo-team/ningbo/refine-error-messages/+merge/303998

Description of the change

Simplified Api for having only one callback for all possible group cancelled cases
added more enum cases as cancel reason

NOTE: this branch should be landed along with https://code.launchpad.net/~ningbo-team/ningbo/refine-error-messages/+merge/303998

Revision history for this message

system-apps-ci-bot (system-apps-ci-bot) wrote on 2016-08-26:

Click here to trigger a rebuild:
https://jenkins.canonical.com/system-apps/job/lp-messaging-framework-ci/45/rebuild

review: Approve (continuous-integration)

lp:~phablet-team/messaging-framework/include-message-when-group-cancelled updated on 2016-08-26

61. By Roberto Mier Escandon on 2016-08-26: delay the close to let the events get to the client before the channel is closed

Revision history for this message

system-apps-ci-bot (system-apps-ci-bot) wrote on 2016-08-26:

Click here to trigger a rebuild:
https://jenkins.canonical.com/system-apps/job/lp-messaging-framework-ci/46/rebuild

review: Approve (continuous-integration)

Revision history for this message

Tiago Salem Herrmann (tiagosh) wrote on 2016-08-26:

tested and works fine.
code looks good too.

review: Approve

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk

Subscribers

People subscribed via source and target branches

to all changes:

Ubuntu Phablet Team

 === modified file 'debian/changelog'
 --- debian/changelog	2016-08-16 20:11:02 +0000
 +++ debian/changelog	2016-08-26 14:12:00 +0000
@@ -1,3 +1,10 @@
++messaging-framework (0.1-1ubuntu25) vivid; urgency=medium
++
++  * Fixed self handle included in members
++  * Simplified group cancelled api callbacks reducing them to only one
++
++ -- Roberto Mier Escandon <roberto.escandon@canonical.com>  Thu, 25 Aug 2016 22:06:07 +0200
++
  messaging-framework (0.1-1ubuntu24) vivid; urgency=medium
    * Implemented code to update group member roles in client
 === modified file 'include/messaging/group_manager.h'
 --- include/messaging/group_manager.h	2016-08-23 22:33:39 +0000
 +++ include/messaging/group_manager.h	2016-08-26 14:12:00 +0000
@@ -42,10 +42,12 @@
  enum class CancelGroupReason
+ {
--    None,
--    Kicked,
--    Gone,
--    Error
++    None,       ///< No or unknown reason
++    Error,      ///< Generic error makes this group temporally or permanent unavaliable for us
++    Rejected,   ///< Group creation or rejoin was rejected
++    Kicked,     ///< We were expelled from group
++    Gone,       ///< Group is gone in server side and is not available anymore
++    Leave       ///< We leave the group
  };
  /// @brief GroupManager models a textual group conversation.
@@ -59,9 +61,7 @@
          virtual ~Observer() = default;
          virtual void on_group_created() = 0;
--        virtual void on_group_creation_rejected() = 0;
--        virtual void on_group_cancelled(CancelGroupReason reason) = 0;
--        virtual void on_group_quit() = 0;
++        virtual void on_group_cancelled(CancelGroupReason reason, const std::string &message) = 0;
          virtual void on_group_title_changed(const std::string& new_title,
                                              const Member& actor,
                                              const std::chrono::system_clock::time_point& when) = 0;
@@ -116,10 +116,7 @@
      void validate_observer();
      void announce_group_created();
--    //TODO TRACE maybe a reason as parameter for knowing the rejected cause?
--    void announce_group_creation_rejected();
--    void announce_group_cancelled(CancelGroupReason reason);
--    void announce_group_quit();
++    void announce_group_cancelled(CancelGroupReason reason, const std::string& message);
      void announce_group_title_changed(const std::string& new_title,
                                        const Member& actor = Member{std::string{}},
                                        const std::chrono::system_clock::time_point& when = std::chrono::system_clock::now());
 === modified file 'include/messaging/qt/tp/text_channel.h'
 --- include/messaging/qt/tp/text_channel.h	2016-08-16 16:31:31 +0000
 +++ include/messaging/qt/tp/text_channel.h	2016-08-26 14:12:00 +0000
@@ -20,7 +20,11 @@
  #include <messaging/chat.h>
  #include <messaging/messenger.h>
  #include <messaging/qt/runtime.h>
++
++#include <messaging/qt/tp/interfaces/base_channel_destroyable.h>
++#include <messaging/qt/tp/interfaces/base_channel_roles.h>
  #include <messaging/qt/tp/interfaces/types.h>
++#include <messaging/qt/tp/interfaces/constants.h>
  #include <TelepathyQt/BaseChannel>
  #include <TelepathyQt/Types>
@@ -45,6 +49,7 @@
          , public messaging::Chat::Observer
          , public messaging::GroupManager::Observer
+ {
++    Q_OBJECT
  public:
      /// @brief Safes us some typing.
      typedef Tp::SharedPtr<TextChannel> Ptr;
@@ -72,14 +77,8 @@
      /// @brief on_group_created when the group has been created ok in the server
      void on_group_created() override;
--    /// @brief on_group_creation_rejected when the group creation has been rejected by the server
--    void on_group_creation_rejected() override;
--
      /// @brief on_group_cancelled when the group is no longer valid
--    void on_group_cancelled(CancelGroupReason reason) override;
--
--    /// @brief on_group_quit when confirmed leaving a group
--    void on_group_quit() override;
++    void on_group_cancelled(CancelGroupReason reason, const std::string &message) override;
      /// @brief on_group_title_changed when group title has changed in the server
      void on_group_title_changed(const std::string& new_title,
@@ -107,6 +106,8 @@
      /// @brief on_participant_ends_typing is invoked when a participant in this chat ends composing a message
      void on_participant_ends_typing(const std::shared_ptr<User> &user) override;
++public Q_SLOTS:
++    void delay_close();
  private:
      TextChannel(Connection* tp_connection,
@@ -143,14 +144,8 @@
          /// @brief on_group_created when the group has been created ok in the server
          void on_group_created() override;
--        /// @brief on_group_creation_rejected when the group creation has been rejected by the server
--        void on_group_creation_rejected() override;
--
          /// @brief on_group_cancelled when the group is no longer valid
--        void on_group_cancelled(CancelGroupReason reason) override;
--
--        /// @brief on_group_quit when confirmed leaving a group
--        void on_group_quit() override;
++        void on_group_cancelled(CancelGroupReason reason, const std::string &message) override;
          /// @brief on_group_title_changed when group title has changed in the server
          void on_group_title_changed(const std::string& new_title,
 === modified file 'src/CMakeLists.txt'
 --- src/CMakeLists.txt	2016-08-11 21:39:57 +0000
 +++ src/CMakeLists.txt	2016-08-26 14:12:00 +0000
@@ -4,6 +4,7 @@
  qt5_wrap_cpp(MESSAGING_FW_MOCS ${CMAKE_SOURCE_DIR}/include/messaging/qt/network_monitor.h
                                 ${CMAKE_SOURCE_DIR}/include/messaging/qt/tp/protocol.h
                                 ${CMAKE_SOURCE_DIR}/include/messaging/qt/tp/connection.h
++                               ${CMAKE_SOURCE_DIR}/include/messaging/qt/tp/text_channel.h
                                 ${CMAKE_SOURCE_DIR}/include/messaging/qt/tp/interfaces/base_channel_destroyable.h
                                 ${CMAKE_SOURCE_DIR}/include/messaging/qt/tp/interfaces/base_channel_destroyable_internal.h
                                 ${CMAKE_SOURCE_DIR}/include/messaging/qt/tp/interfaces/base_channel_roles.h
 === modified file 'src/messaging/group_manager.cpp'
 --- src/messaging/group_manager.cpp	2016-08-16 14:47:42 +0000
 +++ src/messaging/group_manager.cpp	2016-08-26 14:12:00 +0000
@@ -58,23 +58,10 @@
      impl->observer->on_group_created();
+ }
--void messaging::GroupManager::announce_group_creation_rejected()
--{
--    validate_observer();
--    impl->observer->on_group_creation_rejected();
--}
--
--
--void messaging::GroupManager::announce_group_cancelled(CancelGroupReason reason)
--{
--    validate_observer();
--    impl->observer->on_group_cancelled(reason);
--}
--
--void messaging::GroupManager::announce_group_quit()
--{
--    validate_observer();
--    impl->observer->on_group_quit();
++void messaging::GroupManager::announce_group_cancelled(CancelGroupReason reason, const std::string &message)
++{
++    validate_observer();
++    impl->observer->on_group_cancelled(reason, message);
+ }
  void messaging::GroupManager::announce_group_title_changed(const std::string &new_title,
 === modified file 'src/messaging/qt/tp/text_channel.cpp'
 --- src/messaging/qt/tp/text_channel.cpp	2016-08-24 20:10:27 +0000
 +++ src/messaging/qt/tp/text_channel.cpp	2016-08-26 14:12:00 +0000
@@ -19,11 +19,6 @@
  #include <messaging/qt/tp/connection.h>
  #include <messaging/qt/tp/message.h>
--#include <messaging/qt/tp/interfaces/base_channel_destroyable.h>
--#include <messaging/qt/tp/interfaces/base_channel_roles.h>
--#include <messaging/qt/tp/interfaces/types.h>
--#include <messaging/qt/tp/interfaces/constants.h>
--
  #include <messaging/qt/variant.h>
  #include <messaging/connection.h>
@@ -178,27 +173,11 @@
          });
+ }
--void mqt::tp::TextChannel::Observer::on_group_creation_rejected()
--{
--    auto sp = shared_from_this();
--    runtime->enter_with_task([sp](){
--            sp->tc->on_group_creation_rejected();
--        });
--}
--
--void mqt::tp::TextChannel::Observer::on_group_cancelled(CancelGroupReason reason)
--{
--    auto thiz = shared_from_this();
--    runtime->enter_with_task([thiz, reason]() {
--            thiz->tc->on_group_cancelled(reason);
--        });
--}
--
--void mqt::tp::TextChannel::Observer::on_group_quit()
--{
--    auto thiz = shared_from_this();
--    runtime->enter_with_task([thiz]() {
--            thiz->tc->on_group_quit();
++void mqt::tp::TextChannel::Observer::on_group_cancelled(CancelGroupReason reason, const std::string &message)
++{
++    auto thiz = shared_from_this();
++    runtime->enter_with_task([thiz, reason, message]() {
++            thiz->tc->on_group_cancelled(reason, message);
          });
+ }
@@ -612,12 +591,6 @@
+ }
--void mqt::tp::TextChannel::on_group_creation_rejected()
--{
--    LOG(INFO) << __PRETTY_FUNCTION__ ;
--    close_channel_with_reason(Tp::ChannelGroupChangeReasonError, "Group creation rejected");
--}
--
  void mqt::tp::TextChannel::close_channel_with_reason(uint reason, const QString &message)
+ {
      // At this point, self handle should be removed from the group members, providing one of the reasons
@@ -637,11 +610,18 @@
      group_interface->setMembers(members, details);
++    // It is needed to delay the close channel action to be sure the events related with previous action of removing
++    // self handle from members are sent to clients and processed before the channel expires.
++    QTimer::singleShot(2000, this, SLOT(delay_close()));
++
++}
++
++void mqt::tp::TextChannel::delay_close()
++{
      close();
+ }
--
--void mqt::tp::TextChannel::on_group_cancelled(CancelGroupReason cancel_reason)
++void mqt::tp::TextChannel::on_group_cancelled(CancelGroupReason cancel_reason, const std::string &cancel_message)
+ {
      LOG(INFO) << __PRETTY_FUNCTION__;
@@ -649,18 +629,25 @@
      QString message;
      switch (cancel_reason)
+     {
++    case CancelGroupReason::Error:
++        reason = Tp::ChannelGroupChangeReasonError;
++        message = "Group is closed due to an error";
++        break;
++    case CancelGroupReason::Rejected:
++        reason = Tp::ChannelGroupChangeReasonError;
++        message = "Group creation rejected";
++        break;
++    case CancelGroupReason::Kicked:
++        reason = Tp::ChannelGroupChangeReasonKicked;
++        message = "You were expelled from group";
++        break;
      case CancelGroupReason::Gone:
          reason = Tp::ChannelGroupChangeReasonBanned;
          message = "Group not available anymore";
          break;
--    case CancelGroupReason::Kicked:
--        reason = Tp::ChannelGroupChangeReasonKicked;
--        message = "You were expelled from group";
--        break;
--    case CancelGroupReason::Error:
--        reason = Tp::ChannelGroupChangeReasonError;
--        message = "Group is closed due to an error";
--        break;
++    case CancelGroupReason::Leave:
++        reason = Tp::ChannelGroupChangeReasonNone;
++        message = "Quit group";
      case CancelGroupReason::None:
      default:
          reason = Tp::ChannelGroupChangeReasonNone;
@@ -668,14 +655,14 @@
          break;
+     }
++    // cancel message overwrites default one in case it is not empty
++    if (not cancel_message.empty()) {
++        message = QString::fromStdString(cancel_message);
++    }
++
      close_channel_with_reason(reason, message);
+ }
--void mqt::tp::TextChannel::on_group_quit()
--{
--    close_channel_with_reason(Tp::ChannelGroupChangeReasonNone, "Quit group");
--}
--
  void mqt::tp::TextChannel::on_group_title_changed(const std::string &new_title,
                                                    const Member& actor,
                                                    const std::chrono::system_clock::time_point& when)