[Merge] lp:~phablet-team/messaging-framework/group-own-permissions into lp:messaging-framework
Tiago Salem Herrmann
tiago.herrmann at canonical.com
Fri Aug 12 21:34:21 UTC 2016
Review: Needs Fixing
Looks good, just two comments.
Diff comments:
>
> === modified file 'include/messaging/group_manager.h'
> --- include/messaging/group_manager.h 2016-07-01 13:46:08 +0000
> +++ include/messaging/group_manager.h 2016-08-10 11:47:46 +0000
> @@ -23,12 +23,22 @@
> #include <messaging/interface.h>
> #include <messaging/member.h>
> #include <messaging/members.h>
> +#include <messaging/flags.h>
>
> #include <memory>
> #include <chrono>
>
> namespace messaging
> {
> +// Maintain sequence of 2 multiples if adding new values to preserve flags management
> +enum class GroupPermissions : uint
> +{
> + CanChangeTitle = 1 << 1,
> + CanKick = 1 << 2,
> + CanSetAdmin = 1 << 4,
I think CanSetAdmin should be 1 << 3 (which is 8) and CanDissolve should be 1 << 4 (which is 16). Isn't it?
> + CanDissolve = 1 << 8
> +};
> +
> /// @brief GroupManager models a textual group conversation.
> class MESSAGING_FW_PUBLIC GroupManager : public Interface, NonCopyable, NonMovable
> {
>
> === modified file 'src/messaging/qt/tp/text_channel.cpp'
> --- src/messaging/qt/tp/text_channel.cpp 2016-08-03 15:30:10 +0000
> +++ src/messaging/qt/tp/text_channel.cpp 2016-08-10 11:47:46 +0000
> @@ -347,7 +355,7 @@
> }
>
> room_config_interface->setConfigurationRetrieved(true);
> - room_config_interface->setCanUpdateConfiguration(true);
> + room_config_interface->setCanUpdateConfiguration(group_manager->permissions().is_set(GroupPermissions::CanChangeTitle));
I don't think this is quite correct, but on the other hand I don't know the proper way to fix this, since not all properties in the Room Configuration can be updated in this protocol. I think we can keep like this for now. Maybe just add a comment?
>
> // FIXME: check what flags we want by default
> Tp::ChannelGroupFlags groupFlags = Tp::ChannelGroupFlagCanAdd |
--
https://code.launchpad.net/~phablet-team/messaging-framework/group-own-permissions/+merge/302212
Your team Ubuntu Phablet Team is subscribed to branch lp:messaging-framework.
More information about the Ubuntu-reviews
mailing list