messaging-framework

Merge lp:~phablet-team/messaging-framework/last-minute-fixes into lp:messaging-framework

  1. last-minute-fixes
  2. Merge into trunk
Proposed by Roberto Mier Escandon
Status: Merged
Approved by: Tiago Salem Herrmann
Approved revision: 75
Merged at revision: 68
Proposed branch: lp:~phablet-team/messaging-framework/last-minute-fixes
Merge into: lp:messaging-framework
Prerequisite: lp:~phablet-team/messaging-framework/self-id
Diff against target: 136 lines (+29/-14)
5 files modified
include/messaging/group_manager.h (+3/-0)
include/messaging/qt/tp/text_channel.h (+1/-0)
src/messaging/qt/tp/connection.cpp (+5/-4)
src/messaging/qt/tp/text_channel.cpp (+19/-10)
tests/mock_group_manager.h (+1/-0)
To merge this branch: bzr merge lp:~phablet-team/messaging-framework/last-minute-fixes
Reviewer Review Type Date Requested Status
system-apps-ci-bot continuous-integration Approve
Tiago Salem Herrmann (community) Approve
Review via email: mp+308008@code.launchpad.net

Commit message

Bunch of last minute fixes:
- adding selfID along with selfHandle
- using connection->group_admin_ids to know who are the admins, and compare with selfID to know if we are admin also, instead of querying existing members and guess.

NOTE: This branch should be landed along with https://code.launchpad.net/~ningbo-team/ningbo/last-minute-fixes/+merge/308007

Description of the change

Bunch of last minute fixes:
- adding selfID along with selfHandle
- using connection->group_admin_ids to know who are the admins, and compare with selfID to know if we are admin also, instead of querying existing members and guess.

NOTE: This branch should be landed along with https://code.launchpad.net/~ningbo-team/ningbo/last-minute-fixes/+merge/308007

To post a comment you must log in.
Revision history for this message
system-apps-ci-bot (system-apps-ci-bot) wrote :

FAILED: Continuous integration, rev:75
https://jenkins.canonical.com/system-apps/job/lp-messaging-framework-ci/87/
Executed test runs:
    FAILURE: https://jenkins.canonical.com/system-apps/job/build/1834/console
    SUCCESS: https://jenkins.canonical.com/system-apps/job/build-0-fetch/1835
    SUCCESS: https://jenkins.canonical.com/system-apps/job/build-2-binpkg/arch=amd64,release=vivid+overlay/1675
        deb: https://jenkins.canonical.com/system-apps/job/build-2-binpkg/arch=amd64,release=vivid+overlay/1675/artifact/output/*zip*/output.zip
    SUCCESS: https://jenkins.canonical.com/system-apps/job/build-2-binpkg/arch=amd64,release=xenial+overlay/1675
        deb: https://jenkins.canonical.com/system-apps/job/build-2-binpkg/arch=amd64,release=xenial+overlay/1675/artifact/output/*zip*/output.zip
    SUCCESS: https://jenkins.canonical.com/system-apps/job/build-2-binpkg/arch=amd64,release=yakkety/1675
        deb: https://jenkins.canonical.com/system-apps/job/build-2-binpkg/arch=amd64,release=yakkety/1675/artifact/output/*zip*/output.zip
    SUCCESS: https://jenkins.canonical.com/system-apps/job/build-2-binpkg/arch=armhf,release=vivid+overlay/1675
        deb: https://jenkins.canonical.com/system-apps/job/build-2-binpkg/arch=armhf,release=vivid+overlay/1675/artifact/output/*zip*/output.zip
    SUCCESS: https://jenkins.canonical.com/system-apps/job/build-2-binpkg/arch=armhf,release=xenial+overlay/1675
        deb: https://jenkins.canonical.com/system-apps/job/build-2-binpkg/arch=armhf,release=xenial+overlay/1675/artifact/output/*zip*/output.zip
    SUCCESS: https://jenkins.canonical.com/system-apps/job/build-2-binpkg/arch=armhf,release=yakkety/1675
        deb: https://jenkins.canonical.com/system-apps/job/build-2-binpkg/arch=armhf,release=yakkety/1675/artifact/output/*zip*/output.zip
    SUCCESS: https://jenkins.canonical.com/system-apps/job/build-2-binpkg/arch=i386,release=vivid+overlay/1675
        deb: https://jenkins.canonical.com/system-apps/job/build-2-binpkg/arch=i386,release=vivid+overlay/1675/artifact/output/*zip*/output.zip
    SUCCESS: https://jenkins.canonical.com/system-apps/job/build-2-binpkg/arch=i386,release=xenial+overlay/1675
        deb: https://jenkins.canonical.com/system-apps/job/build-2-binpkg/arch=i386,release=xenial+overlay/1675/artifact/output/*zip*/output.zip
    FAILURE: https://jenkins.canonical.com/system-apps/job/build-2-binpkg/arch=i386,release=yakkety/1675/console

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

