chylex / Discord-History-Tracker

Desktop app & browser script that saves Discord chat history into a file, and an offline viewer that displays the file.
https://dht.chylex.com
MIT License
475 stars 83 forks source link

Incorrect `parent_id` check, failure to save channel topic and position? #232

Closed will-ca closed 11 months ago

will-ca commented 1 year ago

What is the reason for skipping setting extra.position and extra.topic if parent_id is available at discord.js:227?

https://github.com/chylex/Discord-History-Tracker/blob/18f5823f2a5502f365d54d01135e0b0e18e59d84/app/Resources/Tracker/scripts/discord.js#L227-L233

As the first mention of parent_id in discord.js appears to be in in 73bf16a21e8f5d1106b4cd253cd34a6b0ad78ca0, I suspect this may be used as a heuristic for handling Threads?

However, it is possible for a channel to have a parent_id, and also a valid position and topic, because parent_id also appears to be used to identify the little channel category things which they added at some point.

For example, the top-level/non-threaded "introduce_yourself" and "general" channels in the screenshotted server above have a parent_id pointing to the "COMMUNITY" category.

(This can be detected by searching in the DOM for their parent_id, which is set in the data-list-item-id="channels___XXX" attribute of a class="mainContent_XXX" <div> for the "COMMUNITY" text label.)

The channels have valid .position and .topic properties too, but I assume they would not be saved due to the presence of .parent_id at the discord.js:227 skip.

will-ca commented 1 year ago

The channels have valid .position and .topic properties too, but I assume they would not be saved due to the presence of .parent_id at the discord.js:227 skip.

Okay, yeah. Confirmed channel order is missing, and channels are displayed alphabetized by name instead in the viewer, using desktop version of DHT in a server with channels sorted under categories.

Why include the if (obj.parent_id) at all, instead of just always assigning all three parameters and letting it read undefined (or ?? -1 or whatever) when missing?

chylex commented 1 year ago

In https://github.com/chylex/Discord-History-Tracker/commit/73bf16a21e8f5d1106b4cd253cd34a6b0ad78ca0 it was always storing position and topic, it was changed in https://github.com/chylex/Discord-History-Tracker/commit/09dce7b062cddd827af11454588b0f7853f70aab and I missed it while testing. There's probably a better way to check if a channel is a thread, which should be used instead.

Threads don't store topics because the topic is basically the contents of the original message, but since the check is wrong it's not storing topics for legitimate channels.

Position is a slightly different issue, DHT does not store anything about categories so it doesn't know their names or order, and cannot determine the order of categorized channels. For now, not storing the position of channels with a parent is correct.

chylex commented 11 months ago

After re-checking, the channel position Discord provides appears to be global with respect to the entire server, including channels that are not even accessible to you, so not storing position of categorized channels was in fact a bug.