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.

Sign up for GitHub

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

Jump to bottom

feat(Managers): ✨ Add GuildInviteManager #5889

Merged
merged 36 commits into from
Jul 4, 2021
Merged

feat(Managers): ✨ Add GuildInviteManager #5889

merged 36 commits into from
Jul 4, 2021

Conversation

DraftProducts
Copy link
Contributor

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

The InviteManager was missing, this new Manager comes with some extra features

  • Invites generation from Guild based on the first accessible channel
  • Invite deletion outside of an Invite instance
  • Fetch a single invite with cache filling to avoid unnecessary future requests (force option is available)

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
  • This PR changes the library's interface (methods or parameters added)

@DraftProducts
feat(Managers): ✨ Add GuildInviteManager
ed777ff 
The InviteManager was missing, this new Manager comes with some extra features
- Invites generation from Guild based on the first accessible channel
- Invite deletion outside of an Invite instance
- Fetch a single invite with cache filling to avoid unnecessary future requests (force option is available)
DraftProducts
DraftProducts commented Jun 20, 2021
View reviewed changes
src/errors/Messages.js Outdated Show resolved Hide resolved
iShibi
iShibi suggested changes Jun 20, 2021
View reviewed changes
typings/index.d.ts Outdated Show resolved Hide resolved
typings/index.d.ts Outdated Show resolved Hide resolved
@DraftProducts @iShibi
types(GuildInviteManager) Fix pluralization of class name in typings
9f6f82e 
Co-authored-by: iShibi <shubhamparihar391@gmail.com>
vladfrangu
vladfrangu previously requested changes Jun 20, 2021
View reviewed changes
Copy link
Member

@vladfrangu vladfrangu left a comment

Choose a reason for hiding this comment

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

At this point, couldn't we get a GuildChannelInviteManager too? /s
Jokes aside, here are some things I'd like to bring up

src/errors/Messages.js Outdated Show resolved Hide resolved
src/managers/GuildInviteManager.js Outdated Show resolved Hide resolved
src/managers/GuildInviteManager.js Show resolved Hide resolved
src/managers/GuildInviteManager.js Outdated Show resolved Hide resolved
src/managers/GuildInviteManager.js Outdated Show resolved Hide resolved
DraftProducts and others added 2 commits June 20, 2021 18:03
@DraftProducts @vladfrangu
refactor: 💬 Adds literary corrections
c576e4b 
Correction of docs and errors messages

Co-authored-by: Vlad Frangu <kingdgrizzle@gmail.com>
@DraftProducts @vladfrangu
refactor: 🥅 Transform sync errors to Promise rejections
f97bcc5 
Co-authored-by: Vlad Frangu <kingdgrizzle@gmail.com>
iShibi
iShibi reviewed Jun 20, 2021
View reviewed changes
typings/index.d.ts Outdated Show resolved Hide resolved
DraftProducts and others added 2 commits June 20, 2021 18:19
@DraftProducts @vladfrangu
fix: 🐛 Remove mutating in case of options is a frozen object
921c81f 
Co-authored-by: Vlad Frangu <kingdgrizzle@gmail.com>
@DraftProducts @iShibi
types: 🏷️ Replace InviteOptions with CreateInviteOptions
8bfd39a 
Co-authored-by: iShibi <shubhamparihar391@gmail.com>
@DraftProducts DraftProducts requested a review from vladfrangu June 20, 2021 16:26
@DraftProducts @iShibi
refactor(Errors): 🥅 Change "unknown invite" error to avoid confusion …
3bc3be9 
…with APIErrors

Co-authored-by: iShibi <shubhamparihar391@gmail.com>
DraftProducts
DraftProducts commented Jun 20, 2021
View reviewed changes
Copy link
Contributor Author

@DraftProducts DraftProducts left a comment

Choose a reason for hiding this comment

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

I think everything is good if you don't have other suggestions for changes 👍

