Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

馃悰 Doesn't respect channel's default archive duration #115

Closed
LekoArts opened this issue Mar 21, 2022 · 8 comments 路 Fixed by #158
Closed

馃悰 Doesn't respect channel's default archive duration #115

LekoArts opened this issue Mar 21, 2022 · 8 comments 路 Fixed by #158
Labels
confirmed bug 馃 Something isn't working

Comments

@LekoArts
Copy link

LekoArts commented Mar 21, 2022

Describe the bug

Hi!
Thanks for creating and hosting the bot, I really appreciate it!

I'm currently using the hosted version and the server I've added it to has the maximum boost level, so one can set the archive duration to 1 week.

In #34 I saw:

to clamp the auto-archive duration to the maximum ACTUAL allowed value

I understand the reasoning for this at that time but I think it's a bug that the default duration that is selected for the channel is not evaluated first before falling back to the actual allowed value.

So I'm talking about this channel setting at the bottom:

Bildschirmfoto 2022-03-21 um 10 11 25

Right now Needle sets every thread to 1 week which is too long, I want to have it at 24 hours.

Steps to reproduce the bug

  1. Server with boosted levels to have more duration than 1 week
  2. Set default duration to 24 hours in channel
  3. Needle uses boosted options, not 24 hours

Expected behavior

I expect the following:

  • Use channel setting
  • If channel setting is set to something that doesn't exist anymore (as server is not boosted anymore) fallback to current max value behavior
@LekoArts LekoArts added the potential bug 馃悰 Has not been replicated yet label Mar 21, 2022
@MarcusOtter MarcusOtter added confirmed bug 馃 Something isn't working and removed potential bug 馃悰 Has not been replicated yet labels Mar 21, 2022
@MarcusOtter
Copy link
Owner

@LekoArts this is a bug with Discord, see the FAQ

@MarcusOtter
Copy link
Owner

MarcusOtter commented Mar 21, 2022

Actually I will re-open this and do some experimenting. It could be that Discord says the default auto-archive duration is null when it's unset, in which case we default to using MAX but the UI shows that the default is 24h.

I know for a fact that this is un-synced with Discord's UI from a previous bug so up until this point I have just assumed all of it is Discord's fault, but I should double check that we're not contributing to the problem here.

I don't want to rely on how things look in the UI vs what the value we are actually getting is, but we might have to do it anyways. It's a known bug, but thank you for pointing this out specifically which made me think about it some more:

I understand the reasoning for this at that time but I think it's a bug that the default duration that is selected for the channel is not evaluated first before falling back to the actual allowed value.

@MarcusOtter MarcusOtter reopened this Mar 21, 2022
@MarcusOtter
Copy link
Owner

MarcusOtter commented Mar 21, 2022

This is the function that calculates the final auto-archive duration:

export function getSafeDefaultAutoArchiveDuration(channel: TextChannel | NewsChannel): ThreadAutoArchiveDuration {
	const archiveDuration = channel.defaultAutoArchiveDuration;
	if (!archiveDuration || archiveDuration === "MAX") return "MAX";

	const highest = getHighestAllowedArchiveDuration(channel);
	return archiveDuration > highest
		? highest
		: archiveDuration;
}

The only time this would make the auto-archive delay higher is if archiveDuration is falsy. Though an argument could be made that we are not technically making the archiveDuration higher since there is none to begin with, and what the UI shows is not really of our business since that could change at any time. But we should stick with what makes sense now.

To clarify: I haven't even tested if it's the falsy case that trips us up here or if it's completely on Discord.

@LekoArts
Copy link
Author

Hi, thanks for pointing to the FAQ. When I followed the steps there I "fixed" my issue. Didn't know that the wiki existed 馃槵

I can also try on my end the API with newly created channels and check what one gets for the auto-archive duration. The code snippet you gave makes sense, I'd probably also have written it this way 馃槃 It would be unfortunate if Discord's API doesn't give back the value that is the default as an actual value.

@LekoArts
Copy link
Author

LekoArts commented Mar 21, 2022

While looking at https://discord.com/developers/docs/resources/channel#get-channel I found:

auto_archive_duration | integer | duration in minutes to automatically archive the thread after recent activity, can be set to: 60, 1440, 4320, 10080

And also: discordjs/discord.js#7687 + discordjs/discord.js#7688

Channels store the properties as numbers as-is from the API and the edit methods allow for a MAX helper. This pull request resolves discordjs/discord.js#7687 and resolves supplying MAX via GuildChannelManager#edit() not working as intended.

@LekoArts
Copy link
Author

Ok, so I've created my own bot for testing purposes and I logged out https://discord.js.org/#/docs/discord.js/stable/class/BaseGuildTextChannel via slash command.

On a newly created channel the response doesn't contain defaultAutoArchiveDuration. A channel where I switched the 24 hours to something else and back now contains "defaultAutoArchiveDuration": 1440,

@MarcusOtter
Copy link
Owner

doesn't contain defaultAutoArchiveDuration

As in, it's null/undefined? That would explain it then.

@LekoArts
Copy link
Author

Yes, It鈥檚 undefined, so doesn鈥檛 exist in the API response.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
confirmed bug 馃 Something isn't working
Projects
None yet
2 participants