refactor: comprehensive permissionOverwrites refactor (#2818)

* wip: comprehensive permissionOverwrites refactor

* PermissionOverwrites.resolve should Promise.reject()

where a promise is the expected return value

* On second thought, async rewrite to automatically reject on throw

* Fix some docs

* Fix a bug

* fix 2 more bugs

* typings: Updated for latest commit

* typings: Add missing method in GuildChannel

* typings: Add missing `| null` in PermissionOverwriteOption type

* Suggested changes

src/stores/GuildChannelStore.js

@@ -2,7 +2,7 @@ const Channel = require('../structures/Channel');
 const { ChannelTypes } = require('../util/Constants');
 const DataStore = require('./DataStore');
 const GuildChannel = require('../structures/GuildChannel');
-const resolvePermissions = require('../structures/shared/resolvePermissions');
+const PermissionOverwrites = require('../structures/PermissionOverwrites');
 
 /**
  * Stores guild channels.
@@ -31,7 +31,7 @@ class GuildChannelStore extends DataStore {
    * @param {number} [options.bitrate] Bitrate of the new channel in bits (only voice)
    * @param {number} [options.userLimit] Maximum amount of users allowed in the new channel (only voice)
    * @param {ChannelResolvable} [options.parent] Parent of the new channel
-   * @param {OverwriteData[]|Collection<Snowflake, PermissionOverwrites>} [options.overwrites]
+   * @param {OverwriteResolvable[]|Collection<Snowflake, OverwriteResolvable>} [options.overwrites]
    * Permission overwrites of the new channel
    * @param {string} [options.reason] Reason for creating the channel
    * @returns {Promise<GuildChannel>}
@@ -52,9 +52,10 @@ class GuildChannelStore extends DataStore {
    *   ],
    * })
    */
-  create(name, { type, topic, nsfw, bitrate, userLimit, parent, overwrites, reason } = {}) {
+  async create(name, { type, topic, nsfw, bitrate, userLimit, parent, overwrites, reason } = {}) {
     if (parent) parent = this.client.channels.resolveID(parent);
-    return this.client.api.guilds(this.guild.id).channels.post({
+
+    const data = await this.client.api.guilds(this.guild.id).channels.post({
       data: {
         name,
         topic,
@@ -63,10 +64,11 @@ class GuildChannelStore extends DataStore {
         bitrate,
         user_limit: userLimit,
         parent_id: parent,
-        permission_overwrites: resolvePermissions.call(this, overwrites),
+        permission_overwrites: overwrites && overwrites.map(o => PermissionOverwrites.resolve(o, this.guild)),
       },
       reason,
-    }).then(data => this.client.actions.ChannelCreate.handle(data).channel);
+    });
+    return this.client.actions.ChannelCreate.handle(data).channel;
   }
 
   /**

src/structures/GuildChannel.js

@@ -1,7 +1,6 @@
 const Channel = require('./Channel');
 const Role = require('./Role');
 const Invite = require('./Invite');
-const resolvePermissions = require('./shared/resolvePermissions');
 const PermissionOverwrites = require('./PermissionOverwrites');
 const Util = require('../util/Util');
 const Permissions = require('../util/Permissions');
@@ -73,11 +72,11 @@ class GuildChannel extends Channel {
   get permissionsLocked() {
     if (!this.parent) return null;
     if (this.permissionOverwrites.size !== this.parent.permissionOverwrites.size) return false;
-    return !this.permissionOverwrites.find((value, key) => {
+    return this.permissionOverwrites.every((value, key) => {
       const testVal = this.parent.permissionOverwrites.get(key);
-      return testVal === undefined ||
+      return testVal !== undefined &&
-        testVal.deny.bitfield !== value.deny.bitfield ||
+        testVal.deny.bitfield === value.deny.bitfield &&
-        testVal.allow.bitfield !== value.allow.bitfield;
+        testVal.allow.bitfield === value.allow.bitfield;
     });
   }
 
@@ -179,7 +178,7 @@ class GuildChannel extends Channel {
   /**
    * Replaces the permission overwrites in this channel.
    * @param {Object} [options] Options
-   * @param {OverwriteData[]|Collection<Snowflake, PermissionOverwrites>} [options.overwrites]
+   * @param {OverwriteResolvable[]|Collection<Snowflake, OverwriteResolvable>} [options.overwrites]
    * Permission overwrites the channel gets updated with
    * @param {string} [options.reason] Reason for updating the channel overwrites
    * @returns {Promise<GuildChannel>}
@@ -195,30 +194,18 @@ class GuildChannel extends Channel {
    * });
    */
   overwritePermissions({ overwrites, reason } = {}) {
-    return this.edit({ permissionOverwrites: resolvePermissions.call(this, overwrites), reason })
+    return this.edit({ permissionOverwrites: overwrites, reason })
       .then(() => this);
   }
 
   /**
-   * An object mapping permission flags to `true` (enabled), `null` (unset) or `false` (disabled).
+   * Updates Overwrites for a user or role in this channel. (creates if non-existent)
-   * ```js
-   * {
-   *  'SEND_MESSAGES': true,
-   *  'EMBED_LINKS': null,
-   *  'ATTACH_FILES': false,
-   * }
-   * ```
-   * @typedef {Object} PermissionOverwriteOption
-   */
-
-  /**
-   * Overwrites the permissions for a user or role in this channel.
    * @param {RoleResolvable|UserResolvable} userOrRole The user or role to update
    * @param {PermissionOverwriteOption} options The options for the update
    * @param {string} [reason] Reason for creating/editing this overwrite
    * @returns {Promise<GuildChannel>}
    * @example
-   * // Overwrite permissions for a message author
+   * // Update or Create permission overwrites for a message author
    * message.channel.updateOverwrite(message.author, {
    *   SEND_MESSAGES: false
    * })
@@ -226,40 +213,34 @@ class GuildChannel extends Channel {
    *   .catch(console.error);
    */
   updateOverwrite(userOrRole, options, reason) {
-    const allow = new Permissions();
+    userOrRole = this.guild.roles.resolve(userOrRole) || this.client.users.resolve(userOrRole);
-    const deny = new Permissions();
+    if (!userOrRole) return Promise.reject(new TypeError('INVALID_TYPE', 'parameter', 'User nor a Role', true));
-    let type;
 
-    const role = this.guild.roles.get(userOrRole);
+    const existing = this.permissionOverwrites.get(userOrRole.id);
+    if (existing) return existing.update(options, reason).then(() => this);
+    return this.createOverwrite(userOrRole, options, reason);
+  }
 
-    if (role || userOrRole instanceof Role) {
+  /**
-      userOrRole = role || userOrRole;
+   * Overwrites the permissions for a user or role in this channel. (replaces if existent)
-      type = 'role';
+   * @param {RoleResolvable|UserResolvable} userOrRole The user or role to update
-    } else {
+   * @param {PermissionOverwriteOption} options The options for the update
-      userOrRole = this.client.users.resolve(userOrRole);
+   * @param {string} [reason] Reason for creating/editing this overwrite
-      type = 'member';
+   * @returns {Promise<GuildChannel>}
-      if (!userOrRole) return Promise.reject(new TypeError('INVALID_TYPE', 'parameter', 'User nor a Role', true));
+   * @example
-    }
+   * // Create or Replace permissions overwrites for a message author
+   * message.channel.createOverwrite(message.author, {
+   *   SEND_MESSAGES: false
+   * })
+   *   .then(channel => console.log(channel.permissionOverwrites.get(message.author.id)))
+   *   .catch(console.error);
+   */
+  createOverwrite(userOrRole, options, reason) {
+    userOrRole = this.guild.roles.resolve(userOrRole) || this.client.users.resolve(userOrRole);
+    if (!userOrRole) return Promise.reject(new TypeError('INVALID_TYPE', 'parameter', 'User nor a Role', true));
 
-    const prevOverwrite = this.permissionOverwrites.get(userOrRole.id);
+    const type = userOrRole instanceof Role ? 'role' : 'member';
-
+    const { allow, deny } = PermissionOverwrites.resolveOverwriteOptions(options);
-    if (prevOverwrite) {
-      allow.add(prevOverwrite.allow);
-      deny.add(prevOverwrite.deny);
-    }
-
-    for (const perm in options) {
-      if (options[perm] === true) {
-        allow.add(Permissions.FLAGS[perm]);
-        deny.remove(Permissions.FLAGS[perm]);
-      } else if (options[perm] === false) {
-        allow.remove(Permissions.FLAGS[perm]);
-        deny.add(Permissions.FLAGS[perm]);
-      } else if (options[perm] === null) {
-        allow.remove(Permissions.FLAGS[perm]);
-        deny.remove(Permissions.FLAGS[perm]);
-      }
-    }
 
     return this.client.api.channels(this.id).permissions[userOrRole.id]
       .put({ data: { id: userOrRole.id, type, allow: allow.bitfield, deny: deny.bitfield }, reason })
@@ -272,12 +253,7 @@ class GuildChannel extends Channel {
    */
   lockPermissions() {
     if (!this.parent) return Promise.reject(new Error('GUILD_CHANNEL_ORPHAN'));
-    const permissionOverwrites = this.parent.permissionOverwrites.map(overwrite => ({
+    const permissionOverwrites = this.parent.permissionOverwrites.map(overwrite => overwrite.toJSON());
-      deny: overwrite.deny.bitfield,
-      allow: overwrite.allow.bitfield,
-      id: overwrite.id,
-      type: overwrite.type,
-    }));
     return this.edit({ permissionOverwrites });
   }
 
@@ -308,18 +284,10 @@ class GuildChannel extends Channel {
    * @property {Snowflake} [parentID] The parent ID of the channel
    * @property {boolean} [lockPermissions]
    * Lock the permissions of the channel to what the parent's permissions are
-   * @property {OverwriteData[]|Collection<Snowflake, PermissionOverwrites>} [permissionOverwrites]
+   * @property {OverwriteResolvable[]|Collection<Snowflake, OverwriteResolvable>} [permissionOverwrites]
    * Permission overwrites for the channel
    */
 
-  /**
-   * The data for a permission overwrite
-   * @typedef {Object} OverwriteData
-   * @property {PermissionResolvable} [allow] The permissions to allow
-   * @property {PermissionResolvable} [deny] The permissions to deny
-   * @property {GuildMemberResolvable|RoleResolvable} memberOrRole Member or role this overwrite is for
-   */
-
   /**
    * Edits the channel.
    * @param {ChannelData} data The new data for the channel
@@ -342,7 +310,11 @@ class GuildChannel extends Channel {
           });
         });
     }
-    return this.client.api.channels(this.id).patch({
+
+    const permission_overwrites = data.permissionOverwrites &&
+      data.permissionOverwrites.map(o => PermissionOverwrites.resolve(o, this.guild));
+
+    const newData = await this.client.api.channels(this.id).patch({
       data: {
         name: (data.name || this.name).trim(),
         topic: data.topic,
@@ -351,14 +323,14 @@ class GuildChannel extends Channel {
         user_limit: typeof data.userLimit !== 'undefined' ? data.userLimit : this.userLimit,
         parent_id: data.parentID,
         lock_permissions: data.lockPermissions,
-        permission_overwrites: data.permissionOverwrites,
+        permission_overwrites,
       },
       reason,
-    }).then(newData => {
-      const clone = this._clone();
-      clone._patch(newData);
-      return clone;
     });
+
+    const clone = this._clone();
+    clone._patch(newData);
+    return clone;
   }
 
   /**

src/structures/PermissionOverwrites.js

@@ -1,5 +1,7 @@
+const Role = require('./Role');
 const Permissions = require('../util/Permissions');
 const Util = require('../util/Util');
+const { TypeError } = require('../errors');
 
 /**
  * Represents a permission overwrite for a role or member in a guild channel.
@@ -50,6 +52,27 @@ class PermissionOverwrites {
     this.allow = new Permissions(data.allow).freeze();
   }
 
+  /**
+   * Updates this prermissionOverwrites.
+   * @param {PermissionOverwriteOption} options The options for the update
+   * @param {string} [reason] Reason for creating/editing this overwrite
+   * @returns {Promise<PermissionOverwrites>}
+   * @example
+   * // Update permission overwrites
+   * permissionOverwrites.update({
+   *   SEND_MESSAGES: false
+   * })
+   *   .then(channel => console.log(channel.permissionOverwrites.get(message.author.id)))
+   *   .catch(console.error);
+   */
+  update(options, reason) {
+    const { allow, deny } = this.constructor.resolveOverwriteOptions(options, this);
+
+    return this.channel.client.api.channels(this.channel.id).permissions[this.id]
+      .put({ data: { id: this.id, type: this.type, allow: allow.bitfield, deny: deny.bitfield }, reason })
+      .then(() => this);
+  }
+
   /**
    * Deletes this Permission Overwrite.
    * @param {string} [reason] Reason for deleting this overwrite
@@ -64,6 +87,102 @@ class PermissionOverwrites {
   toJSON() {
     return Util.flatten(this);
   }
+
+  /**
+   * An object mapping permission flags to `true` (enabled), `null` (unset) or `false` (disabled).
+   * ```js
+   * {
+   *  'SEND_MESSAGES': true,
+   *  'EMBED_LINKS': null,
+   *  'ATTACH_FILES': false,
+   * }
+   * ```
+   * @typedef {Object} PermissionOverwriteOption
+   */
+
+  /**
+   * @typedef {object} ResolvedOverwriteOptions
+   * @property {Permissions} allow The allowed permissions
+   * @property {Permissions} deny The denied permissions
+   */
+
+  /**
+   * Deletes this Permission Overwrite.
+   * @param {PermissionOverwriteOption} options The options for the update
+   * @param {Object} initialPermissions The initial permissions
+   * @param {PermissionResolvable} initialPermissions.allow Initial allowed permissions
+   * @param {PermissionResolvable} initialPermissions.deny Initial denied permissions
+   * @returns {ResolvedOverwriteOptions}
+   */
+  static resolveOverwriteOptions(options, { allow, deny } = {}) {
+    allow = new Permissions(allow);
+    deny = new Permissions(deny);
+
+    for (const [perm, value] of Object.entries(options)) {
+      if (value === true) {
+        allow.add(Permissions.FLAGS[perm]);
+        deny.remove(Permissions.FLAGS[perm]);
+      } else if (value === false) {
+        allow.remove(Permissions.FLAGS[perm]);
+        deny.add(Permissions.FLAGS[perm]);
+      } else if (value === null) {
+        allow.remove(Permissions.FLAGS[perm]);
+        deny.remove(Permissions.FLAGS[perm]);
+      }
+    }
+
+    return { allow, deny };
+  }
+
+  /**
+   * The raw data for a permission overwrite
+   * @typedef {Object} RawOverwriteData
+   * @property {Snowflake} id The id of the overwrite
+   * @property {number} allow The permissions to allow
+   * @property {number} deny The permissions to deny
+   * @property {OverwriteType} type The type of this OverwriteData
+   */
+
+  /**
+   * Data that can be resolved into {@link RawOverwriteData}
+   * @typedef {PermissionOverwrites|OverwriteData} OverwriteResolvable
+   */
+
+  /**
+   * Data that can be used for a permission overwrite
+   * @typedef {Object} OverwriteData
+   * @property {GuildMemberResolvable|RoleResolvable} id Member or role this overwrite is for
+   * @property {PermissionResolvable} [allow] The permissions to allow
+   * @property {PermissionResolvable} [deny] The permissions to deny
+   * @property {OverwriteType} [type] The type of this OverwriteData
+   */
+
+  /**
+   * Resolves an overwrite into {@link RawOverwriteData}.
+   * @param {OverwriteResolvable} overwrite The overwrite-like data to resolve
+   * @param {Guild} guild The guild to resolve from
+   * @returns {RawOverwriteData}
+   */
+  static resolve(overwrite, guild) {
+    if (overwrite instanceof this) return overwrite.toJSON();
+    if (typeof overwrite.id === 'string' && ['role', 'member'].includes(overwrite.type)) {
+      return { ...overwrite,
+        allow: Permissions.resolve(overwrite.allow),
+        deny: Permissions.resolve(overwrite.deny),
+      };
+    }
+
+    const userOrRole = guild.roles.resolve(overwrite.id) || guild.client.users.resolve(overwrite.id);
+    if (!userOrRole) throw new TypeError('INVALID_TYPE', 'parameter', 'User nor a Role', true);
+    const type = userOrRole instanceof Role ? 'role' : 'member';
+
+    return {
+      id: userOrRole.id,
+      type,
+      allow: Permissions.resolve(overwrite.allow),
+      deny: Permissions.resolve(overwrite.deny),
+    };
+  }
 }
 
 module.exports = PermissionOverwrites;

src/structures/shared/resolvePermissions.js

@@ -1,26 +0,0 @@
-const Permissions = require('../../util/Permissions');
-const Collection = require('../../util/Collection');
-
-module.exports = function resolvePermissions(overwrites) {
-  if (overwrites instanceof Collection || overwrites instanceof Array) {
-    overwrites = overwrites.map(overwrite => {
-      const role = this.guild.roles.resolve(overwrite.id);
-      if (role) {
-        overwrite.id = role.id;
-        overwrite.type = 'role';
-      } else {
-        overwrite.id = this.client.users.resolveID(overwrite.id);
-        overwrite.type = 'member';
-      }
-
-      return {
-        allow: Permissions.resolve(overwrite.allow),
-        deny: Permissions.resolve(overwrite.deny),
-        type: overwrite.type,
-        id: overwrite.id,
-      };
-    });
-  }
-
-  return overwrites;
-};

src/util/Util.js

@@ -2,6 +2,7 @@ const { Colors, DefaultOptions, Endpoints } = require('./Constants');
 const fetch = require('node-fetch');
 const { Error: DiscordError, RangeError, TypeError } = require('../errors');
 const has = (o, k) => Object.prototype.hasOwnProperty.call(o, k);
+const isObject = d => typeof d === 'object' && d !== null;
 const { parse } = require('path');
 
 /**
@@ -19,7 +20,6 @@ class Util {
    * @returns {Object}
    */
   static flatten(obj, ...props) {
-    const isObject = d => typeof d === 'object' && d !== null;
     if (!isObject(obj)) return obj;
 
     props = Object.assign(...Object.keys(obj).filter(k => !k.startsWith('_')).map(k => ({ [k]: true })), ...props);
@@ -39,7 +39,7 @@ class Util {
       // If it's an array, flatten each element
       else if (Array.isArray(element)) out[newProp] = element.map(e => Util.flatten(e));
       // If it's an object with a primitive `valueOf`, use that value
-      else if (valueOf && !isObject(valueOf)) out[newProp] = valueOf;
+      else if (typeof valueOf !== 'object') out[newProp] = valueOf;
       // If it's a primitive
       else if (!elemIsObj) out[newProp] = element;
     }

typings/index.d.ts

@@ -519,20 +519,18 @@ declare module 'discord.js' {
 		public rawPosition: number;
 		public clone(options?: GuildChannelCloneOptions): Promise<GuildChannel>;
 		public createInvite(options?: InviteOptions): Promise<Invite>;
+		public createOverwrite(userOrRole: RoleResolvable | UserResolvable, options: PermissionOverwriteOption, reason?: string): Promise<GuildChannel>;
 		public edit(data: ChannelData, reason?: string): Promise<GuildChannel>;
 		public equals(channel: GuildChannel): boolean;
 		public fetchInvites(): Promise<Collection<string, Invite>>;
 		public lockPermissions(): Promise<GuildChannel>;
-		public overwritePermissions(
+		public overwritePermissions(options?: { overwrites?: OverwriteResolvable[] | Collection<Snowflake, OverwriteResolvable>, reason?: string }): Promise<GuildChannel>;
-			options: Array<Partial<PermissionOverwrites|PermissionOverwriteOptions>> | Collection<Snowflake, Partial<PermissionOverwriteOptions>>,
-			reason?: string
-		): Promise<GuildChannel>;
 		public permissionsFor(memberOrRole: GuildMemberResolvable | RoleResolvable): Readonly<Permissions> | null;
 		public setName(name: string, reason?: string): Promise<GuildChannel>;
 		public setParent(channel: GuildChannel | Snowflake, options?: { lockPermissions?: boolean, reason?: string }): Promise<GuildChannel>;
 		public setPosition(position: number, options?: { relative?: boolean, reason?: string }): Promise<GuildChannel>;
 		public setTopic(topic: string, reason?: string): Promise<GuildChannel>;
-		public updateOverwrite(userOrRole: RoleResolvable | UserResolvable, options: Partial<PermissionObject>, reason?: string): Promise<GuildChannel>;
+		public updateOverwrite(userOrRole: RoleResolvable | UserResolvable, options: PermissionOverwriteOption, reason?: string): Promise<GuildChannel>;
 	}
 
 	export class GuildEmoji extends Emoji {
@@ -786,8 +784,11 @@ declare module 'discord.js' {
 		public deny: Readonly<Permissions>;
 		public id: Snowflake;
 		public type: OverwriteType;
+		public update(options: PermissionOverwriteOption, reason?: string): Promise<PermissionOverwrites>;
 		public delete(reason?: string): Promise<PermissionOverwrites>;
 		public toJSON(): object;
+		public static resolveOverwriteOptions(options: ResolvedOverwriteOptions, initialPermissions: { allow?: PermissionResolvable, deny?: PermissionResolvable }): ResolvedOverwriteOptions;
+		public static resolve(overwrite: OverwriteResolvable, guild: Guild): RawOverwriteData;
 	}
 
 	export class Permissions extends BitField<PermissionString> {
@@ -1521,7 +1522,7 @@ declare module 'discord.js' {
 		userLimit?: number;
 		parentID?: Snowflake;
 		lockPermissions?: boolean;
-		permissionOverwrites?: PermissionOverwrites[];
+		permissionOverwrites?: OverwriteResolvable[] | Collection<Snowflake, OverwriteResolvable>;
 	};
 
 	type ChannelLogsQueryOptions = {
@@ -1738,7 +1739,7 @@ declare module 'discord.js' {
 		bitrate?: number;
 		userLimit?: number;
 		parent?: ChannelResolvable;
-		overwrites?: (PermissionOverwrites | ChannelCreationOverwrites)[];
+		overwrites?: OverwriteResolvable[] | Collection<Snowflake, OverwriteResolvable>;
 		reason?: string
 	};
 
@@ -1889,18 +1890,22 @@ declare module 'discord.js' {
 		| 'GUILD_MEMBER_JOIN';
 
 	type OverwriteData = {
-		id: Snowflake;
+		allow?: PermissionResolvable;
-		type: string;
+		deny?: PermissionResolvable;
-		allow?: string;
+		id: GuildMemberResolvable | RoleResolvable;
-		deny?: string;
+		type?: OverwriteType;
 	};
 
+	type OverwriteResolvable = PermissionOverwrites | OverwriteData;
+
 	type OverwriteType = 'member' | 'role';
 
 	type PermissionFlags = Record<PermissionString, number>;
 
 	type PermissionObject = Record<PermissionString, boolean>;
 
+	type PermissionOverwriteOption = { [k in PermissionString]?: boolean | null };
+
 	type PermissionString = 'CREATE_INSTANT_INVITE'
 		| 'KICK_MEMBERS'
 		| 'BAN_MEMBERS'
@@ -1964,12 +1969,24 @@ declare module 'discord.js' {
 		route: string;
 	};
 
+	type RawOverwriteData = {
+		id: Snowflake;
+		allow: number;
+		deny: number;
+		type: OverwriteType;
+	};
+
 	type ReactionCollectorOptions = CollectorOptions & {
 		max?: number;
 		maxEmojis?: number;
 		maxUsers?: number;
 	};
 
+	type ResolvedOverwriteOptions = {
+		allow: Permissions;
+		deny: Permissions;
+	};
+
 	type RoleData = {
 		name?: string;
 		color?: ColorResolvable;