Skip to content
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

Implement user installations #1952

Open
wants to merge 14 commits into
base: master
Choose a base branch
from
Open

Implement user installations #1952

wants to merge 14 commits into from

Conversation

MagM1go
Copy link

@MagM1go MagM1go commented Jul 1, 2024

Summary

Checklist

  • I have run nox and all the pipelines have passed.
  • I have made unittests according to the code I have added/modified/deleted.

Related issues

Copy link
Member

@davfsa davfsa left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice PR, I would be happy to merge it in its current state, but its missing a couple of things to have the full spec of user installations:

  • app_permissions is no longer optional. The typehint should be updated to reflect this, as well as the deserialization
  • Interaction models have two new fields (both found here): authorizing_integration_owners and context
  • New interaction installation fields config is missing.
  • Messages will now contain interaction_metadata if they were created by an interaction followup. Its fields can be found here

(I might be missing more, so please have a look at the changelog

hikari/commands.py Outdated Show resolved Hide resolved
@davfsa
Copy link
Member

davfsa commented Jul 6, 2024

Again, thanks a lot for opening the pull request and working on this. Sorry it took so long to review 😔

@MagM1go
Copy link
Author

MagM1go commented Jul 6, 2024

I add other things tomorrow

@MagM1go
Copy link
Author

MagM1go commented Jul 6, 2024

(and sorry for shitty commits like the last one)

@MagM1go
Copy link
Author

MagM1go commented Jul 8, 2024

okay I'm lied, it's today

@MagM1go
Copy link
Author

MagM1go commented Jul 8, 2024

I might be missing something, but I'm not sure what I'm doing anymore)))

Copy link
Member

@davfsa davfsa left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Also, modules should export in the __all__ what you added :)

hikari/applications.py Outdated Show resolved Hide resolved
hikari/impl/entity_factory.py Outdated Show resolved Hide resolved
application_models.ApplicationIntegrationType, application_models.ApplicationInstallParameters
]
] = {}
if (integration_types_config_payload := payload.get("integration_types_config")) is not None:
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
if (integration_types_config_payload := payload.get("integration_types_config")) is not None:
if integration_types_config_payload := payload.get("integration_types_config"):

hikari/impl/entity_factory.py Show resolved Hide resolved
authorizing_integration_owners: typing.Mapping[
application_models.ApplicationIntegrationType, snowflakes.Snowflake
] = {}
if (authorizing_integration_owners_payload := payload.get("authorizing_integration_owners")) is not None:
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
if (authorizing_integration_owners_payload := payload.get("authorizing_integration_owners")) is not None:
if authorizing_integration_owners_payload := payload.get("authorizing_integration_owners"):

registered_guild_id=snowflakes.Snowflake(data_payload["guild_id"]) if "guild_id" in data_payload else None,
entitlements=entitlements,
authorizing_integration_owners=payload["authorizing_integration_owners"],
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Shouldnt this be deserialized as a mapping of ApplicationIntegrationType to Snowflake?

@@ -2622,6 +2692,8 @@ def deserialize_autocomplete_interaction(
guild_locale=locales.Locale(payload["guild_locale"]) if "guild_locale" in payload else None,
registered_guild_id=snowflakes.Snowflake(data_payload["guild_id"]) if "guild_id" in data_payload else None,
entitlements=[self.deserialize_entitlement(entitlement) for entitlement in payload.get("entitlements", ())],
authorizing_integration_owners=payload["authorizing_integration_owners"],
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Dito above

@@ -2622,6 +2692,8 @@ def deserialize_autocomplete_interaction(
guild_locale=locales.Locale(payload["guild_locale"]) if "guild_locale" in payload else None,
registered_guild_id=snowflakes.Snowflake(data_payload["guild_id"]) if "guild_id" in data_payload else None,
entitlements=[self.deserialize_entitlement(entitlement) for entitlement in payload.get("entitlements", ())],
authorizing_integration_owners=payload["authorizing_integration_owners"],
context=payload.get("context"),
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I feel like there is a cast missing here

authorizing_integration_owners: typing.Mapping[
application_models.ApplicationIntegrationType, snowflakes.Snowflake
] = {}
if (authorizing_integration_owners_payload := payload.get("authorizing_integration_owners")) is not None:
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
if (authorizing_integration_owners_payload := payload.get("authorizing_integration_owners")) is not None:
if authorizing_integration_owners_payload := payload.get("authorizing_integration_owners"):

@beagold beagold added the enhancement New feature or request label Aug 28, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants