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

feat(content): implementado ação do botão de deletar conteúdo #353

Merged
merged 1 commit into from
Jun 5, 2022

Conversation

gabrieldev525
Copy link
Contributor

@gabrieldev525 gabrieldev525 commented May 20, 2022

Implementa a ação do botão de deletar conteúdo.

Feedback exibido quando alguém acessa um conteúdo que foi deletado:
image

Além disso, conforme especificado na issue, foi comentado temporariamente o botão de Despublicar

Dependência

Esse pull request tem dependência da issue #348 que implementa o backend da atividade

feat #349

@vercel
Copy link

vercel bot commented May 20, 2022

@gabrieldev525 is attempting to deploy a commit to the TabNews Team on Vercel.

To accomplish this, @gabrieldev525 needs to request access to the Team.

Afterwards, an owner of the Team is required to accept their membership request.

If you're already a member of the respective Vercel Team, make sure that your Personal Vercel Account is connected to your GitHub account.

@filipedeschamps
Copy link
Owner

Geniaaaaal 😍 😍 😍 eu to sinceramente emocionado com a participação de vocês 🤝 💪

Um detalhe importante: um conteúdo com status deleted não é retornado na API. Na visão de quem está consumindo, esse conteúdo não existe mais, então a condicional que foi feita na linha 94 da página do conteúdo não precisará ser feito eu acredito.

Agora, o modo de conteúdo deletado continua valendo caso um usuário delete um conteúdo. Nesse caso, logo após ele usar o botão de Deletar, deveria aparecer a mensagem que o conteúdo foi "deletado com sucesso" e nada mais, não deveria ter um botão, nem redirecionar ele. Pelo menos rodando a situação na minha cabeça 😂 Digo isso, pois se for um conteúdo filho, ficaria estranho ter um botão para acessar mais conteúdos, ou tirar ele da thread. Faz sentido?

@andreghisleni
Copy link
Contributor

Opaaa eu estava vendo a discussão e fiquei com uma duvida, o que acontece com os conteúdos filhos?? recebem o status de deletados ou vão continuar aparecendo??

@gabrieldev525
Copy link
Contributor Author

Opaaa eu estava vendo a discussão e fiquei com uma duvida, o que acontece com os conteúdos filhos?? recebem o status de deletados ou vão continuar aparecendo??

Pois é. É um bom ponto. Se um conteúdo pai é deletado, todos os seus filhos também deveriam ser deletados e não exibidos mais.

Na minha opinião, essa abordagem faz sentido.

@tembra
Copy link
Contributor

tembra commented May 21, 2022

Se um conteúdo pai é deletado, todos os seus filhos também deveriam ser deletados e não exibidos mais.

Acredito que não deveria ser assim. Pois como dito diversas vezes pelo @filipedeschamps, as vezes uma resposta tem mais valor concreto do que o próprio post original. Com certeza a maioria de nós já passamos por essa experiência em comentários de respostas no StackOverflow, por exemplo.

Isso também daria poder para OUTROS excluírem um conteúdo que EU gerei (e que talvez até tenha ganhado TabCoins por isso), desincentivando respostas de qualidade a um contéudo.

IMHO se um conteúdo pai é deletado, o filho deveria continuar existindo e sendo exibido normalmente. E essa abordagem funcionaria da seguinte forma:

  • Se a deleção foi de um conteúdo raiz: Obviamente o conteúdo raiz perderia a listagem, mas todas as respostas à ele permaneceriam válidas, com suas URLs únicas. Possivelmente no futuro poderíamos pensar em dar a opção de converter estas respostas em um conteúdo raiz, modificando o parent_id para null, mas claro, somente nos casos em que o parent_id deste conteúdo estiver deletado.

    • Poderíamos permitir ainda que o owner defina uma slug para ele, já que agora ele iria virar raiz. Entretanto isso aumenta a complexidade, pois não o slug anterior (por uuid) deveria também continuar válido, assim o conteúdo teria dois slugs, um semântico (porque virou raiz) e outro como uuid.
  • Se a deleção foi de uma resposta: Essa resposta deveria ser listada com uma mensagem específica, como por exemplo: Este conteúdo foi excluído pelo autor/administração. E todas as respostas a este conteúdo excluído continuaram existindo e encadeadas abaixo desta mensagem.

Finalmente ainda existem alguns pontos que devem ser levados em consideração se a solução de não excluir as respostas a um conteúdo for adotada:

  • O desenvolvimento da funcionalidade terá que ser feito com bastante cautela pois hoje a API retorna somente respostas com status published.

  • Existirá o problema de que parte (ou todo) o conteúdo excluído poderá estar citado em uma resposta, o que é difícil de auditar e rastrear da forma como está hoje. Apenas para citar o "problema", vamos pensar no caso de exclusões de conteúdo devido a decisões judiciais: como saber que as respostas ao conteúdo ainda não contém o que deveria ser removido segundo ordem judicial?

