Allow members to self assign roles through a persistent view #2341

shtlrs · 2022-11-26T18:49:47Z

As stated by Zig, subscribing to our public roles is currently not super visible since it revolves around executing the !subscribe command that usually goes by unnoticed.

To make this easier, more visible & to align with other communities have, we decided to add a persistent view in the #roles channel that allows people to subscribing to their chosen roles by a click of a button.

…AssignableRolesButton button Note that these are still dummy views & have no behavior

This comes with another dependency to try to locate the message that needs to hold the view. If that message is not found, a new one will be created & sent.

… button

…the appropriate roles

…sButton

The messages were intended for a button that will automatically assign all availables roles to the interactor This changes since it's not the intended behavior

…ction

…nable_roles_view

This also enhances the docstrings for a better clarification of its purpose

This documents the attributes the class has, which gives better context for future readers/contributors

mbaruh

Looks pretty good! this is purely a code review, will test later.

bot/exts/info/subscribe.py

config-default.yml

mbaruh

misclick 😅

This also renames to method to align more with our fetch or {action} pattern

This will help clarify the relationship between the button & the viw since it'll only be used there

…ct with

…inside the view itself

onerandomusername · 2022-12-04T18:19:43Z

I know I'm late but what about a button that shows an ephemeral view which takes into consideration what roles the user already has, so they can select all of the roles they want in one single go?

shtlrs · 2022-12-04T18:26:31Z

But that's similar to what we already have, isn't it? or did I understand that wrong?

The ones they have are highlighted in red, and the message states that just fine, e.g. Whether the role will be added or removed.

And how would they select them all in one go?

onerandomusername · 2022-12-05T05:21:06Z

But that's similar to what we already have, isn't it? or did I understand that wrong?

The ones they have are highlighted in red, and the message states that just fine, e.g. Whether the role will be added or removed.

And how would they select them all in one go?

This is what I have for roles in my guilds.

This implementation uses a slash command to show the select menu, but it could be anything: like a button in the #roles channel which shows this select.

This is nicer from a UX perspective IMO because it shows which roles the user already has, and while its true the button view does communicate this fact, the button view is a bit confusing and does not allow for descriptions.

shtlrs · 2022-12-05T06:26:00Z

Hummm, i guess this could be discussed in another issue.
If it is going to be implemented, I think it should be done in another separate PR.

The current goal is to keep the same behavior for the subscribe command and add more visibility by '' pinning '' it in the roles channel.

I like the idea, and if approved, i will be more than happy to do it.

mbaruh · 2022-12-05T13:29:52Z

Let's keep this PR focused on generating the permanent view. We can discuss changing the subscription UI in a separate issue if you want to open one @onerandomusername

mbaruh

Looks great, just a few nitpicks

bot/bot/exts/info/subscribe.py

Line 101 in 91a869e

LABEL_FORMAT = "{action} role {role_name}."

Let's get rid of that dot at the end while we're at it.

bot/exts/info/subscribe.py

Dismissed because the approval was invalidated by another commit

Robin5605

Nothing major. Thanks for the PR! I haven't tested it either, so I'm hoping we didn't letting anything slip by haha 😄

Robin5605 · 2023-01-13T21:30:58Z

bot/exts/info/subscribe.py

