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 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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Custom Emoji Remove MessageReaction Partial error on fetch every time
3 participants