Repository: nyxx-discord/nyxx
Pull request #296 (2022-01-30 14:53:20)

Fix permission serialisation
# Description

Fixes issues with permission serialization.


## Type of change

- [x] Bug fix (non-breaking change which fixes an issue)

# Checklist:

- [x] Ran `dart analyze` or `make analyze` and fixed all issues
- [x] Ran `dart format --set-exit-if-changed -l 160 ./lib` or `make format` and fixed all issues
- [x] I have performed a self-review of my own code
- [x] I have commented my code, particularly in hard-to-understand areas
- [x] I have made corresponding changes to the documentation
- [ ] I have added tests that prove my fix is effective or that my feature works
- [ ] I have checked my changes haven't lowered code coverage



Patch:
diff --git a/lib/src/internal/http_endpoints.dart b/lib/src/internal/http_endpoints.dart
index 330385c5..d008ffb9 100644
--- a/lib/src/internal/http_endpoints.dart
+++ b/lib/src/internal/http_endpoints.dart
@@ -890,18 +890,14 @@ class HttpEndpoints implements IHttpEndpoints {
 
   @override
   Future<void> editChannelPermissions(Snowflake channelId, PermissionsBuilder perms, SnowflakeEntity entity, {String? auditReason}) async {
-    final permSet = perms.build();
-
     await executeSafe(BasicRequest("/channels/$channelId/permissions/${entity.id.toString()}",
-        method: "PUT", body: {"type": entity is Role ? "role" : "member", "allow": permSet.allow, "deny": permSet.deny}, auditLog: auditReason));
+        method: "PUT", body: {"type": entity is IRole ? 0 : 1, ...perms.build()}, auditLog: auditReason));
   }
 
   @override
   Future<void> editChannelPermissionOverrides(Snowflake channelId, PermissionOverrideBuilder permissionBuilder, {String? auditReason}) async {
-    final permSet = permissionBuilder.build();
-
     await executeSafe(BasicRequest("/channels/$channelId/permissions/${permissionBuilder.id.toString()}",
-        method: "PUT", body: {"type": permissionBuilder.type, "allow": permSet.allow, "deny": permSet.deny}, auditLog: auditReason));
+        method: "PUT", body: permissionBuilder.build(), auditLog: auditReason));
   }
 
   @override
diff --git a/lib/src/utils/builders/channel_builder.dart b/lib/src/utils/builders/channel_builder.dart
index 2570d3f2..b06e6af6 100644
--- a/lib/src/utils/builders/channel_builder.dart
+++ b/lib/src/utils/builders/channel_builder.dart
@@ -1,5 +1,4 @@
 import 'package:nyxx/nyxx.dart';
-import 'package:nyxx/src/core/channel/channel.dart';
 
 /// Builder for creating mini channel instance
 abstract class ChannelBuilder implements Builder {
@@ -20,7 +19,7 @@ abstract class ChannelBuilder implements Builder {
         if (type != null) "type": type!.value,
         if (position != null) "position": position,
         if (parentChannel != null) "parent_id": parentChannel!.id.toString(),
-        if (permissionOverrides != null) "permission_overwrites": permissionOverrides!.map((e) => e.build().build()).toList(),
+        if (permissionOverrides != null) "permission_overwrites": permissionOverrides!.map((e) => e.build()).toList(),
       };
 }
 
