GuildChannel#permissionsLocked incorrect when parent category updated

[MattIPv4]

Please describe the problem you are having in as much detail as possible:

Assume a situation where a text channel is in a category, with the channel's permissions set in Discord to sync with the category.

Accessing GuildChannel#permissionsLocked for that channel will confirm this.

Now, assume that the permissions overwrites in the category are updated, and you have an event listener set for channel updates.

First, you will see the update come through for the category permissions overwrites being updated, which is all fine (or is it).

Then, you will see the update come through for the channel's permissions overwrites being updated. At this point though, if you access GuildChannel#permissionsLocked on the old channel, it will report that the permissions were not locked, even though they were, and GuildChannel#permissionsLocked on the new channel will report that the permissions are not locked.

This happens (from my understanding) because we've already received the update event for the category overwrites changing, so both the old and new channel in the channel update event have the same, already updated, category as their parent which is used for the GuildChannel#permissionsLocked calculation.

Include a reproducible code sample here, if possible:

client.on('channelUpdate', (oldChannel, newChannel) => {
    const permsUpdated = newChannel.permissionOverwrites.size !== oldChannel.permissionOverwrites.size
        || newChannel.permissionOverwrites.some(overwrite => !oldChannel.permissionOverwrites.has(overwrite.id)
            || !oldChannel.permissionOverwrites.get(overwrite.id).allow.equals(overwrite.allow)
            || !oldChannel.permissionOverwrites.get(overwrite.id).deny.equals(overwrite.deny));

    if (!permsUpdated) return;

    if (newChannel.type === 'category') {
        console.log('Category permissions updated');
        return;
    }

    if (newChannel.type === 'text') {
        console.log('Text channel permissions updated');
        console.log('Synced before update:', oldChannel.permissionsLocked);
        console.log('Synced after update:', newChannel.permissionsLocked);
        return;
    }
});

Further details:

• discord.js version: 13.0.0 (dev)
• Node.js version: 14.15.4
• Operating system: macOS
• Priority this issue should have – please be realistic and elaborate if possible: P2 -- its an annoying issue that I'd love to see a fix or workaround for, but also somewhat minor

Relevant client options:

• partials: channel (though both GuildChannel and Channel do not have a partial prop)

• gateway intents: guilds

• other: none

• I have also tested the issue on latest master, commit hash: fe6cc0c15dde99caa1049d35f75b9335ace1721d

[almostSouji]

could this be related to https://github.com/discordjs/discord.js/issues/6082 and accordingly be fixed now?

[MattIPv4]

Nope, this still happens as of f72ce7c136cf2dfe31a67b190c00e30ba7d70bfa. I believe this is just how Discord sends the permissions updates for the parent channel and the children, but it would be nice if d.js handled it better.

[nimit2801]

Hey @MattIPv4, from what I understood in this issue, in the child category the new and old channel gives the same out when called for permissionsLocked.

Also, what is permissionOverwrites.some?

[MattIPv4]

in the child category the new and old channel gives the same out when called for permissionsLocked.

That is one consequence of the issue, yes, but it is not the root cause. The root cause is that the parent receives an update first, so when the child update comes through the child is perceived to be out of sync with the parent, when in reality the child is being updated because it is in sync with the parent.

Also, what is permissionOverwrites.some?

https://discord.js.org/#/docs/collection/master/class/Collection?scrollTo=some

(This is now actually permissionOverwrites.cache.some per recent changes iirc)

[nimit2801]

Do you think it is possible that the child is updated first and then the parent channel, which could be a possible reason why this is happening? I'm testing trying to debug and this issue and had this thought.

Thoughts @MattIPv4 ?

[MattIPv4]

No, the parent is updated first. If you run my repro code locally you will be able to observe the parent update event arrives first, which updates the d.js cache for both the parent and therefore the child. The child update event then arrives after, at which point the parent has already been updated and so the child is perceived as out of sync.

[nimit2801]

I'm trying to run your code and getting this error TypeError: newChannel.permissionOverwrites.some is not a function Node: 14.6.0

[MattIPv4]

Yup, as I noticed two replies above, with recent changes to master I believe it is now permissionOverwrites.cache.some...

[nimit2801]

From what I understood and debugged only we can see channel category being updated for new and old channel. So we need to handle the events in such a way that the differences in the permission would be seen in the child channel too(Text, Voice, etc). And yes you were right first the category is being updated and then the child channels.

[nimit2801]

Hey @MattIPv4 so I tried this:

client.on('channelUpdate', (oldChannel, newChannel) => {
    console.log(newChannel.type);
    if (newChannel.type === 'GUILD_CATEGORY') {
        console.log('Category permissions updated');
        return;
    }

    if (newChannel.type === 'GUILD_TEXT') {
        console.log('Text channel permissions updated');
        console.log('Synced before update:', oldChannel.permissionsLocked);
        console.log('Synced after update:', newChannel.permissionsLocked);
        return;
    }
    if (newChannel.type === 'GUILD_VOICE') {
        console.log('Voice channel permissions updated');
        console.log('Synced before update:', oldChannel.permissionsLocked);
        console.log('Synced after update:', newChannel.permissionsLocked);
        return;
    }
});

