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

Função "del lista #142

Open
wants to merge 2 commits into
base: master
Choose a base branch
from
Open

Função "del lista #142

wants to merge 2 commits into from

Conversation

saulo-m
Copy link

@saulo-m saulo-m commented Feb 7, 2022

No description provided.

Adiciona botões inline para excluir pacotes
@rougeth
Copy link
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!

@saulo-m
Copy link
Author

saulo-m 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
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?

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
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.

shipments, qtd = list_packages(message.chat.id, False, False)
inline_keyboard = types.InlineKeyboardMarkup()
shipments_buttons = []
if len(shipments) > 0:
Copy link
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

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
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?

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
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.

try:
bot.edit_message_text("Operação cancelada.", call.message.chat.id, call.message.message_id, reply_markup=None)
except Exception:
pass
Copy link
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.

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
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.

if len(shipments) > 0:
msg_split = shipments.split('\n')
print(msg_split)
for elem in range(0, len(msg_split) - 1):
Copy link
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