feat: introduce CommunityAdminSettings in CommunityDescription
#2668
+456
−374
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
0x-r4bbit
requested review from
richard-ramos and
cammellos
and removed request for
richard-ramos
Pull Request Checklist
|
Jenkins BuildsClick to see older builds (29)
|
0x-r4bbit
force-pushed
the
feat/enable-pin-message-members
branch
from
8d72710
to
931dd57
Compare
0x-r4bbit
added a commit
to status-im/status-desktop
that referenced
this pull request
May 10, 2022
This adds a checkbox to configure whether or not community members are allowed to pin any message of a community channel, based on the newly introduced `CommunityAdminSettings` that are introduced in status-im/status-go#2668 Closes #5662
0x-r4bbit
force-pushed
the
feat/enable-pin-message-members
branch
from
931dd57
to
13b2047
Compare
jrainville
reviewed
May 10, 2022
protocol/messenger_handler.go
Outdated
| isMemberAdmin := community.IsMemberAdmin(chatEntity.GetSigPubKey()) | ||
| pinMessageAllowed := community.AllowsAllMembersToPinMessage() | ||
|
|
||
| if (!isMemberAdmin && !pinMessageAllowed) || (!emojiReaction && !canPost) { |
There was a problem hiding this comment.
Should there be an additionnal check on pin? It seems like this might block people from posting normal messages as well.
Something like
(isPinMessage && !isMemberAdmin && !pinMessageAllowed)Where isPinMessage comes from a new case in the switch above.
There was a problem hiding this comment.
Ah, forgot to add that. In my head I was still inside HandlePinMessage...
Thanks man, will update
0x-r4bbit
force-pushed
the
feat/enable-pin-message-members
branch
2 times, most recently
from
76c200e
to
687247b
Compare
Samyoul
approved these changes
May 12, 2022
0x-r4bbit
force-pushed
the
feat/enable-pin-message-members
branch
from
687247b
to
1246821
Compare
0x-r4bbit
added a commit
to status-im/status-desktop
that referenced
this pull request
May 13, 2022
This adds a checkbox to configure whether or not community members are allowed to pin any message of a community channel, based on the newly introduced `CommunityAdminSettings` that are introduced in status-im/status-go#2668 Closes #5662
0x-r4bbit
force-pushed
the
feat/enable-pin-message-members
branch
2 times, most recently
from
b2f6fe1
to
194b120
Compare
|
Tests are failing but unrelated to these changes. Also, I think status-im/status-desktop#5729 is related. Investigating... |
|
So it seems it affects all tests that involve |
Comment on lines
+47
to
52
| CommunityAdminSettings admin_settings = 10; | ||
| } | ||
|
|
||
| message CommunityAdminSettings { | ||
| bool pin_message_all_members_enabled = 1; | ||
| } |
richard-ramos
approved these changes
May 13, 2022
0x-r4bbit
force-pushed
the
feat/enable-pin-message-members
branch
3 times, most recently
from
3336b4a
to
459b456
Compare
This allows to store community admin settings that are meant to be propagated to community members (as opposed to the already existing `CommunitySettings` which are considered local to every account). The first setting introduced as part of this commit is one that enables community admins to configure whether or not members of the community are allowed to pin messages in community channels. Prior to this commit, this was not restricted at all on the protocol level and only enforced by clients via UI (e.g. members don't see an option to pin messages, although they could). This config setting now ensures that: 1. If turned off, members cannot send a pin message 2. If turned off, pin messages from members are not handled/processed This is needed by status-im/status-desktop#5662
0x-r4bbit
force-pushed
the
feat/enable-pin-message-members
branch
from
459b456
to
6695097
Compare
|
Hello! |
|
@churik That one is on me! |
|
@PascalPrecht |
0x-r4bbit
added a commit
to status-im/status-desktop
that referenced
this pull request
May 19, 2022
This adds a checkbox to configure whether or not community members are allowed to pin any message of a community channel, based on the newly introduced `CommunityAdminSettings` that are introduced in status-im/status-go#2668 Closes #5662
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This allows to store community admin settings that are meant to be propagated
to community members (as opposed to the already existing
CommunitySettingswhich are considered local to every account).The first setting introduced as part of this commit is one that enables
community admins to configure whether or not members of the community
are allowed to pin messages in community channels.
Prior to this commit, this was not restricted at all on the protocol
level and only enforced by clients via UI (e.g. members don't see an
option to pin messages, although they could).
This config setting now ensures that:
This is needed by status-im/status-desktop#5662