Output:

Is this the output you were expecting?

Node: 14.6.0 Discord.js@dev

[MattIPv4]

Yeah, that looks like it also demonstrates the issue here -- permissionsLocked is incorrectly reported as false for the children updates, because the category has already been updated in the cache.

[nimit2801]

Yeah, that looks like it also demonstrates the issue here -- permissionsLocked is incorrectly reported as false for the children updates, because the category has already been updated in the cache.

Hey, Matt isn't it how it is supposed to be? Let's assume: Category Channel Updated -> TextChannel Updated so the oldChannel.permissionsLocked (text channel) would be false because the parent channel has different perms. And the updated NewChannel.permissionsLocked (text channel) should be true because now the permissions are the same. Or am I understanding it incorrectly?

[MattIPv4]

Yes, based on the current logic, it is the expected behaviour. However, given that permissionsLocked is a helper provided by D.js rather than by Discord, it would be nice for it to be improved, so that it can handle this situation and correctly reflect that the permissions were set to be in sync the whole time.

[nimit2801]

What should be the desired output here instead, how can we change the handling here?

Can you give me an example?

[MattIPv4]

I would expect permissionsLocked to report true always here, as the permissions were still set to be synced, and were updating because they are synced with the parent category.

[nimit2801]

console.log('Synced before update:', oldChannel.permissionsLocked);
console.log('Synced after update:', newChannel.permissionsLocked);

So basically both should return true?

[MattIPv4]

Yes 👍, because they were still set to be synced, and were updated in exactly the same way the parent was, as they are synced.

[nimit2801]

This would then defeat the purpose, would it be better if we just had NewChannel.permissionsLocked so that it's always in sync?

[MattIPv4]

No, it does not defeat the purpose... the whole idea here is that I want to be able to accurately report when a channel has its permissions updated in such a way that means the permissions are no longer set to sync, or have been updated when they were previously not in sync and are now in sync.

This current behaviour results in continual false positives, as the value of permissionsLocked switches indicating that the sync of the channel permissions was updated, when in reality the option to sync wasn't updated, the permissions were in fact updating to remain in syc.

[nimit2801]

Oh, got it! I was not able to understand the sync thingy for a while now I get it. Ok, so we can actually do that by check the permissionsLocked of oldChannel(text channel) with the old Permission of the Category channel. This would result in oldChannel.permissionLocked to be true and the channel to be synced always if it was synced.

I hope this would solve the issues here?

[MattIPv4]

Yup, that sounds like the right theory.

[nimit2801]

So I went a bit into the folders and pointed where it permissions were getting allocated.

~~I hope this solves how the functioning of the sync~~ I hope this shows the desired output and pinpoints where things are allocated

Original

      const channelVal = this.permissionOverwrites.cache.get(key);
      const parentVal = this.parent.permissionOverwrites.cache.get(key);

After Changes made

      const channelVal = this.parent.permissionOverwrites.cache.get(key);
      const parentVal = this.parent.permissionOverwrites.cache.get(key);

This keeps it sync always @MattIPv4 @almostSouji Thoughts?

[MattIPv4]

To put it in simple terms, no, this doesn't sound like it solves the issue. It sounds like you're removing the concept of permissions overwrites on channels, which makes no sense at all.

[MattIPv4]

I recommend creating a PR though so that proposed changes to solve this issue can be discussed there, directly on your proposed changes, rather than in the issue :)

[nimit2801]

To put it in simple terms, no, this doesn't sound like it solves the issue. It sounds like you're removing the concept of permissions overwrites on channels, which makes no sense at all.

Ohh ok, I think I'll wait for someone to add more comments on how to solve this.

[monbrey]

permissionsLocked here isn't a clear value though - the API doesn't provide a boolean telling us that a channel is synced or not. It's a getter that compares the channel's overwrites to the parent's overwrites at runtime. Because it's a runtime check, there's several reasons this simply can't be accurate.

oldChildChannel.permissionsLocked would be comparing to the current, updated parent channels overwrites. I don't think its even possible to freeze the parent's old state for this, or somehow freeze the value of this getter, since as you said, the event for the update on the category is already processed before the children emit.

The closest thing I can think of to a solution would be some mess where when the Category is updated it checks all its children, sees if they're currently synced, and creates some sort of oldParent, oldParentOverwrites or oldPermissionsLocked on those channels before doing its own update, purely so that value could be used when the child event emits.

Side note - since this isn't a proper API-provided boolean doesnt this mean it can return a false positive if a parent and child have the same overwrites without being in sync?

[MattIPv4]

Side note - since this isn't a proper API-provided boolean doesnt this mean it can return a false positive if a parent and child have the same overwrites without being in sync?

From my quick testing, no -- Discord seems to potentially apply the same logic as D.js does. If you have a channel & parent sync'ed, and then update the channel to be out of sync, and then match that update in the parent the channel will show as sync'ed again.