Skip to content

Função "del lista#142

Open
inno3759 wants to merge 2 commits intoGabrielRF:masterfrom
inno3759:master
Open

Função "del lista#142
inno3759 wants to merge 2 commits intoGabrielRF:masterfrom
inno3759:master

Conversation

@inno3759
Copy link
Copy Markdown

@inno3759 inno3759 commented Feb 7, 2022

No description provided.

Adiciona botões inline para excluir pacotes
@rougeth
Copy link
Copy Markdown
Collaborator

rougeth commented Feb 7, 2022

Obrigado pelo PR @saulo-m! Você poderia adicionar uma descrição sobre as mudanças? Vi que você também adicionou alguns comandos, poderia também colocar uns exemplos de como será usado na descrição? Valeu!

@inno3759
Copy link
Copy Markdown
Author

inno3759 commented Feb 7, 2022

Oi @rougeth. Não houve adição de comandos, o que fiz foi o seguinte:

Se o usuário não especificar um pacote para ser deletado na mensagem, ou seja, digitou apenas "/del" em vez de "/del SE123456789BR", o script vai puxar os pacotes desse usuário e retornar uma mensagem com n botões para o usuário escolher qual pacote deseja excluir.

Quando ele apertar algum botão, o bot recebe o callback, que é usado nas outras duas funções que adicionei. Não tem comando, uma verifica o pacote que ele deseja apagar e o apaga, e a outra função cancela a operação, se ele clicou em cancelar.

Eu enviei acidentalmente o pull, ai acabou ficando sem a descrição, desculpe.
Pelo que conversei com o @GabrielRF, alguns usuários tem uma quantidade enorme de pacotes, então se essa função chegar a ser implementada, deve haver um limitador pelo número de pacotes, ou um botão de "Próximo..." na lista.

Copy link
Copy Markdown
Collaborator

@rougeth rougeth left a comment

Choose a reason for hiding this comment

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

Entendi agora Saulo, obrigado pela explicação. Eu deixei alguns comentários, mas o principal talvez seja separar em dois comandos, um só para o /del e outro para /del <código>. O código abaixo é só uma sugestão, não tenho testei e pode ser que nem funcione, mas me diz o que você acha?

DELETE_COMMAND_OPTIONS = ["/del", "/remover", "/apagar"]

def is_delete_command(message):
    return message.text.lower() in DELETE_COMMAND_OPTIONS

def is_delete_with_code_command(message):
    command, *code = message.text.lower().split()

    if command not in DELETE_COMMAND_OPTIONS:
        return False

    if len(code) != 1 or not re.search(r'\w{2}\d{9}\w{2}', code[0]):
        return False

    return True


@bot.message_handler(func=is_delete_command)
    ...

@bot.message_handler(func=is_delete_with_code_command)
    ...

Dessa forma, o fluxo fica um pouco mais claro e ainda facilita até criar testes unitários (o que também precisamos melhor no projeto). O que acha?

Comment thread rastreiobot.py
msg_split = shipments.split('\n')
print(msg_split)
for elem in range(0, len(msg_split) - 1):
package = (msg_split[elem][1:14])
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Poderia deixar um comentário explicando o motivo do [1:14]? Pensando no futuro, vai ficar mais fácil o que essa parte está fazendo.

Comment thread rastreiobot.py
shipments, qtd = list_packages(message.chat.id, False, False)
inline_keyboard = types.InlineKeyboardMarkup()
shipments_buttons = []
if len(shipments) > 0:
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Podemos diminuir um nível de indentação, se fizermos algo assim:

if not shipments:
    bot.send_message(message.chat.id, "Não há pacotes cadastrados.",
        disable_web_page_preview=False, reply_markup=inline_keyboard)
    return

# a partir daqui, é só seguir com a lógica

Comment thread rastreiobot.py
else:
bot.send_message(message.chat.id, "Não há pacotes cadastrados.",
disable_web_page_preview=False, reply_markup=inline_keyboard)
except Exception:
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

A gente tem migrado aos poucos esses except Exception. É muito genérico e pode esconder erros reais que precisam ser tratados. Tem como você colocar uma exceção mais específica aqui?

Comment thread rastreiobot.py
db.remove_user_from_package(code, call.message.chat.id)
bot.edit_message_text("Pacote removido.", call.message.chat.id, call.message.message_id, reply_markup=None)
except Exception:
pass
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Mesmo comentário sobre except Exception que fiz ali em cima.

Comment thread rastreiobot.py
try:
bot.edit_message_text("Operação cancelada.", call.message.chat.id, call.message.message_id, reply_markup=None)
except Exception:
pass
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Mesmo comentário sobre except Exception que fiz ali em cima.

Comment thread rastreiobot.py
try:
code = call.data
db.remove_user_from_package(code, call.message.chat.id)
bot.edit_message_text("Pacote removido.", call.message.chat.id, call.message.message_id, reply_markup=None)
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Se der algum erro na hora de remover o pacote no banco, o bot não vai remover teclado e nem avisar que deu erro. Talvez seja uma boa colocar algo no bloco except para isso.

Comment thread rastreiobot.py
if len(shipments) > 0:
msg_split = shipments.split('\n')
print(msg_split)
for elem in range(0, len(msg_split) - 1):
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Aqui, acho que você pode trocar por:

for elem in msg_split:
    package = elem[1:14]

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants