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’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Perguntas Anónimas #62

Open
wants to merge 13 commits into
base: main
Choose a base branch
from
64 changes: 64 additions & 0 deletions application/usecases/readDirectMessage.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,64 @@
import { Message, Client, MessageEmbed, MessageActionRow, MessageButton } from "discord.js";
import { ChannelSlug } from "../../types";
import QuestionChatService from "./sendMessageToChannel/sendPerguntaToChannel";
import DiscordEmbedService from "../../infrastructure/service/discordEmbedService";
import ChatService from "../../domain/service/chatService";
import DiscordChatService from "../../infrastructure/service/discordChatService";
import ChannelResolver from "../../domain/service/channelResolver";

const askedRecently = new Set();
class DirectMessage {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Por norma temos declarado as classes com o mesmo nome do ficheiro e com o sufixo UseCase. Consegues alterar?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

feito

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

feito

constructor(private message: Message, private client: Client, private channelResolver: ChannelResolver) {}

async validate() {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Tendo em conta que a função validate retorna um bool e não um void, sugiro um isValid, penso fazer mais sentido.

E assim o teu if poderia ser um if (await isValid()) {.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

feito

if (askedRecently.has(this.message.author.id)) {
this.message.channel.send("Ainda não podes enviar outra pergunta. Tenta mais tarde.");
} else {
const chatService: ChatService = new DiscordChatService(this.client);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

O ChatService recebes no constructor, não tens de o criar.

As dependências são inicializadas no index.ts e depois passadas para cada UseCase.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

feito

if (this.message.author.id === this.client.user?.id) {
return false;
}

if (
this.message.channel.type === "DM" &&
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Como tinha sugerido o sudo, sugiro também passarmos este valor para um enum.

this.message.content.startsWith("!pergunta") &&
this.message.content.split(" ").length > 1 &&
this.message.content.length <= 1500
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Podemos passar o 1500 para uma constante (mesmo que dentro desta classe)?

É preferível ficar num nome percetível do que significa este valor do que ficar um valor arbitrário no código.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

feito

) {
askedRecently.add(this.message.author.id);
setTimeout(() => {
askedRecently.delete(this.message.author.id);
}, 60000);
return true;
}
chatService.sendDM(this.message.author.id, "Por favor usa o seguinte formato:\n!pergunta <mensagem>");
return false;
}
return true;
}
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

O que achas de uma abordagem mais deste género? Penso que fique mais explícito que tem que esperar os 60s para executar a ação seguinte.

Suggested change
setTimeout(() => {
askedRecently.delete(this.message.author.id);
}, 60000);
return true;
}
chatService.sendDM(this.message.author.id, "Por favor usa o seguinte formato:\n!pergunta <mensagem>");
return false;
}
return true;
}
await this.timeout(60000);
askedRecently.delete(this.message.author.id);
return true;
}
chatService.sendDM(this.message.author.id, "Por favor usa o seguinte formato:\n!pergunta <mensagem>");
return false;
}
return true;
}
function timeout(ms) {
return new Promise(resolve => setTimeout(resolve, ms));
}

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

só ficou esta por fazer porque ainda não percebi bem, tenho que ver com mais tempo, entretanto tento resolver !


