feat: Guild Builder #2662

DA-344 · 2024-11-29T06:42:37Z

Summary

This PR adds a new guild builder to allow users to better customize a guild before creating it (see POST /guilds)

Current implementation changes the create_guild logic but it is going to be changed to a new method called build_guild.

I will try to add all the available parameters to create_guild in a understandable way.

Information

This PR fixes an issue.
This PR adds something new (e.g. new method or parameters).
This PR is a breaking change (e.g. methods or parameters removed/renamed).
This PR is not a code change (e.g. documentation, README, typehinting,
examples, ...).

Checklist

I have searched the open pull requests for duplicates.
If code changes were made then they have been tested.
- I have updated the documentation to reflect the changes.
If type: ignore comments were used, a comment is also left explaining why.
I have updated the changelog to include these changes.

…ildBuilder

Paillat-dev · 2024-11-29T09:11:01Z

As mentioned by @NeloBlivion too, the builder feature does not in my opinion too belong to the core

ok yeah my previous opinion stays the same, create_guild should retain existing behavior while supporting the new parameters, and there should be a separate Client function for using GuildBuilder

https://discord.com/channels/881207955029110855/881735314987708456/1311779043531558935

But I am happy to discuss on this.

DA-344 · 2024-11-29T14:03:49Z

Yeah, it is mentioned on the PR message:

Current implementation changes the create_guild logic but it is going to be changed to a new method called build_guild.

…into feat/create-guild-parameters

…344/pycord into feat/create-guild-parameters

Paillat-dev · 2024-11-29T16:33:40Z

discord/http.py

    def delete_guild(self, guild_id: Snowflake) -> Response[None]:
        return self.request(Route("DELETE", "/guilds/{guild_id}", guild_id=guild_id))

-    def create_guild(self, name: str, icon: str | None) -> Response[guild.Guild]:


The icon parameter is removed, was it used anywhere in the library ? Just make sure this is changed everywhere it should be. Otherwise this is fine and not considered breaking.

The icon parameter is a key for payload so it is not required as a parameter anymore, and no, this method is only used on Client.create_guild.

Paillat-dev · 2024-11-29T16:34:36Z

discord/types/guild.py

    failed_users: list[Snowflake]
+
+
+class GuildCreate(TypedDict):


Suggested change

class GuildCreate(TypedDict):

class GuildCreatePayload(TypedDict):

Paillat-dev · 2024-11-29T16:35:49Z

discord/guild_builder.py

+
+    Attributes
+    ----------
+    code: Optional[:class:`str`]


Maybe smth more explicit than just code ? E.g. template_code.

Paillat-dev · 2024-11-29T16:39:57Z

discord/guild_builder.py

+        self.system_channel_id: int | None = None
+        self.system_channel_flags: SystemChannelFlags | None = None
+
+    async def _do_create(self) -> Guild:


This could be public, to allow devs to use it as an alternative to the __await__ approach, but maybe under another name than do_create.

Agreed on this, just calling await GuildBuilder() seems too ambiguous

Paillat-dev · 2024-11-29T16:41:22Z

discord/guild_builder.py

+        category_id: Optional[:class:`int`]
+            The category placeholder ID this channel belongs to.
+        bitrate: :class:`int`
+        The channel bitrate.


Missindented

NeloBlivion · 2024-11-29T16:59:24Z

discord/client.py

+        if afk_timeout is not MISSING:
+            metadata["afk_timeout"] = afk_timeout
+
+        # TODO: remove support of passing ``None`` to ``icon``.


Not sure why you rewrote it like this, the old code for icon was sufficient

NeloBlivion · 2024-11-29T17:00:18Z

discord/client.py

+        if code is MISSING:
+            code = None  # type: ignore

-        if code:


I personally don't know if code should use GuildBuilder, considering all the new parameters would be ignored (if anything it should raise a ValueError for passing it with them), though regardless i suggest create_guild be completely detached from GuildBuilder so all of its logic can remain isolated in build_guild (though i think you've said you're planning on this, either way there's no rush)

Also why type ignore ? If needed used an overload.

Lulalaby

I feel like this isn't in the scope for the core library. It would be a better fit as (community-)extension.

NeloBlivion · 2024-11-29T21:57:51Z

