Criar comando !jobs #46
Conversation
NightmareR1
commented
Sep 15, 2022
NightmareR1
commented
Co-authored-bY: C0ffeeL0ver
|
|
.vs/VSWorkspaceState.json
Outdated
| { | ||
| "ExpandedNodes": [""], | ||
| "SelectedNode": "\\index.js", | ||
| "PreviewInSolutionExplorer": false | ||
| } |
There was a problem hiding this comment.
Penso que todos os ficheiros dentro da pasta .vs tenham sido enviados por engano. Consegues removê-los?
Co-authored-by: Nightmare <[email protected]>
Merge branch 'main' of https://github.com/devpt-org/welcome-bot
|
|
adição do error/warning/info ao loggerservice (com cores)
| if (context.guildId === undefined) { | ||
| throw new Error(`Guild not found!`); | ||
| } |
There was a problem hiding this comment.
Em que situação poderia acontecer isto?
There was a problem hiding this comment.
o type do discord faz com que o guild obtido através da mensagem possa ser indefinido
| throw new Error(`Guild not found!`); | ||
| } | ||
|
|
||
| this.chatService.deleteMessageFromChannel(this.loggerService, context.message.id, context.channel.id); |
There was a problem hiding this comment.
Diria para eventualmente implementarmos um Dependency Injector container pois começamos a ter mais dependências e era bom termos isto gerido automaticamente. Se possível vejo se consigo implementar isso num outro PR para que possamos usar aqui.
| const capturedMessages: string[] = await this.chatService.readMessagesFromChannel( | ||
| this.loggerService, | ||
| createdChannel, | ||
| context.guildId, | ||
| context.user, | ||
| this.messageRepository.getJobQuestions() | ||
| ); |
There was a problem hiding this comment.
Parece-me que esta função está a fazer demasiadas coisas tendo em conta a quantidade de parâmetros que se está a passar.
Sugeria separar a parte das job questions para um outro método.
E se passarmos o user no método, deveríamos alterar o nome do método para readMessagesFromChannelAndUser.
|
|
||
| this.chatService.deleteChannel(this.loggerService, createdChannel); | ||
|
|
||
| const embed: EmbedMessage = await this.chatService.buildEmbedFromCapturedMessages( |
There was a problem hiding this comment.
capturedMessages é um conceito deste use case, não de domínio, como tal, um chatService não sabe o que são mensagens capturadas. Talvez buildEmbedFromMessageCollection ou algo semelhante?
|
|
||
| const embed: EmbedMessage = await this.chatService.buildEmbedFromCapturedMessages( | ||
| this.loggerService, | ||
| this.messageRepository.getJobQuestions(), |
There was a problem hiding this comment.
Tem demasiadas responsabilidades também. Se o objetivo dele é criar um embed de key (jobQuestion) + resposta, diria que não devemos misturar os dois.
Sugeria usarmos o Builder Pattern e criar um EmbedBuilder.
Basicamente, aqui depois poderias fazer algo do género:
const embed: Embed = new EmbedBuilder()
.withAuthor(new Author('username'))
.withReactions("👍", "👎")
.withTitle('Test')
.withFields(
new EmbedField('key-1', 'value-1'),
new EmbedField('key-1', 'value-2'),
)
.build();O próprio Discord tem um já (new MessageEmbed()), que é só deles, poderíamos criar um novo como ilustrei acima.
| const guild = await this.client.guilds.fetch(guildId); | ||
| const author = (await guild.members.fetch(user.id)).user; |
There was a problem hiding this comment.
Talvez possamos ir buscar o user diretamente sem usar as guilds, e assim nem precisamos de passar o guildId para aqui?
https://stackoverflow.com/a/65980351
Quando menos coisas específicas de implementações usarmos melhor, porque senão isto pode trazer-nos problemas caso queiramos usar outra implementação que não Discord e pode a obrigar-nos a mudar um pouco a arquitetura.
| if (channel === null) { | ||
| throw new Error(`Channel with id ${channelId} not found!`); | ||
| } |
There was a problem hiding this comment.
Porque não passar diretamente o channel para a função e evitávamos uma outra verificação (tendo em conta que se temos uma instância válida de um Canal, é porque este já foi validado)?
| .setDescription(embed.description) | ||
| .addFields(embed.fields) | ||
| .setTimestamp(embed.timestamp) | ||
| .setFooter(embed.footer); | ||
| channel.send({ embeds: [messageEmbed] }).then((m: Message) => { | ||
| m.react("👍"); | ||
| m.react("👎"); | ||
| m.startThread({ | ||
| name: `${author.username}`, | ||
| }).then((thread: ThreadChannel) => { | ||
| thread.send(`Thread automatically created by ${author.username} in <#${channel.id}>`); | ||
| }); |
There was a problem hiding this comment.
Deixei umas notas sobre isto mais acima. Sugeria usarmos um Builder nosso que criava o Builder do Discord. Assim consegues ter flexibilidade no Use Case para criares objetos de domínio, e opcionais caso necessites.
|
@NightmareR1 ainda planeias continuar com este MR? |