async messageApprove() {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Trocaria talvez o verbo para ficar em primeiro? Isto porque lemos "aprovar mensagem" e não "mensagem aprovar" 😅 que achas?

Suggested change
async messageApprove() {
async approveMessage(): void {

const chatService: ChatService = new DiscordChatService(this.client);
const channelAnonQuestion = this.client.channels.cache.get(this.channelResolver.getBySlug(ChannelSlug.QUESTION));
const questionChatService: QuestionChatService = new DiscordEmbedService(this.client);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Não estamos a usar aqui o ChatService por alguma razão em particular? Do que vi o método que foi implemetando nesta classe pode ser implementado no ChatService, pareceu-me pelo menos.

E o UseCase não deveria ter conhecimento que estamos a usar Discord, ou qualquer outra plataforma.

const sentence = this.message.content.split(" ").slice(1).join(" ");
const buttons = new MessageActionRow().addComponents(
new MessageButton().setLabel("Aprovar").setStyle("SUCCESS").setCustomId("bt1"),
new MessageButton().setLabel("Eliminar").setStyle("DANGER").setCustomId("bt2")
);
const mensagem = new MessageEmbed().setColor("#0099ff").setTitle("Pergunta Anónima").setDescription(sentence);

await questionChatService.sendEmbedToChannel(
mensagem,
buttons,
this.channelResolver.getBySlug(ChannelSlug.MOD_CHANNEL)
);

await chatService.sendDM(
this.message.author.id,
`A tua pergunta foi colocada com sucesso.\nApós aprovação poderás visualizar no ${channelAnonQuestion} `
);
}
}

export default DirectMessage;
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
import { MessageEmbed, MessageActionRow } from "discord.js";
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Está a importar coisas específicas do Discord.js. Aqui deveria estar a importar alguma abstração. Podemos discutir isto com o @Adjilino que ele tem lá uns pontos semelhantes também no MR dele.


export default interface SendMessageToChannel {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

O nome do ficheiro está diferente do nome da classe e presumo que o queiras deixar todo em inglês?

sendEmbedToChannel(message: MessageEmbed, row: MessageActionRow, channelId: string): void;
}
2 changes: 2 additions & 0 deletions domain/service/channelResolver.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,8 @@ import { ChannelSlug } from "../../types";
const fallbackChannelIds: Record<ChannelSlug, string> = {
[ChannelSlug.ENTRANCE]: "855861944930402344",
[ChannelSlug.JOBS]: "876826576749215744",
[ChannelSlug.QUESTION]: "1066328934825865216",
[ChannelSlug.MOD_CHANNEL]: "987719981443723266",
};

export default class ChannelResolver {
Expand Down
2 changes: 2 additions & 0 deletions domain/service/chatService.ts
Original file line number Diff line number Diff line change
@@ -1,3 +1,5 @@
export default interface ChatService {
sendMessageToChannel(message: string, channelId: string): void;
deleteMessageFromChannel(messageId: string, channelId: string): void;
sendDM(message: string, userId: string): void;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Preferível ser mais explícito e evitar acrónimos.

Suggested change
sendDM(message: string, userId: string): void;
sendDirectMessageToUser(message: string, userId: string): void;

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

feito

}
64 changes: 62 additions & 2 deletions index.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
import { GuildMember, Message, Client, Intents } from "discord.js";
import { GuildMember, Message, Client, Intents, MessageEmbed } from "discord.js";
import * as dotenv from "dotenv";
import { ChannelSlug } from "./types";
import SendWelcomeMessageUseCase from "./application/usecases/sendWelcomeMessageUseCase";
import FileMessageRepository from "./infrastructure/repository/fileMessageRepository";
import ChatService from "./domain/service/chatService";
Expand All @@ -11,13 +12,21 @@ import CommandUseCaseResolver from "./domain/service/commandUseCaseResolver";
import ChannelResolver from "./domain/service/channelResolver";
import KataService from "./domain/service/kataService/kataService";
import CodewarsKataService from "./infrastructure/service/codewarsKataService";
import DirectMessage from "./application/usecases/readDirectMessage";

dotenv.config();

const { DISCORD_TOKEN } = process.env;

const client = new Client({
intents: [Intents.FLAGS.GUILDS, Intents.FLAGS.GUILD_MEMBERS, Intents.FLAGS.GUILD_MESSAGES],
partials: ["CHANNEL"],
intents: [
Intents.FLAGS.GUILDS,
Intents.FLAGS.DIRECT_MESSAGES,
Intents.FLAGS.DIRECT_MESSAGE_TYPING,
Intents.FLAGS.GUILD_MEMBERS,
Intents.FLAGS.GUILD_MESSAGES,
],
});

const messageRepository: MessageRepository = new FileMessageRepository();
Expand Down Expand Up @@ -61,3 +70,54 @@ client.on("messageCreate", (messages: Message) => {
loggerService.log(error);
}
});

client.on("message", async (message) => {
const directMessage = new DirectMessage(message, client, channelResolver);
const validationCheck = await directMessage.validate();
if (validationCheck) {
directMessage.messageApprove();
}
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Como referi num comment acima, se mudarmos o naming da função podemos inclusive melhorar a legibilidade deste check e até usar early returns para a validação. (se quisermos até podemos dar logo embed do await no if.

Suggested change
const validationCheck = await directMessage.validate();
if (validationCheck) {
directMessage.messageApprove();
}
const isValid = await directMessage.isValid();
if (isValid) {
return;
}
directMessage.messageApprove();

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

feito

});

client.on("interactionCreate", async (interaction) => {
if (!interaction.isButton()) return;

const {
message: {
id: messageId,
embeds: [{ description: messageContent }],
},
channelId,
} = interaction;

const userName = interaction.member?.user.username;

if (!messageContent) return;
const messageApprovedEmbed = new MessageEmbed()
.setColor("#0099ff")
.setTitle("Pergunta Anónima")
.setDescription(messageContent)
.setFooter({ text: `Aprovado por ${userName}` });

switch (interaction.customId) {
case "bt1":
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Este Ids deveriam ser algo mais específico caso queiramos vir a adicionar novos botões mais tarde.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

feito

{
const sentence = `PERGUNTA ANÓNIMA:\n${messageContent}`;
chatService.sendMessageToChannel(sentence, channelResolver.getBySlug(ChannelSlug.QUESTION));
interaction.update({ components: [], embeds: [messageApprovedEmbed] });
chatService.sendDM(interaction.user.id, "A tua mensagem foi aprovada.");
}
break;

case "bt2":
chatService.deleteMessageFromChannel(messageId, channelId);
chatService.sendDM(
interaction.user.id,
"A tua mensagem não foi aprovada.\nVerifica se está de acordo com as regras."
);
break;
default: {
console.log("default");
}
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Presumo que possamos retirar isto daqui?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

não retirei porque senão não passa no lint, dá para desativar esse check ?

}
});
26 changes: 25 additions & 1 deletion infrastructure/service/discordChatService.ts
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,31 @@ export default class DiscordChatService implements ChatService {
if (!channel.isText()) {
throw new Error(`Channel with id ${channelId} is not a text channel!`);
}

channel.send(message);
}

async deleteMessageFromChannel(messageId: string, channelId: string): Promise<void> {
const channel = await this.client.channels.fetch(channelId);

if (channel === null) {
throw new Error(`Channel with id ${channelId} not found!`);
}

if (!channel.isText()) {
throw new Error(`Channel with id ${channelId} is not a text channel!`);
}

channel.messages.delete(messageId);
}

async sendDM(userId: string, message: string): Promise<boolean> {
try {
const user = await this.client.users.fetch(userId);
await user.send(message);
return true;
} catch (e) {
console.error(e);
return false;
}
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Tendo em conta que não estamos a fazer nada com o resultado desta função (bool), estamos a retornar isso por alguma razão?

Presumo que possamos inclusive retirar o try/catch.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

e feito

}
}
19 changes: 19 additions & 0 deletions infrastructure/service/discordEmbedService.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,19 @@
import { Client, MessageEmbed, MessageActionRow } from "discord.js";

export default class DiscordEmbedService {
constructor(private client: Client) {}

async sendEmbedToChannel(embed: MessageEmbed, row: MessageActionRow, channelId: string): Promise<void> {
const channel = await this.client.channels.fetch(channelId);

if (channel === null) {
throw new Error(`Channel with id ${channelId} not found!`);
}

if (!channel.isText()) {
throw new Error(`Channel with id ${channelId} is not a text channel!`);
}

channel.send({ embeds: [embed], components: [row] });
}
}
Loading