+
+    SELF_ASSIGNABLE_ROLES_MESSAGE = (
+        f"Hi there {GREETING_EMOJI},"
+        "\nWe have self-assignable roles for server updates an events!"


I'm not sure what this sentence is supposed to mean entirely. Perhaps it could be reworded to be concise?

We're just trying to let people know what these roles are for.

That's supposed to be an "and", it's a typo.
Do you still think it's unclear then ? If yes, a suggestion is appreciated :D

Robin5605 · 2023-01-13T21:35:06Z

bot/exts/info/subscribe.py

+        async for message in roles_channel.history(limit=30):
+            if message.content == self.SELF_ASSIGNABLE_ROLES_MESSAGE:
+                log.debug(f"Found self assignable roles view message: {message.id}")
+                return message, None


Is there a reason we are returning None for the discord.ui.View? Couldn't we just use discord.ui.View.from_message?

We can't really recreate the view using from_message, as this persistent view needs to be instantiated with the self assignable roles, and from_message cannot pick that up.

Robin5605

Great job, thanks! I have no more suggestions 😃

shtlrs added 18 commits November 26, 2022 18:58

add the roles channel to the config

3686f60

add the AllSelfAssignableRolesView and its corresponding ClaimAllSelf…

5795140

…AssignableRolesButton button Note that these are still dummy views & have no behavior

add the logic for attaching the persistent view

6ab9f9c

This comes with another dependency to try to locate the message that needs to hold the view. If that message is not found, a new one will be created & sent.

add assignable_roles as a property to the ClaimAllSelfAssignableRoles…

b25ad80

… button

add implementation of the button's callback that'll handle assigning …

1d60403

…the appropriate roles

rely on original_message to delete view once it times out

af58814

restore original value of DELETE_MESSAGE_AFTER

3cedc99

rename ClaimAllSelfAssignableRolesButton to ShowAllSelfAssignableRole…

6c61d34

…sButton

update message content of the persistent view

bde96fd

The messages were intended for a button that will automatically assign all availables roles to the interactor This changes since it's not the intended behavior

fix docstrings of the ShowAllSelfAssignableRolesButton's callback fun…

dd4903c

…ction

rename prepare_available_role_subscription_view to prepare_self_assig…

bd4b0b4

…nable_roles_view

update docs for the newly added view & button

54dfcf9

rename the method that attaches the persistent view

28c733c

This also enhances the docstrings for a better clarification of its purpose

remove waring in setup docstrings

c628722

misc style & doc improvements

f748ceb

add docstrings to RoleButtonView

c14e75a

This documents the attributes the class has, which gives better context for future readers/contributors

make the roles view ephemeral when sent in roles channel

e7d0987

Merge branch 'main' into 2332-permanent-role-view

d8850b4

shtlrs marked this pull request as ready for review November 27, 2022 22:03

shtlrs requested review from Den4200 and jb3 as code owners November 27, 2022 22:03

shtlrs changed the title ~~2332 permanent role view~~ Allow members to self assign roles through a persistent view Nov 28, 2022

minalike requested a review from mbaruh November 28, 2022 20:22

mbaruh previously approved these changes Dec 3, 2022

View reviewed changes

mbaruh requested changes Dec 3, 2022

View reviewed changes

shtlrs added 5 commits December 3, 2022 23:37

do not use name mangling

0a60b5d

This also renames to method to align more with our fetch or {action} pattern

call super without referencing the current class

28dac62

rename #Roles section to #Information

72b2f80

use button decorator instead of subclassing discord.ui.Button

24c36e5

This will help clarify the relationship between the button & the viw since it'll only be used there

update custom id of the AllSelfAssignableRolesView's button

a6fa4b1

shtlrs added 5 commits December 4, 2022 11:19

make SELF_ASSIGNABLE_ROLES_MESSAGE more inviting for people to intera…

c630e1b

…ct with

ditch prepare_self_assignable_roles_view by constructing the buttons …

37b43a6

…inside the view itself

relay the newly created view to __attach_persistent_roles_view

6c6b2bc

edit view through interaction.response.edit_message

7118c51

use follow.send to send the role update message

7976bc2

shtlrs requested review from mbaruh and removed request for Den4200 and jb3 December 4, 2022 13:03

do not use name mangling in _attach_persistent_roles_view

91a869e

mbaruh approved these changes Dec 17, 2022

View reviewed changes

bot/exts/info/subscribe.py Outdated Show resolved Hide resolved

fix Zig's nit comments

c55b9ff

Robin5605 requested changes Jan 13, 2023

View reviewed changes

fix typos in the SELF_ASSIGNABLE_ROLES_MESSAGE

52d69de

wookie184 added t: feature New feature or request s: needs review Author is waiting for someone to review and approve labels Jan 14, 2023

Robin5605 approved these changes Jan 22, 2023

View reviewed changes

Merge branch 'main' into 2332-permanent-role-view

8af86ae

mbaruh merged commit 0937e95 into python-discord:main Jan 22, 2023

wookie184 mentioned this pull request Jan 29, 2023

Replacing !subscribe with reaction roles #2216

Closed

Uh oh!

Allow members to self assign roles through a persistent view #2341

Allow members to self assign roles through a persistent view #2341

Uh oh!

Conversation

shtlrs commented Nov 26, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

mbaruh left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

mbaruh left a comment

Choose a reason for hiding this comment

Uh oh!

onerandomusername commented Dec 4, 2022

Uh oh!

shtlrs commented Dec 4, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

onerandomusername commented Dec 5, 2022

Uh oh!

shtlrs commented Dec 5, 2022

Uh oh!

mbaruh commented Dec 5, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

mbaruh left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Robin5605 left a comment

Choose a reason for hiding this comment

Uh oh!

Robin5605 Jan 13, 2023

Choose a reason for hiding this comment

Uh oh!

shtlrs Jan 14, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Robin5605 Jan 13, 2023

Choose a reason for hiding this comment

Uh oh!

shtlrs Jan 14, 2023

Choose a reason for hiding this comment

Uh oh!

Robin5605 left a comment

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

shtlrs commented Nov 26, 2022 •

edited

Loading

shtlrs commented Dec 4, 2022 •

edited

Loading

mbaruh commented Dec 5, 2022 •

edited

Loading

shtlrs Jan 14, 2023 •

edited

Loading