I feel like this isn't in the scope for the core library. It would be a better fit as (community-)extension.

I generally agree, but the main motivation for this PR is adding support for channels and roles in guild creation since we have no friendly way for users to make these objects from scratch, hence using this builder structure.

That being said, this approach isn't ideal because we're effectively duplicating parts of the existing role and channel classes just for the sake of a relatively niche endpoint for bots (not by fault of OP, this just wasn't considered necessary in the original design). This might benefit from being held off until we have more widely applicable objects (e.g. Partial objects where users can assign any valid attributes beyond just ID for any valid model, or maybe in 3.0 we could let users arbitrarily instantiate Discord Models)

Lulalaby · 2024-11-29T22:07:41Z

I generally agree, but the main motivation for this PR is adding support for channels and roles in guild creation since we have no friendly way for users to make these objects from scratch, hence using this builder structure.

Yeah. Since I only see it from discords side, the core is working as intended. Yeah is not friendly, but that's the api.
Also bots rarely create guilds themself and are limited.

That being said, this approach isn't ideal

Agreed

This might benefit from being held off until we have more widely applicable objects

Also agreed, I'm gonna be marking this PR on hold until further notice. It's not in the scope of 2.X releases.

DA-344 and others added 3 commits November 28, 2024 19:53

feat: Add more Client.create_guild parameters and make it return a Gu…

325417d

…ildBuilder

chore: Update channel.py:__all__ and added guild_builder to __init__.py

4de7469

style(pre-commit): auto fixes from pre-commit.com hooks

5029863

DA-344 and others added 5 commits November 29, 2024 15:58

Merge branch 'master' of https://github.com/Pycord-Development/pycord …

ec60303

…into feat/create-guild-parameters

chore: Update builder

2f37843

style(pre-commit): auto fixes from pre-commit.com hooks

2c2b5b0

fix: Client.create_guild return on docstring

da9f389

Merge branch 'feat/create-guild-parameters' of https://github.com/DA-…

1458bcc

…344/pycord into feat/create-guild-parameters

Paillat-dev reviewed Nov 29, 2024

View reviewed changes

NeloBlivion reviewed Nov 29, 2024

View reviewed changes

Lulalaby requested changes Nov 29, 2024

View reviewed changes

Lulalaby self-assigned this Nov 29, 2024

Lulalaby added invalid This doesn't seem right priority: low Low Priority on hold Out Of Scope labels Nov 29, 2024

DA-344 closed this Dec 11, 2024

	class GuildCreate(TypedDict):
	class GuildCreatePayload(TypedDict):

Uh oh!

feat: Guild Builder #2662

feat: Guild Builder #2662

Uh oh!

Conversation

DA-344 commented Nov 29, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

Information

Checklist

Uh oh!

Paillat-dev commented Nov 29, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

DA-344 commented Nov 29, 2024

Uh oh!

Paillat-dev Nov 29, 2024

Choose a reason for hiding this comment

Uh oh!

DA-344 Nov 29, 2024

Choose a reason for hiding this comment

Uh oh!

Paillat-dev Nov 29, 2024

Choose a reason for hiding this comment

Uh oh!

Paillat-dev Nov 29, 2024

Choose a reason for hiding this comment

Uh oh!

Paillat-dev Nov 29, 2024

Choose a reason for hiding this comment

Uh oh!

NeloBlivion Nov 29, 2024

Choose a reason for hiding this comment

Uh oh!

Paillat-dev Nov 29, 2024 • 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!

NeloBlivion Nov 29, 2024

Choose a reason for hiding this comment

Uh oh!

NeloBlivion Nov 29, 2024 • 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!

Paillat-dev Nov 29, 2024 • 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!

Lulalaby left a comment

Choose a reason for hiding this comment

Uh oh!

NeloBlivion commented Nov 29, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Lulalaby commented Nov 29, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

DA-344 commented Nov 29, 2024 •

edited

Loading

Paillat-dev commented Nov 29, 2024 •

edited

Loading

Paillat-dev Nov 29, 2024 •

edited

Loading

NeloBlivion Nov 29, 2024 •

edited

Loading

Paillat-dev Nov 29, 2024 •

edited

Loading

NeloBlivion commented Nov 29, 2024 •

edited

Loading

Lulalaby commented Nov 29, 2024 •

edited

Loading