NotSugden
NotSugden reviewed Jun 20, 2021
View reviewed changes
src/errors/Messages.js Outdated Show resolved Hide resolved
@DraftProducts @NotSugden
fix(Errors): Update quotes & formulation
1890cd1 
Co-authored-by: Sugden <28943913+NotSugden@users.noreply.github.com>
vladfrangu
vladfrangu previously requested changes Jun 22, 2021
View reviewed changes
src/managers/GuildInviteManager.js Outdated Show resolved Hide resolved
src/managers/GuildInviteManager.js Outdated Show resolved Hide resolved
@DraftProducts @vladfrangu
docs: Provide more details in docs about invite creation
8b727b8 
Co-authored-by: Vlad Frangu <kingdgrizzle@gmail.com>
@iCrawl iCrawl requested a review from vladfrangu June 22, 2021 22:38
vladfrangu
vladfrangu previously requested changes Jun 23, 2021
View reviewed changes
src/managers/GuildInviteManager.js Outdated Show resolved Hide resolved
@DraftProducts @vladfrangu
refactor: Change [] to ()
edced3a 
Co-authored-by: Vlad Frangu <kingdgrizzle@gmail.com>
@DraftProducts @kyranet
refactor: ♻️ Remove useless type check
7844337 
Co-authored-by: Antonio Román <kyradiscord@gmail.com>
kyranet
kyranet previously requested changes Jul 3, 2021
View reviewed changes
Copy link
Member

@kyranet kyranet left a comment

Choose a reason for hiding this comment

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

Just one thing I noticed:

src/managers/GuildInviteManager.js Outdated Show resolved Hide resolved
DraftProducts
DraftProducts commented Jul 3, 2021
View reviewed changes
Copy link
Contributor Author

@DraftProducts DraftProducts left a comment •
edited
Loading

Choose a reason for hiding this comment

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

I merged master to resolve conflicts with breaking changes of Guild#fetchInvites()

@DraftProducts DraftProducts requested review from kyranet and SpaceEEC July 3, 2021 15:47
@iCrawl iCrawl dismissed stale reviews from kyranet and SpaceEEC July 3, 2021 20:01

Stale

SpaceEEC
SpaceEEC requested changes Jul 3, 2021
View reviewed changes
src/structures/GuildChannel.js Outdated Show resolved Hide resolved
@DraftProducts DraftProducts requested a review from SpaceEEC July 3, 2021 21:13
@iCrawl
Copy link
Member

iCrawl commented Jul 4, 2021

This needs a rebase.

kyranet
kyranet approved these changes Jul 4, 2021
View reviewed changes
Copy link
Member

@kyranet kyranet left a comment

Choose a reason for hiding this comment

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

Just the rebase, otherwise LGTM.

iCrawl
iCrawl requested changes Jul 4, 2021
View reviewed changes
Copy link
Member

@iCrawl iCrawl left a comment

Choose a reason for hiding this comment

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

ID -> Id changes required.

src/managers/GuildInviteManager.js Outdated Show resolved Hide resolved
src/managers/GuildInviteManager.js Outdated Show resolved Hide resolved
src/managers/GuildInviteManager.js Outdated Show resolved Hide resolved
src/managers/GuildInviteManager.js Outdated Show resolved Hide resolved
src/managers/GuildInviteManager.js Outdated Show resolved Hide resolved
src/managers/GuildInviteManager.js Outdated Show resolved Hide resolved
@DraftProducts
Copy link
Contributor Author

Arf I was already working on it 👀

@DraftProducts DraftProducts requested a review from iCrawl July 4, 2021 19:11
@iCrawl iCrawl merged commit 9e08b02 into discordjs:master Jul 4, 2021
@DraftProducts DraftProducts deleted the feature-invite-manager branch July 4, 2021 22:26
@advaith1
Copy link
Contributor

advaith1 commented Jul 5, 2021 •
edited
Loading

was this actually tested? it seems to not actually add the Guild#invites property, breaking all the places which now try to access it 😬

image

@monbrey monbrey mentioned this pull request Jul 5, 2021
Merged
@kyranet kyranet mentioned this pull request Jul 18, 2021
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@iShibi iShibi iShibi requested changes

@NotSugden NotSugden NotSugden requested changes

@SpaceEEC SpaceEEC SpaceEEC approved these changes

@kyranet kyranet kyranet approved these changes

@iCrawl iCrawl iCrawl approved these changes

@vladfrangu vladfrangu vladfrangu approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
Version 13
Development

Successfully merging this pull request may close these issues.
8 participants
@DraftProducts @iCrawl @advaith1 @vladfrangu @kyranet @SpaceEEC @NotSugden @iShibi