feat/40-reroll-on-disinterest #54
No reviewers
Labels
No Milestone
No project
No Assignees
2 Participants
Notifications
Due Date
No due date set.
Dependencies
No dependencies set.
Reference: kenobi/jellyfin-discord-bot#54
Loading…
Reference in New Issue
Block a user
No description provided.
Delete Branch "feat/40-reroll-on-disinterest"
Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?
closes #40
feat/40-reroll-on-disinterestto WIP: feat/40-reroll-on-disinterest@ -0,0 +9,4 @@
export async function execute(messageReaction: MessageReaction, user: User) {
if (user.id == client.user?.id)
logger.info('Skipping bot reaction')
need to return here
added
@ -0,0 +12,4 @@
if (!messageReaction.message.guild) return 'No guild'
if (messageIsVoteMessage(reactedUponMessage)) {
logger.debug(`${reactedUponMessage.id} is vote message`, { requestId, guildId })
if (messageReaction.message.reactions.cache.find(reaction => reaction.emoji.toString() == NONE_OF_THAT)) {
duplicate check. already checked before call in handleMessageReactionAdd
removed
@ -0,0 +10,4 @@
public async handleNoneOfThatVote(messageReaction: MessageReaction, user: User, reactedUponMessage: Message, requestId: string, guildId: string) {
if (!messageReaction.message.guild) return 'No guild'
if (messageIsVoteMessage(reactedUponMessage)) {
Move check to handleMessageReactionAdd
moved
@ -0,0 +1,31 @@
Rename file to something like "handleVoteReaction" to make it separate from future reactionadd handlers
Agreement to use handleMessageReactionAdd as a 'reaction pre processor' to hand off reaction emoji to other handling methods and keep the eventHandler itself generic.
@ -0,0 +1,11 @@
import { Message } from "discord.js";
export function messageIsVoteMessage(msg: Message): boolean {
Would like an enum of message types and a method with input message and output array of message types for possible messages with multiple things.
Also maybe remove message at start of function names to improve visibility by shorter function name. "isVoteMessage(message)" is precise and readable enough
@ -1,5 +1,8 @@
import { createLogger, format, transports } from "winston"
import { config } from "./configuration"
import { v4 } from "uuid"
export const newRequestId = v4()
why?
This is a complete and total, boneheaded error :D
The intention was to present an more memorizable way to get a new uuid/v4.
Instead this just sets it once, making it useless.
It should be an exported function, returning the output of
v4()
and the callsite would beconst requestId = newRequestId()
This would prevent the
import { v4 as uuid } from "uuid"
orconst requestId = v4()
which, I think, are tedious or non-obvious.If you think this is a bad idea, I will gladly revert it though :)
I dont think it's bad tbh if we dont need the v4 as uuid import then. But I would also like an eventhandler framework where we would prevent usual boilerplate code like looking for guild Id or making a request id and put those base checks and things in a base class and just have handling code in a subclass. But that is quite hard as I learned
@ -81,6 +85,11 @@ export class ExtendedClient extends Client {
logger.info(`Error refreshing slash commands: ${error}`)
}
}
private async fetchAnnouncementChannelMessage(channels: Collection<string, TextChannel>): Promise<void> {
why is this necessary?
It's complicated. I put in some JSDoc comments:
Figured something like that. Thanks for the comment
@ -0,0 +1,15 @@
export enum Emotes { "1️⃣", "2️⃣", "3️⃣", "4️⃣", "5️⃣", "6️⃣", "7️⃣", "8️⃣", "9️⃣", "🔟" }
export const NONE_OF_THAT = "❌"
Can this be integrated into Emoji? Maybe provide method to only get number emojis then? By definition "NONE_OF_THAT" is also an emoji
Emotes got renamed to validVoteEmotes
fb4ab59dc6
preliminary solution
Also linting warnings in some files because of unused imports
fixed
WIP: feat/40-reroll-on-disinterestto WIP: feat/40-reroll-on-disinterestWIP: feat/40-reroll-on-disinterestto WIP: feat/40-reroll-on-disinterestFunktionalität scheint endlich fertig implementiert zu sein.
Jetzt sollten noch Unit Tests folgen.
WIP: feat/40-reroll-on-disinterestto feat/40-reroll-on-disinterest@ -39,3 +31,3 @@
return
}
let message = `[Abstimmung] für https://discord.com/events/${event.guildId}/${event.id}\n<@&${config.bot.announcement_role}> Es gibt eine neue Abstimmung für die nächste Watchparty ${createDateStringFromEvent(event, event.guildId, requestId)}! Stimme hierunter für den nächsten Film ab!\n`
const sentMessageText = client.voteController.createVoteMessageText(event.id, event.scheduledStartAt, movies, event.guild?.id ?? "", requestId)
Warum erst eine message erstellen lassen die dann vom gleichen controller in der nächsten Zeile verschickt wird? Fände besser dem Controller zu sagen "Schick ne Vote message, hier sind die Infos die du brauchst"
True. Habe das handling in den vote controller verlegt.
Die Input Parameter sind dadurch allerdings lang genug geworden, dass ich ein separates Interface dafür erstellt habe.
4600820889
@ -58,3 +36,1 @@
sentMessage.react(NONE_OF_THAT)
// sentMessage.pin() //todo: uncomment when bot has permission to pin messages. Also update closepoll.ts to only fetch pinned messages
sentMessage.pin()
Das pinnen sollte glaube ich auch nicht das event übernehmen sondern der Controller
Evtl halt über nen Parameter bestimmen. Auf lange Sicht gehört das eigentliche verschicken der Message eh in einen eigenen controller aber der voteController kann die Daten für eine voteMessage entgegennehmen die aufbereiten und dann über den messageController die eigentliche message schicken lassen
4600820889
gelöst durch das Handling in #54 (comment)
Es gibt noch ein Auftreten vom 'separaten' Pinnen (vote.controller.ts:114). Dort ist ein bisschen mehr refactoring nötig um die Input Paramter zu füllen.
Dieser Kommentar sollte allerdings fertig behandelt sein.
@ -0,0 +37,4 @@
return client.voteController.handleNoneOfThatVote(messageReaction, reactedUponMessage, requestId, guildId)
}
if (messageReaction.emoji.toString() === Emoji.one) {
// do something
?
Replace this do something with a meaningful todo
removed
03b6a30ffa
@ -0,0 +57,4 @@
}
}
private async removeMessage(msg: Message): Promise<Message<boolean>> {
Parameter die eine Message sind heißen oft "message", manchmal aber auch "msg". sollte vereinheitlicht sein
66507cb08f
fc64728a78
Jetzt sollten alle Occurences beseitigt sein.
@ -0,0 +63,4 @@
}
return await msg.delete()
}
public isAboveThreshold(vote: Vote): boolean {
above WHAT threshold? What does it do??
20da25f2bf
Mit einem Kommentar versehen und entsprechend deines vorschlags umbenannt
@ -0,0 +69,4 @@
return aboveThreshold
}
public async handleReroll(voteMessage: VoteMessage, guildId: string, requestId: string) {
//get movies that already had votes to give them a second chance
comment is misleading, voteinfo is also used to get the eventid and eventdate in line 93
119343c916
@ -0,0 +64,4 @@
return await msg.delete()
}
public isAboveThreshold(vote: Vote): boolean {
const aboveThreshold = (vote.count - 1) >= 1
threshold seems to be a magic number
Or rename method to "hasAtLeastOneVote"
296a490e93
done
@ -0,0 +90,4 @@
// create new message
logger.info(`Creating new poll message with new movies: ${movies}`, { requestId, guildId })
const message = this.createVoteMessageText(voteInfo.eventId, voteInfo.eventDate, movies, guildId, requestId)
is not a message, only messagetext
a455fd8ff7
done
@ -0,0 +73,4 @@
const voteInfo: VoteMessageInfo = await this.parseVoteInfoFromVoteMessage(voteMessage, requestId)
let movies: string[] = Array()
if (config.bot.reroll_retains_top_picks) {
maybe extract this if-else to a method to keep code more compact
7d794a8001
done
@ -0,0 +105,4 @@
}
const sentMessage = await this.sendVoteMessage(message, movies.length, announcementChannel)
sentMessage.pin()
why not pin message in method above?
7d794a8001
should have been moved to prepareAndSendVoteMessage()
@ -0,0 +147,4 @@
}
public parseEventDateFromMessage(message: string, guildId: string, requestId: string): Date {
logger.warn(`Falling back to RegEx parsing to get Event Date`, { guildId, requestId })
const datematcher = RegExp(/((?:0[1-9]|[12][0-9]|3[01])\.(?:0[1-9]|1[012])\.)(?:\ um\ )((?:(?:[01][0-9]|[2][0-3])\:[0-5][0-9])|(?:[2][4]\:00))!/i)
wtf :D
it's magic ✨
@ -0,0 +171,4 @@
return message
}
public async sendVoteMessage(message: string, movieCount: number, announcementChannel: TextChannel) {
rename message to messageText
done
ca99987a20
@ -0,0 +227,4 @@
this.updateEvent(event, votes, guild, guildId, requestId)
this.sendVoteClosedMessage(event, votes[0].movie, guildId, requestId)
}
lastMessage.unpin() //todo: uncomment when bot has permission to pin/unpin
remove todo
removed
ca99987a20
@ -0,0 +254,4 @@
logger.debug(`Found events: ${JSON.stringify(voteEvents, null, 2)}`, { guildId, requestId })
if (!voteEvents || voteEvents.length <= 0) {
logger.error("Could not find vote event. Cancelling update!", { guildId, requestId })
cancelling uptade log seems wrong here
6d40930dc1
done
@ -0,0 +247,4 @@
}
return votes
}
public async getEvent(guild: Guild, guildId: string, requestId: string): Promise<GuildScheduledEvent | null> {
rename to "getOpenVoteEvent", since other events get filtered out
ca99987a20
done
renamed to getOpenPollEvent
4e9fe587b0
renamed
@ -0,0 +259,4 @@
}
return voteEvents[0]
}
public async updateEvent(voteEvent: GuildScheduledEvent, votes: Vote[], guild: Guild, guildId: string, requestId: string) {
should be renamed so it's clear what the event gets updated with and what it looks like in the end and what kind of event gets updated (I guess open poll events)
ca99987a
done
@ -0,0 +271,4 @@
}
public async sendVoteClosedMessage(event: GuildScheduledEvent, movie: string, guildId: string, requestId: string): Promise<Message<boolean>> {
const date = event.scheduledStartAt ? format(event.scheduledStartAt, "dd.MM.") : "Fehler, event hatte kein Datum"
const time = event.scheduledStartAt ? format(event.scheduledStartAt, "HH:mm") : "Fehler, event hatte kein Datum"
"Fehler, event hatte keine Uhrzeit" pls
ca99987a20
done
@ -0,0 +280,4 @@
const announcementChannel = this.client.getAnnouncementChannelForGuild(guildId)
logger.info("Sending vote closed message.", { guildId, requestId })
if (!announcementChannel) {
const errorMessages = "Could not find announcement channel. Please fix!"
why plural?
Because this was unintentional :)
ca99987a20
fixed
@ -0,0 +286,4 @@
}
return announcementChannel.send(options)
}
private extractMovieFromMessageByEmote(lastMessages: Message, emote: string): string {
rename parameter lastMessages to message. "last" seems to be very specific as a parameter, also false plural
ca99987a20
done
Kp, was man hier rein schreibt, hab n paar Dinge angemerkt