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

fix(MessageReaction): fetching a removed partial custom emoji #3955

Merged
merged 3 commits into from Mar 17, 2020

Conversation

@izexi
Copy link
Contributor

izexi commented Mar 17, 2020

Fixes #3946

After a "partial" MessageReaction (being a custom emoji) was completely removed, the emoji getter would return an instance of GuildEmoji which was being passed into ReactionManager#_fetchReaction(). Since GuildEmoji#reaction will always return undefined it would end up throwing an error since _patch() was trying to be called on that

reactionEmoji.reaction._patch({ count: 0 });

And as for why this doesn't occur within the messageReactionAdd event (which was asked on #library-discussion channel is because that line will only execute when a reaction emoji has been completely removed and/or when all reactions which is checked beforehand.

To fix this issue: instead of manually fetching the message and handling the fetched message to update the reactions Message#fetch() will be used to indirectly update the reaction, which is much simpler to how it was before so it can be done directly within MessageReaction#fetch() to avoid having trouble finding a reference externally to get access to the _patch method. It somewhat does the same by fetching the message and then updating Message#reactions (Message#fetch()MessageManager#fetchId()MessageManager#add()Message#_patch()ReactionManager#add() → ... MessageReaction#_patch()). When an emoji / all reactions has been completely removed it wouldn't be present within Message#reactions, so MessageReaction#count should obviously be set to 0
which is handled.

Here's a snippet of how I tested these changes
import { Client, DMChannel, ClientUser, Message } from 'discord.js';

interface MockEmoji {
  name: string;
  id: string | null;
}

const mockClient = new Client({ partials: ['MESSAGE', 'REACTION', 'CHANNEL'] });
const mockCustomEmoji: MockEmoji = { name: 'POGGERS', id: '4' };
const mockUnicodeEmoji: MockEmoji = { name: '😩', id: null };
const mockFetch = (reactions: { emoji: MockEmoji; count: number }[] = []) =>
  jest
    .spyOn(Message.prototype, 'fetch')
    .mockImplementation(async function(this: Message) {
      // @ts-ignore
      this._patch({
        reactions,
        id: this.id
      });
      return this;
    });

beforeAll(() => {
  // Add a mock ClientUser
  mockClient.user = new ClientUser(mockClient, { user: { id: '0' } });
  // Add a mock me
  mockClient.users.add({
    username: 'Havoc',
    id: '191615925336670208',
    discriminator: '7078',
    avatar: 'a_dde53f5afa943f4a2628d47045ef92c3'
  });
  // Add a mock channel
  mockClient.channels.add({
    id: '2',
    type: 1
  });
  // Add a partial message to the channel
  (mockClient.channels.cache.first() as DMChannel).messages.add({
    id: '3',
    channel_id: '2'
  });
});

describe('messageReactionAdd', () => {
  it('should fetch the custom reaction', done => {
    mockClient.once('messageReactionAdd', async reaction => {
      expect(reaction.partial).toBe(true);
      await reaction.fetch();
      expect(reaction.partial).toBe(false);
      expect(reaction.count).toBe(1);
      done();
    });
    mockFetch([{ emoji: mockCustomEmoji, count: 1 }]);
    // Mock a "partial" reaction being added to the message
    // @ts-ignore
    mockClient.actions.MessageReactionAdd.handle({
      user_id: '191615925336670208',
      emoji: mockCustomEmoji,
      message_id: '3',
      channel_id: '2'
    });
  });

  it('should fetch the unicode reaction', done => {
    mockClient.once('messageReactionAdd', async reaction => {
      expect(reaction.partial).toBe(true);
      await reaction.fetch();
      expect(reaction.partial).toBe(false);
      expect(reaction.count).toBe(1);
      done();
    });
    mockFetch([{ emoji: mockUnicodeEmoji, count: 1 }]);
    // Mock a "partial" reaction being added to the message
    // @ts-ignore
    mockClient.actions.MessageReactionAdd.handle({
      user_id: '191615925336670208',
      emoji: mockUnicodeEmoji,
      message_id: '3',
      channel_id: '2'
    });
  });
});

describe('messageReactionRemove', () => {
  it('should fetch the custom cleared reaction', done => {
    mockClient.once('messageReactionRemove', async reaction => {
      expect(reaction.partial).toBe(true);
      await reaction.fetch();
      expect(reaction.partial).toBe(false);
      expect(reaction.count).toBe(0);
      done();
    });
    mockFetch();
    // Mock a "partial" reaction being removed from the message
    // @ts-ignore
    mockClient.actions.MessageReactionRemove.handle({
      user_id: '191615925336670208',
      emoji: mockCustomEmoji,
      message_id: '3',
      channel_id: '2'
    });
  });

  it('should fetch the cleared reaction', done => {
    mockClient.once('messageReactionRemove', async reaction => {
      expect(reaction.partial).toBe(true);
      await reaction.fetch();
      expect(reaction.partial).toBe(false);
      expect(reaction.count).toBe(0);
      done();
    });
    mockFetch();
    // Mock a "partial" reaction being removed from the message
    // @ts-ignore
    mockClient.actions.MessageReactionRemove.handle({
      user_id: '191615925336670208',
      emoji: mockUnicodeEmoji,
      message_id: '3',
      channel_id: '2'
    });
  });
});

afterEach(() =>
  (mockClient.channels.cache.first() as DMChannel).messages.cache
    .first()
    ?.reactions.cache.clear()
);

Status

  • 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

Semantic versioning classification:

  • This PR changes the library's interface (methods or parameters added)
    • This PR includes breaking changes (methods removed or renamed, parameters moved or removed)
  • This PR only includes non-code changes, like changes to documentation, README, etc.
@iCrawl
iCrawl approved these changes Mar 17, 2020
@iCrawl iCrawl merged commit a36a65b into discordjs:master Mar 17, 2020
3 checks passed
3 checks passed
ESLint
Details
TSLint
Details
Documentation
Details
@izexi izexi deleted the izexi:partial-reaction-fix branch Mar 18, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

3 participants
You can’t perform that action at this time.