diff --git a/lib/src/utils/builders/guild_builder.dart b/lib/src/utils/builders/guild_builder.dart
index 3f1d9a4d..46b7be63 100644
--- a/lib/src/utils/builders/guild_builder.dart
+++ b/lib/src/utils/builders/guild_builder.dart
@@ -85,7 +85,7 @@ class RoleBuilder extends Builder {
         if (color != null) "color": color!.value,
         if (hoist != null) "hoist": hoist,
         if (position != null) "position": position,
-        if (permission != null) "permission": permission!.build().build(),
+        if (permission != null) "permissions": permission!.calculatePermissionValue().toString(),
         if (mentionable != null) "mentionable": mentionable,
         if (roleIcon != null) "icon": roleIcon!.getBase64(),
         if (roleIconEmoji != null) "unicode_emoji": roleIconEmoji
diff --git a/lib/src/utils/builders/permissions_builder.dart b/lib/src/utils/builders/permissions_builder.dart
index 65b9b79e..ee33adaf 100644
--- a/lib/src/utils/builders/permissions_builder.dart
+++ b/lib/src/utils/builders/permissions_builder.dart
@@ -1,9 +1,6 @@
-import 'package:nyxx/src/core/snowflake.dart';
-import 'package:nyxx/src/core/snowflake_entity.dart';
+import 'package:nyxx/nyxx.dart';
 import 'package:nyxx/src/core/guild/role.dart';
 import 'package:nyxx/src/core/permissions/permissions.dart';
-import 'package:nyxx/src/core/permissions/permissions_constants.dart';
-import 'package:nyxx/src/typedefs.dart';
 
 /// Set of permissions ints
 class _PermissionsSet {
@@ -32,10 +29,17 @@ class PermissionOverrideBuilder extends PermissionsBuilder {
       : type = entity is IRole ? 0 : 1,
         id = entity.id,
         super();
+
+  @override
+  RawApiMap build() => {
+        ...super.build(),
+        "id": id.toString(),
+        "type": type,
+      };
 }
 
 /// Builder for permissions.
-class PermissionsBuilder {
+class PermissionsBuilder extends Builder {
   /// The raw permission code.
   int? raw;
 
@@ -189,12 +193,12 @@ class PermissionsBuilder {
 
   /// Calculates permission int
   int calculatePermissionValue() {
-    final set = build();
+    final set = _calculatePermissionSet();
 
     return set.allow & ~set.deny;
   }
 
-  _PermissionsSet build() {
+  _PermissionsSet _calculatePermissionSet() {
     final permissionSet = _PermissionsSet();
 
     _apply(permissionSet, createInstantInvite, PermissionsConstants.createInstantInvite);
@@ -234,6 +238,16 @@ class PermissionsBuilder {
     return permissionSet;
   }
 
+  @override
+  RawApiMap build() {
+    _PermissionsSet permissionSet = _calculatePermissionSet();
+
+    return {
+      "allow": permissionSet.allow.toString(),
+      "deny": permissionSet.deny.toString(),
+    };
+  }
+
   void _apply(_PermissionsSet perm, bool? applies, int constant) {
     if (applies == null) {
       return;


Test patch:
diff --git a/test/unit/builders_test.dart b/test/unit/builders_test.dart
index 037f17e7..5d1c1846 100644
--- a/test/unit/builders_test.dart
+++ b/test/unit/builders_test.dart
@@ -46,7 +46,12 @@ main() {
       final builder = TextChannelBuilder.create("test");
       builder.permissionOverrides = [PermissionOverrideBuilder.from(0, Snowflake.zero(), Permissions.empty())];
 
-      final expectedResult = {'permission_overwrites': [{'allow': 0, 'deny': 122406567679}], 'name': 'test'};
+      final expectedResult = {
+        'permission_overwrites': [
+          {'allow': "0", 'deny': "122406567679", 'id': '0', 'type': 0}
+        ],
+        'name': 'test'
+      };
       expect(builder.build(), expectedResult);
     });
   });
@@ -98,31 +103,29 @@ main() {
 
   test('PermissionOverrideBuilder', () {
     final builder = PermissionOverrideBuilder(0, Snowflake.zero());
-    expect(builder.build().build(), equals({"allow": 0, "deny": 0}));
+    expect(builder.build(), equals({"allow": "0", "deny": "0", 'id': '0', 'type': 0}));
 
     final fromBuilder = PermissionOverrideBuilder.from(0, Snowflake.zero(), Permissions.empty());
-    expect(fromBuilder.build().build(), equals({"allow": 0, "deny": 122406567679}));
+    expect(fromBuilder.build(), equals({"allow": "0", "deny": "122406567679", 'id': '0', 'type': 0}));
     expect(fromBuilder.calculatePermissionValue(), equals(0));
 
     final ofBuilder = PermissionOverrideBuilder.of(MockMember(Snowflake.zero()))
       ..sendMessages = true
       ..addReactions = false;
 
-    expect(ofBuilder.build().build(), equals({"allow": 1 << 11, "deny": 1 << 6}));
+    expect(ofBuilder.build(), equals({"allow": (1 << 11).toString(), "deny": (1 << 6).toString(), 'id': '0', 'type': 1}));
     expect(ofBuilder.calculatePermissionValue(), equals(1 << 11));
   });
 
   group('MemberBuilder', () {
     test('channel empty', () {
-      final builder = MemberBuilder()
-          ..channel = Snowflake.zero();
+      final builder = MemberBuilder()..channel = Snowflake.zero();
 
       expect({}, builder.build());
     });
 
     test('channel with value', () {
-      final builder = MemberBuilder()
-        ..channel = Snowflake(123);
+      final builder = MemberBuilder()..channel = Snowflake(123);
 
       expect({'channel_id': '123'}, builder.build());
     });
@@ -130,17 +133,17 @@ main() {
     test('timeout empty', () {
       final now = DateTime.now();
 
-      final builder = MemberBuilder()
-        ..timeoutUntil = now;
+      final builder = MemberBuilder()..timeoutUntil = now;
 
       expect({'communication_disabled_until': now.toIso8601String()}, builder.build());
     });
 
     test('roles serialization', () {
-      final builder = MemberBuilder()
-        ..roles = [Snowflake(1), Snowflake(2)];
+      final builder = MemberBuilder()..roles = [Snowflake(1), Snowflake(2)];
 
-      expect({'roles': ['1', '2']}, builder.build());
+      expect({
+        'roles': ['1', '2']
+      }, builder.build());
     });
   });
 
@@ -156,29 +159,38 @@ main() {
 
       final result = builder.build();
 
-      expect(result, equals({'embeds': [{'description': 'test1'}, {'description': 'test2'}]}));
+      expect(
+          result,
+          equals({
+            'embeds': [
+              {'description': 'test1'},
+              {'description': 'test2'}
+            ]
+          }));
     });
 
     test('text', () {
       final dateTime = DateTime(2000);
 
       final builder = MessageBuilder()
-          ..appendSpoiler('spoiler')
-          ..appendNewLine()
-          ..appendItalics('italics')
-          ..appendBold('bold')
-          ..appendStrike('strike')
-          ..appendCodeSimple('this is code simple')
-          ..appendMention(MockMember(Snowflake.zero()))
-          ..appendTimestamp(dateTime)
-          ..appendCode('dart', 'final int = 124;');
-
-      expect(builder.build(), equals({
-        'content': '||spoiler||\n'
-            '*italics***bold**~~strike~~`this is code simple`<@0><t:${dateTime.millisecondsSinceEpoch ~/ 1000}:f>\n'
-            '```dart\n'
-            'final int = 124;```'
-      }));
+        ..appendSpoiler('spoiler')
+        ..appendNewLine()
+        ..appendItalics('italics')
+        ..appendBold('bold')
+        ..appendStrike('strike')
+        ..appendCodeSimple('this is code simple')
+        ..appendMention(MockMember(Snowflake.zero()))
+        ..appendTimestamp(dateTime)
+        ..appendCode('dart', 'final int = 124;');
+
+      expect(
+          builder.build(),
+          equals({
+            'content': '||spoiler||\n'
+                '*italics***bold**~~strike~~`this is code simple`<@0><t:${dateTime.millisecondsSinceEpoch ~/ 1000}:f>\n'
+                '```dart\n'
+                'final int = 124;```'
+          }));
 
       expect(builder.getMappedFiles(), isEmpty);
       expect(builder.canBeUsedAsNewMessage(), isTrue);