review: Needs Fixing (continuous-integration)
Revision history for this message
Tiago Salem Herrmann (tiagosh) wrote :

there was only one bug leading to a crash when using broadcast chats, but it is now fixed.
thanks.

review: Approve
Revision history for this message
system-apps-ci-bot (system-apps-ci-bot) :
review: Approve (continuous-integration)

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'include/messaging/group_manager.h'
2--- include/messaging/group_manager.h 2016-09-14 07:44:10 +0000
3+++ include/messaging/group_manager.h 2016-10-12 14:43:53 +0000
4@@ -105,6 +105,9 @@
5 /// @brief creator returns the creator of the group being managed
6 virtual std::shared_ptr<Member> group_creator() = 0;
7
8+ /// @brief group_admins returns the list of admin identifiers
9+ virtual std::set<std::string> group_admins() = 0;
10+
11 /// @brief members returns current participants in the group.
12 virtual Members members() = 0;
13
14
15=== modified file 'include/messaging/qt/tp/text_channel.h'
16--- include/messaging/qt/tp/text_channel.h 2016-09-07 08:22:52 +0000
17+++ include/messaging/qt/tp/text_channel.h 2016-10-12 14:43:53 +0000
18@@ -211,6 +211,7 @@
19 void register_callbacks_once();
20 void plug_interfaces_once();
21 void plug_interface_if_available(const Tp::AbstractChannelInterfacePtr &interface);
22+ bool self_is_admin();
23
24 Tp::BaseChannelTextTypePtr text_type_interface;
25 Tp::BaseChannelMessagesInterfacePtr messages_interface;
26
27=== modified file 'src/messaging/qt/tp/connection.cpp'
28--- src/messaging/qt/tp/connection.cpp 2016-10-12 14:43:53 +0000
29+++ src/messaging/qt/tp/connection.cpp 2016-10-12 14:43:53 +0000
30@@ -155,7 +155,8 @@
31 qRegisterMetaType<messaging::Recipient::shared_ptr>();
32
33 setSelfHandle(1);
34- handles_.insert(HandleIdPair(1, QString::fromStdString(connection->self_identifier())));
35+ setSelfID(QString::fromStdString(connection->normalize_identifier(connection->self_identifier())));
36+ handles_.insert(HandleIdPair(selfHandle(), selfID()));
37
38 // Wire up the actual request for connecting.
39 setConnectCallback(Tp::memFun(this, &tp::Connection::connect));
40@@ -594,7 +595,7 @@
41 {
42 std::shared_ptr<GroupStarter> group_starter = connection->group_starter();
43 group_manager = group_starter->accept_group(group);
44- if (group_manager->group_id().empty())
45+ if (not group_manager || group_manager->group_id().empty())
46 {
47 LOG(ERROR) << "Group creation rejected";
48 error->set(TP_QT_ERROR_CANCELLED, "Group creation rejected");
49@@ -649,7 +650,7 @@
50 messaging::Group::shared_ptr group = std::dynamic_pointer_cast<messaging::Group>(recipient);
51 group_manager = group_starter->rejoin_group(group);
52
53- if (group_manager->group_id().empty())
54+ if (not group_manager || group_manager->group_id().empty())
55 {
56 LOG(ERROR) << "Group rejoin rejected";
57 error->set(TP_QT_ERROR_CANCELLED, "Group rejoin rejected");
58@@ -761,7 +762,7 @@
59 messaging::Group::shared_ptr group = std::dynamic_pointer_cast<messaging::Group>(recipient);
60 group_manager = group_starter->create_group(group);
61
62- if (group_manager->group_id().empty())
63+ if (not group_manager || group_manager->group_id().empty())
64 {
65 LOG(ERROR) << "Group creation rejected";
66 error->set(TP_QT_ERROR_CANCELLED, "Group creation rejected");
67
68=== modified file 'src/messaging/qt/tp/text_channel.cpp'
69--- src/messaging/qt/tp/text_channel.cpp 2016-10-12 14:43:53 +0000
70+++ src/messaging/qt/tp/text_channel.cpp 2016-10-12 14:43:53 +0000
71@@ -757,10 +757,6 @@
72 QStringList member_ids;
73 QStringList remote_member_ids;
74 interfaces::HandleRolesMap roles_map;
75- // flag assuming there is at least one admin (See if this is happening in all protocols and if this solution
76- // is valid for all them). In case any of the participants have admin role, we set it to self handle and add
77- // that to updating roles.
78- bool found_admin = false;
79
80 for (auto member : received_members) {
81
82@@ -800,11 +796,6 @@
83 }
84 }
85
86- // search for admin in current member roles
87- if (member->roles().is_set(Role::Admin)) {
88- found_admin = true;
89- }
90-
91 // add current member roles to map
92 roles_map[handle] = member->roles().underlying_type();
93 }
94@@ -816,7 +807,7 @@
95
96 // if not found admin, add admin role to ourselves
97 interfaces::ChannelRoles self_roles(interfaces::ChannelRole::ChannelRoleMember);
98- if (not found_admin) {
99+ if (self_is_admin()) {
100 self_roles |= interfaces::ChannelRole::ChannelRoleAdmin;
101 }
102 roles_map[tp_connection->selfHandle()] = static_cast<uint>(self_roles);
103@@ -1191,3 +1182,21 @@
104 }
105 plugInterface(interface);
106 }
107+
108+bool mqt::tp::TextChannel::self_is_admin()
109+{
110+ std::shared_ptr<messaging::GroupManager> group_manager = chat->interface<GroupManager>();
111+ if (!group_manager) {
112+ return false;
113+ }
114+ std::set<std::string> admins = group_manager->group_admins();
115+ std::string self_id = tp_connection->selfID().toStdString();
116+
117+ for (const std::string &admin : admins) {
118+ if (admin == self_id) {
119+ return true;
120+ }
121+ }
122+
123+ return false;
124+}
125
126=== modified file 'tests/mock_group_manager.h'
127--- tests/mock_group_manager.h 2016-09-14 07:36:33 +0000
128+++ tests/mock_group_manager.h 2016-10-12 14:43:53 +0000
129@@ -48,6 +48,7 @@
130 MOCK_METHOD0(group_title, std::string());
131 MOCK_METHOD0(group_subject, std::string());
132 MOCK_METHOD0(group_creator, std::shared_ptr<messaging::Member>());
133+ MOCK_METHOD0(group_admins, std::set<std::string>());
134 MOCK_METHOD0(members, messaging::Members());
135 MOCK_METHOD1(set_observer, void(const std::shared_ptr<Observer>&));
136 MOCK_METHOD1(on_plugged, void(const std::weak_ptr<messaging::HasInterfaces>&));

Subscribers

People subscribed via source and target branches

to all changes: