refactor: Move member role-related functions to a special store (#2242)

* Ignore this I need a patch branch for Git

* Move all member role related stuff to a new DataStore
GuildMemberRoleStore is a new store that holds all member role related stuffs
Because its consistent!

* Minorest doc fix ever
To whoever did this object, they forgot a `{`

* Fix the spacing in the docs

* Resue the stores resolve rather than copy paste
Cause I'm dum and it overwrite resolve to the guild role stores resolve soo

* Fix some requests
- Removed the bs private functions
- Set the roles in the constructor

But, I need feedback. There is no way, that I saw, to make a member have roles whenever GuildMmber#_patch is called,
due to the roles being null on the guild. So the only way might be the for loop and getter.

* Fix an issue that I caused in #add
I was testing some other things, and changed that to test. Forgot to change it back

* Actually make the store generate just once when the member is created by first initializing the roles in the guild
Also replaces GuildMember#_roles with GuildMemberRoleStore#_roles
Tested all functions to make sure the expected result happens.

* I missed this from moving remove from GuildMember to GuildMemberRoleStore

* Fix RoleStore#create docs
For real this time

* Do all the requested changes
- Rename all `somethingRole` to `something` (hoistRole => hoist)
- Refactor add and remove to be cleaner and re-use set

* Fix a bug where the store would loose some roles due to null roles that can throw an error in the for loop after they've been deleted

* Remove the `role.id || role` part of the add and remove functions as Appel suggested

* Replace roles.resolve with roles.resolveID for GuildMemberRoleStore#remove

* Don't use Array.isArray in checks
Use instanceof instead

* Woops, I forgot to change this
Renamed colorRole to color

* The docs have dots, so we place the dots back

src/index.js

@@ -29,6 +29,7 @@ module.exports = {
   GuildChannelStore: require('./stores/GuildChannelStore'),
   GuildEmojiStore: require('./stores/GuildEmojiStore'),
   GuildMemberStore: require('./stores/GuildMemberStore'),
+  GuildMemberRoleStore: require('./stores/GuildMemberRoleStore'),
   GuildStore: require('./stores/GuildStore'),
   ReactionUserStore: require('./stores/ReactionUserStore'),
   MessageStore: require('./stores/MessageStore'),

src/stores/GuildMemberRoleStore.js

@@ -0,0 +1,150 @@
+const DataStore = require('./DataStore');
+const Role = require('../structures/Role');
+const Collection = require('../util/Collection');
+const { TypeError } = require('../errors');
+
+/**
+ * Stores member roles
+ * @extends {DataStore}
+ */
+class GuildMemberRoleStore extends DataStore {
+  constructor(member) {
+    super(member.client, null, Role);
+    this.member = member;
+    this.guild = member.guild;
+  }
+
+  /**
+   * Adds a role (or multiple roles) to the member.
+   * @param {RoleResolvable|RoleResolvable[]|Collection<Snowflake, Role>} roleOrRoles The role or roles to add
+   * @param {string} [reason] Reason for adding the role(s)
+   * @returns {Promise<GuildMember>}
+   */
+  add(roleOrRoles, reason) {
+    if (roleOrRoles instanceof Collection) return this.add(roleOrRoles.keyArray(), reason);
+    if (!(roleOrRoles instanceof Array)) return this.add([roleOrRoles], reason);
+
+    roleOrRoles = roleOrRoles.map(r => this.guild.roles.resolve(r));
+
+    if (roleOrRoles.includes(null)) {
+      return Promise.reject(new TypeError('INVALID_TYPE', 'roles',
+        'Array or Collection of Roles or Snowflakes', true));
+    } else {
+      for (const role of roleOrRoles) super.set(role.id, role);
+    }
+
+    return this.set(this, reason);
+  }
+
+  /**
+   * Sets the roles applied to the member.
+   * @param {Collection<Snowflake, Role>|RoleResolvable[]} roles The roles or role IDs to apply
+   * @param {string} [reason] Reason for applying the roles
+   * @returns {Promise<GuildMember>}
+   * @example
+   * // Set the member's roles to a single role
+   * guildMember.roles.set(['391156570408615936'])
+   *   .then(console.log)
+   *   .catch(console.error);
+   * @example
+   * // Remove all the roles from a member
+   * guildMember.roles.set([])
+   *   .then(member => console.log(`Member roles is now of ${member.roles.size} size`))
+   *   .catch(console.error);
+   */
+  set(roles, reason) {
+    return this.member.edit({ roles }, reason);
+  }
+
+  /**
+   * Removes a role (or multiple roles) from the member.
+   * @param {RoleResolvable|RoleResolvable[]|Collection<Snowflake, Role>} roleOrRoles The role or roles to remove
+   * @param {string} [reason] Reason for removing the role(s)
+   * @returns {Promise<GuildMember>}
+   */
+  remove(roleOrRoles, reason) {
+    if (roleOrRoles instanceof Collection) return this.remove(roleOrRoles.keyArray(), reason);
+    if (!(roleOrRoles instanceof Array)) return this.remove([roleOrRoles], reason);
+
+    roleOrRoles = roleOrRoles.map(r => this.guild.roles.resolveID(r));
+
+    if (roleOrRoles.includes(null)) {
+      return Promise.reject(new TypeError('INVALID_TYPE', 'roles',
+        'Array or Collection of Roles or Snowflakes', true));
+    } else {
+      for (const role of roleOrRoles) super.remove(role);
+    }
+
+    return this.set(this, reason);
+  }
+
+  /**
+   * The role of the member used to hoist them in a separate category in the users list
+   * @type {?Role}
+   * @readonly
+   */
+  get hoist() {
+    const hoistedRoles = this.filter(role => role.hoist);
+    if (!hoistedRoles.size) return null;
+    return hoistedRoles.reduce((prev, role) => !prev || role.comparePositionTo(prev) > 0 ? role : prev);
+  }
+
+  /**
+   * The role of the member used to set their color
+   * @type {?Role}
+   * @readonly
+   */
+  get color() {
+    const coloredRoles = this.filter(role => role.color);
+    if (!coloredRoles.size) return null;
+    return coloredRoles.reduce((prev, role) => !prev || role.comparePositionTo(prev) > 0 ? role : prev);
+  }
+
+  /**
+   * The role of the member with the highest position
+   * @type {Role}
+   * @readonly
+   */
+  get highest() {
+    return this.reduce((prev, role) => !prev || role.comparePositionTo(prev) > 0 ? role : prev);
+  }
+
+  /**
+   * Patches the roles for this store
+   * @param {Snowflake[]} roles The new roles
+   * @private
+   */
+  _patch(roles) {
+    this.clear();
+
+    const everyoneRole = this.guild.roles.get(this.guild.id);
+    if (everyoneRole) super.set(everyoneRole.id, everyoneRole);
+
+    if (roles) {
+      for (const roleID of roles) {
+        const role = this.guild.roles.resolve(roleID);
+        if (role) super.set(role.id, role);
+      }
+    }
+  }
+
+  /**
+   * Resolves a RoleResolvable to a Role object.
+   * @method resolve
+   * @memberof GuildMemberRoleStore
+   * @instance
+   * @param {RoleResolvable} role The role resolvable to resolve
+   * @returns {?Role}
+   */
+
+  /**
+   * Resolves a RoleResolvable to a role ID string.
+   * @method resolveID
+   * @memberof GuildMemberRoleStore
+   * @instance
+   * @param {RoleResolvable} role The role resolvable to resolve
+   * @returns {?Snowflake}
+   */
+}
+
+module.exports = GuildMemberRoleStore;

src/stores/RoleStore.js

@@ -34,7 +34,7 @@ class RoleStore extends DataStore {
    *     name: 'Super Cool People',
    *     color: 'BLUE'
    *   },
-   *   reason: 'we needed a role for Super Cool People',
+   *   'we needed a role for Super Cool People',
    * })
    *   .then(console.log)
    *   .catch(console.error);
@@ -61,22 +61,22 @@ class RoleStore extends DataStore {
    */
 
   /**
-    * Resolves a RoleResolvable to a Role object.
+   * Resolves a RoleResolvable to a Role object.
-    * @method resolve
+   * @method resolve
-    * @memberof RoleStore
+   * @memberof RoleStore
-    * @instance
+   * @instance
-    * @param {RoleResolvable} role The role resolvable to resolve
+   * @param {RoleResolvable} role The role resolvable to resolve
-    * @returns {?Role}
+   * @returns {?Role}
-    */
+   */
 
   /**
-    * Resolves a RoleResolvable to a role ID string.
+   * Resolves a RoleResolvable to a role ID string.
-    * @method resolveID
+   * @method resolveID
-    * @memberof RoleStore
+   * @memberof RoleStore
-    * @instance
+   * @instance
-    * @param {RoleResolvable} role The role resolvable to resolve
+   * @param {RoleResolvable} role The role resolvable to resolve
-    * @returns {?Snowflake}
+   * @returns {?Snowflake}
-    */
+   */
 }
 
 module.exports = RoleStore;

src/structures/Guild.js

@@ -179,6 +179,18 @@ class Guild extends Base {
     this.available = !data.unavailable;
     this.features = data.features || this.features || [];
 
+    if (data.channels) {
+      this.channels.clear();
+      for (const rawChannel of data.channels) {
+        this.client.channels.add(rawChannel, this);
+      }
+    }
+
+    if (data.roles) {
+      this.roles.clear();
+      for (const role of data.roles) this.roles.add(role);
+    }
+
     if (data.members) {
       this.members.clear();
       for (const guildUser of data.members) this.members.add(guildUser);
@@ -192,18 +204,6 @@ class Guild extends Base {
       this.ownerID = data.owner_id;
     }
 
-    if (data.channels) {
-      this.channels.clear();
-      for (const rawChannel of data.channels) {
-        this.client.channels.add(rawChannel, this);
-      }
-    }
-
-    if (data.roles) {
-      this.roles.clear();
-      for (const role of data.roles) this.roles.add(role);
-    }
-
     if (data.presences) {
       for (const presence of data.presences) {
         this.presences.add(presence);

src/structures/GuildMember.js

@@ -1,10 +1,10 @@
 const TextBasedChannel = require('./interfaces/TextBasedChannel');
 const Role = require('./Role');
 const Permissions = require('../util/Permissions');
-const Collection = require('../util/Collection');
+const GuildMemberRoleStore = require('../stores/GuildMemberRoleStore');
 const Base = require('./Base');
 const { Presence } = require('./Presence');
-const { Error, TypeError } = require('../errors');
+const { Error } = require('../errors');
 
 /**
  * Represents a member of a guild on Discord.
@@ -27,7 +27,12 @@ class GuildMember extends Base {
      */
     this.user = {};
 
-    this._roles = [];
+    /**
+     * A list of roles that are applied to this GuildMember, mapped by the role ID
+     * @type {GuildMemberRoleStore<Snowflake, Role>}
+     */
+
+    this.roles = new GuildMemberRoleStore(this);
 
     if (data) this._patch(data);
 
@@ -67,7 +72,7 @@ class GuildMember extends Base {
     if (data.joined_at) this.joinedTimestamp = new Date(data.joined_at).getTime();
 
     this.user = this.guild.client.users.add(data.user);
-    if (data.roles) this._roles = data.roles;
+    if (data.roles) this.roles._patch(data.roles);
   }
 
   get voiceState() {
@@ -134,52 +139,13 @@ class GuildMember extends Base {
     return this.frozenPresence || this.guild.presences.get(this.id) || new Presence(this.client);
   }
 
-  /**
-   * A list of roles that are applied to this GuildMember, mapped by the role ID
-   * @type {Collection<Snowflake, Role>}
-   * @readonly
-   */
-  get roles() {
-    const list = new Collection();
-    const everyoneRole = this.guild.roles.get(this.guild.id);
-
-    if (everyoneRole) list.set(everyoneRole.id, everyoneRole);
-
-    for (const roleID of this._roles) {
-      const role = this.guild.roles.get(roleID);
-      if (role) list.set(role.id, role);
-    }
-
-    return list;
-  }
-
-  /**
-   * The role of the member with the highest position
-   * @type {Role}
-   * @readonly
-   */
-  get highestRole() {
-    return this.roles.reduce((prev, role) => !prev || role.comparePositionTo(prev) > 0 ? role : prev);
-  }
-
-  /**
-   * The role of the member used to set their color
-   * @type {?Role}
-   * @readonly
-   */
-  get colorRole() {
-    const coloredRoles = this.roles.filter(role => role.color);
-    if (!coloredRoles.size) return null;
-    return coloredRoles.reduce((prev, role) => !prev || role.comparePositionTo(prev) > 0 ? role : prev);
-  }
-
   /**
    * The displayed color of the member in base 10
    * @type {number}
    * @readonly
    */
   get displayColor() {
-    const role = this.colorRole;
+    const role = this.roles.color;
     return (role && role.color) || 0;
   }
 
@@ -189,21 +155,10 @@ class GuildMember extends Base {
    * @readonly
    */
   get displayHexColor() {
-    const role = this.colorRole;
+    const role = this.roles.color;
     return (role && role.hexColor) || '#000000';
   }
 
-  /**
-   * The role of the member used to hoist them in a separate category in the users list
-   * @type {?Role}
-   * @readonly
-   */
-  get hoistRole() {
-    const hoistedRoles = this.roles.filter(role => role.hoist);
-    if (!hoistedRoles.size) return null;
-    return hoistedRoles.reduce((prev, role) => !prev || role.comparePositionTo(prev) > 0 ? role : prev);
-  }
-
   /**
    * Whether this member is muted in any way
    * @type {boolean}
@@ -395,104 +350,6 @@ class GuildMember extends Base {
     return this.edit({ channel });
   }
 
-  /**
-   * Sets the roles applied to the member.
-   * @param {Collection<Snowflake, Role>|RoleResolvable[]} roles The roles or role IDs to apply
-   * @param {string} [reason] Reason for applying the roles
-   * @returns {Promise<GuildMember>}
-   * @example
-   * // Set the member's roles to a single role
-   * guildMember.setRoles(['391156570408615936'])
-   *   .then(console.log)
-   *   .catch(console.error);
-   * @example
-   * // Remove all the roles from a member
-   * guildMember.setRoles([])
-   *   .then(member => console.log(`Member roles is now of ${member.roles.size} size`))
-   *   .catch(console.error);
-   */
-  setRoles(roles, reason) {
-    return this.edit({ roles }, reason);
-  }
-
-  /**
-   * Adds a single role to the member.
-   * @param {RoleResolvable} role The role or ID of the role to add
-   * @param {string} [reason] Reason for adding the role
-   * @returns {Promise<GuildMember>}
-   */
-  addRole(role, reason) {
-    role = this.guild.roles.resolve(role);
-    if (!role) return Promise.reject(new TypeError('INVALID_TYPE', 'role', 'Role nor a Snowflake'));
-    if (this._roles.includes(role.id)) return Promise.resolve(this);
-    return this.client.api.guilds(this.guild.id).members(this.user.id).roles(role.id)
-      .put({ reason })
-      .then(() => {
-        const clone = this._clone();
-        if (!clone._roles.includes(role.id)) clone._roles.push(role.id);
-        return clone;
-      });
-  }
-
-  /**
-   * Adds multiple roles to the member.
-   * @param {Collection<Snowflake, Role>|RoleResolvable[]} roles The roles or role IDs to add
-   * @param {string} [reason] Reason for adding the roles
-   * @returns {Promise<GuildMember>}
-   */
-  addRoles(roles, reason) {
-    let allRoles = this._roles.slice();
-    for (let role of roles instanceof Collection ? roles.values() : roles) {
-      role = this.guild.roles.resolve(role);
-      if (!role) {
-        return Promise.reject(new TypeError('INVALID_TYPE', 'roles',
-          'Array or Collection of Roles or Snowflakes', true));
-      }
-      allRoles.push(role.id);
-    }
-    return this.edit({ roles: allRoles }, reason);
-  }
-
-  /**
-   * Removes a single role from the member.
-   * @param {RoleResolvable} role The role or ID of the role to remove
-   * @param {string} [reason] Reason for removing the role
-   * @returns {Promise<GuildMember>}
-   */
-  removeRole(role, reason) {
-    role = this.guild.roles.resolve(role);
-    if (!role) return Promise.reject(new TypeError('INVALID_TYPE', 'role', 'Role nor a Snowflake'));
-    if (!this._roles.includes(role.id)) return Promise.resolve(this);
-    return this.client.api.guilds(this.guild.id).members(this.user.id).roles(role.id)
-      .delete({ reason })
-      .then(() => {
-        const clone = this._clone();
-        const index = clone._roles.indexOf(role.id);
-        if (~index) clone._roles.splice(index, 1);
-        return clone;
-      });
-  }
-
-  /**
-   * Removes multiple roles from the member.
-   * @param {Collection<Snowflake, Role>|RoleResolvable[]} roles The roles or role IDs to remove
-   * @param {string} [reason] Reason for removing the roles
-   * @returns {Promise<GuildMember>}
-   */
-  removeRoles(roles, reason) {
-    const allRoles = this._roles.slice();
-    for (let role of roles instanceof Collection ? roles.values() : roles) {
-      role = this.guild.roles.resolve(role);
-      if (!role) {
-        return Promise.reject(new TypeError('INVALID_TYPE', 'roles',
-          'Array or Collection of Roles or Snowflakes', true));
-      }
-      const index = allRoles.indexOf(role.id);
-      if (index >= 0) allRoles.splice(index, 1);
-    }
-    return this.edit({ roles: allRoles }, reason);
-  }
-
   /**
    * Sets the nickname for the guild member.
    * @param {string} nick The nickname for the guild member