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

feat(PartialGroupDMChannel): add missing properties #10502

Open
wants to merge 5 commits into
base: main
Choose a base branch
from

Conversation

imnaiyar
Copy link
Contributor

@imnaiyar imnaiyar commented Sep 13, 2024

Please describe the changes this PR makes and why it should be merged:
Props Added:

  • ownerId
  • lastMessageId
  • lastPinTimestamp
  • lastMessage
  • lastPinAt
  • awaitMessageComponent
  • createMessageComponentCollector

I noticed ownerId, last_message_id and last_pin_timestamp was missing from PartialGroupDMChannel. This PR fixes that.

While looking at the api documentation alone, it is hard to determine for me if the owner_id can be missing for Group DM Channel's, but from my testing so far, I found it to be always present. So taking that at face value

Status and versioning classification:

  • Code changes have been tested against the Discord API, or there are no code changes
  • I know how to update typings and have done so, or typings don't need updating

@imnaiyar imnaiyar requested a review from a team as a code owner September 13, 2024 12:38
Copy link

vercel bot commented Sep 13, 2024

The latest updates on your projects. Learn more about Vercel for Git ↗︎

2 Skipped Deployments
Name Status Preview Comments Updated (UTC)
discord-js ⬜️ Ignored (Inspect) Visit Preview Dec 5, 2024 9:41pm
discord-js-guide ⬜️ Ignored (Inspect) Visit Preview Dec 5, 2024 9:41pm

@vladfrangu vladfrangu changed the title fix(PartialGroupDMChannel): add missing ownerId property feat(PartialGroupDMChannel): add missing ownerId property Sep 13, 2024
@vladfrangu vladfrangu added this to the discord.js 14.17 milestone Sep 13, 2024
@nyapat
Copy link
Contributor

nyapat commented Sep 15, 2024

While looking at the api documentation alone, it is hard to determine for me if the owner_id can be missing for Group DM Channel's, but from my testing so far, I found it to be always present. So taking that at face value

owner_id? has been optional since before threads were added, I think you should leave it as such

@imnaiyar
Copy link
Contributor Author

imnaiyar commented Sep 15, 2024

Yeah it's optional because it can be missing for other channel types, my thinking was it would always be present for GroupDMs, but now that you mention it, looking at other djs structures (ThreadChannel) it accounts for that. So I think I should stick to the library behavior

@imnaiyar imnaiyar force-pushed the fix/missing-owner-id branch from 8a3bdbb to 4d51ba4 Compare September 15, 2024 16:28
@imnaiyar imnaiyar force-pushed the fix/missing-owner-id branch 2 times, most recently from e0154a4 to aceb5b3 Compare September 19, 2024 16:35
@imnaiyar
Copy link
Contributor Author

imnaiyar commented Sep 19, 2024

I noticed last_message_id and last_pin_timestamp is also missing, I am wondering if it was left out for a reason or I should add those too

@vladfrangu
Copy link
Member

I noticed last_message_id and last_pin_timestamp is also missing, I am wondering if it was left out for a reason or I should add those too

If they are returned, feel free to add them in your PR and update the title/description

@imnaiyar imnaiyar changed the title feat(PartialGroupDMChannel): add missing ownerId property feat(PartialGroupDMChannel): add missing properties Sep 23, 2024
@imnaiyar imnaiyar force-pushed the fix/missing-owner-id branch from 10798f2 to fa796dd Compare September 27, 2024 06:50
@imnaiyar
Copy link
Contributor Author

imnaiyar commented Sep 27, 2024

Added, awaitMessageComponent & createMessageComponentCollector, noticed them missing, but there was no issue on creating collector on group dm for components. I see no reasons to exclude it.
image

@vladfrangu
Copy link
Member

Added, awaitMessageComponent & createMessageComponentCollector, noticed them missing, but there was no issue on creating collector on group dm for components. I see no reasons to exclude it. image

Do these actually work 👀?

@imnaiyar
Copy link
Contributor Author

imnaiyar commented Sep 29, 2024

Do these actually work 👀?

Yes, I've tested it. The channel object returned in the interaction payload contains the group dm's id, which is the most you would need for a channel component collector. At least I have not encountered any edge cases where it wouldn't work. As long as you have Channel partial, it should work

@imnaiyar imnaiyar force-pushed the fix/missing-owner-id branch from 7faf350 to 21dfa0e Compare November 4, 2024 11:12
@imnaiyar imnaiyar force-pushed the fix/missing-owner-id branch from 21dfa0e to 6e1f29d Compare December 5, 2024 21:41
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Status: Review in Progress
Development

Successfully merging this pull request may close these issues.

4 participants