Que sigamos a discussão muito bem levantada pelo @andreghisleni

@andreghisleni
Copy link
Contributor

Concordo, faz sentido mas poderíamos fazer de outra forma, conforme a clarificação de uma resposta, no caso a sua relevância ela poderia ser levada a virar um conteúdo raiz e no caso da deleção na minha opinião se o conteúdo raiz foi deletado as respostas não deveriam aparecer pelo fato de que pode ficar fora de contexto

@filipedeschamps
Copy link
Owner

@andreghisleni excelente pergunta e @tembra excelente resposta!

Hoje o campo parent_id na API é volátil, ou seja, qualquer conteúdo pode receber um PATCH com parent_id=null e ele será motivo para a raiz do site (mas sem alterar data de publicação) e é obrigatório atender a todos os requisitos de um conteúdo raiz, como por exemplo, ter o title preenchido. Isto está testado aqui:

test('Content with "title" and "parent_id" set to Null', async () => {

E aqui:

test('Content without "title" and "parent_id" set to Null', async () => {

E um teste garantindo erro caso o parent_id receba o seu próprio id:

test('Content with "parent_id" set to itself', async () => {

E o slug também pode ser alterado a qualquer momento, mas que hoje não mantém nenhuma referência com o slug antigo, então as URLs não são permanentes, no sentido de sempre estarem lá, mesmo que isso resulte num 301.


E sobre o ato de deletar propagar o delete nos conteúdos filho, também não concordo que isso aconteça e o @tembra justificou muito bem.

Podemos separar essa implementação em duas fases:

  1. Usar o comportamento atual da API que é: não propagar o delete, mas também não devolver nenhuma referência, o que fará com que na árvore de conteúdo a cadeia seja quebrada e as respostas abaixo do conteúdo deletado sejam escondidas (mas continuarão 100% disponíveis pelo seu slug).
  2. Devolver da API que o conteúdo foi deleted e mockar os campos de conteúdo com "Conteúdo deletado." ou qualquer outra coisa, pois daí o link dentro da árvore continua existindo, assim como no Twitter quando um tweet é deletado, as respostas continuam.

@filipedeschamps
Copy link
Owner

@gabrieldev525 com a parte do backend provavelmente finalizada no PR #431 vou fazer um merge dessa implementação 🤝 vi que ela vai dar conflito e vou tentar resolver por aqui 👍

@filipedeschamps
Copy link
Owner

@gabrieldev525 só pra confirmar, apesar do handleClickDelete() ter sido implementado nesse seu PR, ele não é chamado ao clicar na opção de deletar, correto?

@aprendendofelipe
Copy link
Collaborator

Será que ainda é WIP? Tem mais pontos de atenção:

Dentro de handleClickDelete usa user.username, mas deveria usar o contentObject.username.

  • isLoading está atoa, então, como também não precisa de user, o useUser pode sair do ViewMode

Deve ser só conflito de merge, mas tudo relacionado ao publishedSinceText , incluindo o useEffect deve sair, pois agora é usado o componente PublishedSince.

Isso só em uma olhada rápida...

Aproveitando... Qual é a necessidade do DeletedMode e de verificar status !== 'deleted' também no pages/[username]/[slug]/index.public.js?

A API vai retornar o status deleted se for pedido um conteúdo deletado? Imagino que não! E sobre a página da publicação, não adianta fazer essa verificação, pois ao revalidar já vai dar como página inexistente e, antes disso, irá continuar renderizando normalmente o conteúdo em cache.

@filipedeschamps
Copy link
Owner

@aprendendofelipe sensacional seus comentários, esbarrei nos mesmos detalhes aqui e estou refatorando. Adicionaria mais um que é a verificação do modo deletado dessa forma:

  if (componentMode === 'deleted') {
    return <DeletedMode />;
  }

Isso considerando que não vão vir por enquanto registros deletados pela API. Se viesse, daí a forma como estava caberia melhor 🤝

@filipedeschamps filipedeschamps merged commit ce3a2db into main Jun 5, 2022
@filipedeschamps filipedeschamps deleted the delete-post branch June 5, 2022 18:11
@filipedeschamps
Copy link
Owner

@gabrieldev525 você foi citado nesse post de comemoração: https://www.tabnews.com.br/filipedeschamps/novas-melhorias-habilidade-de-apagar-suas-publicacoes-e-edicao-pela-moderacao 🎉

@gabrieldev525
Copy link
Contributor Author

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.

None yet

5 participants