-
-
Notifications
You must be signed in to change notification settings - Fork 478
feat: Add support for editing application info and new fields #2994
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: master
Are you sure you want to change the base?
Conversation
Introduces the AppInfo.edit coroutine to allow editing application settings. Updates AppInfo and related types to support new fields such as bot, flags, event webhooks, integration_types_config, and approximate_user_authorization_count. Also refactors type hints and improves handling of optional fields for better API compatibility.
|
Thanks for opening this pull request! This pull request can be checked-out with: git fetch origin pull/2994/head:pr-2994
git checkout pr-2994This pull request can be installed with: pip install git+https://github.com/Pycord-Development/pycord@refs/pull/2994/head |
Soheab
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Also missing docs for the new attributes.
Introduces the AppInfo.edit() method to allow editing application settings, including new and previously missing fields such as icon, cover_image, tags, install_params, integration_types_config, flags, event_webhooks_url, event_webhooks_status, and event_webhooks_types. Also adds related helper classes and properties for handling these fields and updates the CHANGELOG accordingly.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
This pull request adds an AppInfo.edit() method for editing application settings and includes missing fields from the Discord API to the AppInfo class.
Key Changes:
- Added
AppInfo.edit()method to modify application settings via the Discord API - Added new fields to
AppInfoincludingbot,event_webhooks_*,integration_types_config, andapproximate_user_authorization_count - Restructured TypedDict definitions in
discord/types/appinfo.pyto better reflect the Discord API structure - Added
IntegrationTypesConfighelper class for per-installation context configuration
Reviewed Changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated 10 comments.
Show a summary per file
| File | Description |
|---|---|
| discord/types/appinfo.py | Restructured TypedDict definitions, changed from User to PartialUser, moved many fields to NotRequired, and added new type aliases for application integration types |
| discord/http.py | Added edit_current_application() HTTP method to support the new edit functionality |
| discord/client.py | Removed workaround for missing rpc_origins field and cleaned up unused imports |
| discord/appinfo.py | Added new edit() method, new fields and properties, IntegrationTypesConfig class, and to_payload() method for AppInstallParams |
| CHANGELOG.md | Documented the new feature |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Co-authored-by: Copilot <[email protected]> Signed-off-by: Lumouille <[email protected]>
Co-authored-by: Copilot <[email protected]> Signed-off-by: Lumouille <[email protected]>
Co-authored-by: Copilot <[email protected]> Signed-off-by: Lumouille <[email protected]>
Co-authored-by: Copilot <[email protected]> Signed-off-by: Lumouille <[email protected]>
Co-authored-by: Copilot <[email protected]> Signed-off-by: Lumouille <[email protected]>
Co-authored-by: Copilot <[email protected]> Signed-off-by: Lumouille <[email protected]>
Updated the AppInfo class to only accept bytes or None for the icon and cover_image parameters, removing support for str. This change clarifies the expected types and may prevent type-related errors.
|
could we some review here ? |
…atus and add setter for event_webhooks_enabled
Co-authored-by: Paillat <[email protected]> Signed-off-by: Lumouille <[email protected]>
|
Im not home until Tuesday so feel free to edit it |
|
No stress, this one is not urgent |
| return raw.get(skey) | ||
|
|
||
| @staticmethod | ||
| def _decode_ctx(value: dict | None) -> AppInstallParams | None: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
| def _decode_ctx(value: dict | None) -> AppInstallParams | None: | |
| def _decode_ctx(value: dict[Any, Any] | None) -> AppInstallParams | None: |
Or other relevant typing, I don't have the context to know here
| self.user = user | ||
|
|
||
| @staticmethod | ||
| def _get_ctx(raw: dict | None, key: int): |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Idem as below, we should generally avoid bare dicts in types going forward, at least type them and if it makes sense use a TypedDict from discord.types
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
yep, i forgot those 2 as they were before inside another function...
| return None | ||
| return {"oauth2_install_params": value._to_payload()} | ||
|
|
||
| def _to_payload(self) -> dict[int, dict[str, object] | None]: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
| def _to_payload(self) -> dict[int, dict[str, object] | None]: | |
| def _to_payload(self) -> dict[int, dict[str, Any] | None]: |
| return {"oauth2_install_params": value._to_payload()} | ||
|
|
||
| def _to_payload(self) -> dict[int, dict[str, object] | None]: | ||
| payload: dict[int, dict[str, object] | None] = {} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
| payload: dict[int, dict[str, object] | None] = {} | |
| payload: dict[int, dict[str, Any] | None] = {} |
I'm curious, why did you go with object instead of Any ? Maybe I'm missing something.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
i dont fully remember, but i was basically trying to avoid Any, and object seems like a good feat as its a bit less opaque than Any.
We can revert it to only Any tho
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think we should use Any for consistency, it also is more correct from a typing perspective, see:
Albeit ideally we should just avoid Any as much as we can.
|
|
||
| def _encode_install_params( | ||
| self, value: AppInstallParams | None | ||
| ) -> dict[str, object] | None: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
| ) -> dict[str, object] | None: | |
| ) -> dict[str, Any] | None: |
| self.scopes: list[str] = data.get("scopes", []) | ||
| self.permissions: Permissions = Permissions(int(data["permissions"])) | ||
|
|
||
| def _to_payload(self) -> dict[str, object]: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
| def _to_payload(self) -> dict[str, object]: | |
| def _to_payload(self) -> dict[str, Any]: |
discord/appinfo.py
Outdated
| "role_connections_verification_url" | ||
| ) | ||
| self.event_webhooks_url: str | None = data.get("event_webhooks_url") | ||
| self._event_webhooks_status: int | None = data.get("event_webhooks_status") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
self._event_webhooks_status supposedly can't be None.
https://discord.mintlify.app/developers/docs/resources/application#application-object
Also, actually this should probably not be private. I think me or someone said the opposite before, I personally incorrectly assumed that it could have just two enum states (1, 2), but since it has 1, 2, 3, we might want to let the user know which one it is if they want. Keep the convenience method tho.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yup... seems like: discord/discord-api-docs#8015
I saw that you use try_enum now, but by doing that you access the dict property twice.
Maybe something like this could work ?
event_webhooks_status = data.get("event_webhooks_status")
self.event_webhooks_status: ApplicationEventWebhookStatus | None = try_enum(ApplicationEventWebhookStatus, event_webhooks_status) if event_webhooks_status else None
Introduces the AppInfo.edit coroutine to allow editing application settings. Updates AppInfo and related types to support new fields such as bot, flags, event webhooks, integration_types_config, and approximate_user_authorization_count. Also refactors type hints and improves handling of optional fields for better API compatibility.
Summary
Information
examples, ...).
Checklist
type: ignorecomments were used, a comment is also left explaining why.