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

content(api): criar novo status deleted #348

Closed
Tracked by #347
filipedeschamps opened this issue May 19, 2022 · 18 comments
Closed
Tracked by #347

content(api): criar novo status deleted #348

filipedeschamps opened this issue May 19, 2022 · 18 comments
Labels
back Envolve modificações no backend

Comments

@filipedeschamps
Copy link
Owner

filipedeschamps commented May 19, 2022

Contexto

Para fazer um "soft delete" de um content, vamos criar um novo status deleted e um novo campo deleted_at.

Execução

Hoje a tabela contents possui uma constraint entre owner_id e slug para que um usuário não possa criar posts com um mesmo identificador.

await pgm.addConstraint('contents', 'contents_uniqueness_fkey', 'UNIQUE ("owner_id", "slug")');

O problema de manter essa constraint, é que acaba não se tornando mais possível deletar contents que acabem tendo o mesmo slug. Por exemplo:

  1. Eu crio um post /filipedeschamps/teste
  2. Em seguida me arrependo e deleto ele (soft delete, pois continua existindo na base).
  3. Ao tentar criar um outro post /filipedeschamps/teste, o backend vai reclamar, pois já existe essa combinação, mas uma delas (a primeira) está com o status deleted.

E se adicionar o status nessa constraint, não adianta também, pois daí eu não vou poder soft delete dois conteúdos com o mesmo indicador.

Então em conversas passadas com o @rodrigoKulb , ele fez uma sugestão de criar uma propriedade deleted_at e adicionar ela na constraint. Achei genial.


Fora isso, uma vez um content ganhando o status de deleted, não é mais possível voltar atrás.

@filipedeschamps filipedeschamps changed the title content(api): criar novo status deleted content(api): criar novo status deleted May 19, 2022
@filipedeschamps filipedeschamps added the back Envolve modificações no backend label May 19, 2022
@filipedeschamps filipedeschamps added this to the Milestone 4: TabCoins milestone May 19, 2022
@tembra
Copy link
Contributor

tembra commented May 19, 2022

Você acredita ter necessidade de criar um novo status deleted?

Assim ficaria organizado e teríamos acesso direto para consulta, mas o deleted_at resolve todos os problemas.

Isso nos ajudaria a manter o último status do conteúdo antes dele ter sido deletado e poderíamos contar com uma das maravilhas de se usar soft delete: análise de dados.

Com o tempo poderíamos saber se a maioria dos conteúdos são deletadas enquanto ainda estão com o status draft, pois as pessoas estão desistindo de postar algo. Ou então, se as pessoas se arrependeram de postar e estão deletadas depois que o status está como published.

Uma pergunta: Hoje existe a possibilidade de voltar um conteúdo pra draft após ele estar como published ? Se não existe, será adicionado no futuro?

Pergunto isso pois se não tiver e não formos adicionar esta possibilidade no futuro, o campo published_at já atenderia até o uso do status e na verdade nem precisaria ser utilizado, a não ser que depois surgissem status como pending-approval para aguardar a aprovação de alguém.

EDIT: Verifiquei na issue #349 que hoje o status draft não está acessível.

@filipedeschamps
Copy link
Owner Author

Ótimo ponto @tembra mas na minha visão fica menos semântico, por exemplo, imagina montar uma query que agrupa todos os posts pelos seus status ou o PATCH que será feito contra a API para mudar de um status para o outro, e também como consultar isso pela API.

Talvez se um dia quisermos fazer essa leitura dos dados, a modelagem do banco deveria ser INSERT ONLY (o que eu acharia ótimo), ou salvar essas ações em outro lugar para consulta. Dado a isso, voto em manter uma modelagem mais comum e previsível, mesmo que nesse caso seja um dado redundante.

EDIT: Verifiquei na issue #349 que hoje o status draft não está acessível.

Na verdade o status draft está disponível e funcional, porém hoje todos os endpoints forçam buscar os dados como published. Então não há pela API hoje ninguém que listem os posts em draft.

NahtanN added a commit that referenced this issue May 26, 2022
Alterações que habilitam a função de excluir postagem, seja ela um comentário ou o post "raiz"

BREAKING CHANGE: Alterações no banco de dados, querys e alguns schemas de validação; além de algumas
mudanças no componente "Content"

feat #348 e feat #349
@filipedeschamps
Copy link
Owner Author

filipedeschamps commented Jun 1, 2022

Sabe aquele tipo de problema que você acha que vai matar rapidão, mas quem te mata é o problema? 😂 então, quem está me matando é a CONSTRAINT UNIQUE envolvendo owner_id, slug e deleted_at, sendo que o deleted_at pode ser null e isso faz o Postgres aceitar múltiplas linhas iguais.

Por exemplo, considerando o que está em Produção, o segundo insert daria problema porque ele possui o mesmo owner_id e slug:

INSERT INTO contents (owner_id, slug, title, body, status) VALUES ('88717cd5-d255-4ada-a235-e8c38bffe173', 'slug', 'title', 'body', 'published');
INSERT INTO contents (owner_id, slug, title, body, status) VALUES ('88717cd5-d255-4ada-a235-e8c38bffe173', 'slug', 'title', 'body', 'published');

Resultado:

ERROR:  duplicate key value violates unique constraint "contents_uniqueness_fkey"
DETAIL:  Key (owner_id, slug)=(88717cd5-d255-4ada-a235-e8c38bffe173, slug) already exists.

Isso porque temos isso de constraint:

image

Agora, rodando uma nova migration para considerar também o deleted_at:

exports.up = async (pgm) => {
  await pgm.addColumns('contents', {
    deleted_at: {
      type: 'timestamp with time zone',
      notNull: false,
    },
  });

  await pgm.dropConstraint('contents', 'contents_uniqueness_fkey');
  await pgm.addConstraint('contents', 'contents_uniqueness_fkey', 'UNIQUE (owner_id, slug, deleted_at)');
};

Show, atualizou a constraint:

image

Só que agora eu posso fazer múltiplos inserts, pois por padrão uma coluna NULL não faz dar clash nos valores:

image

Que !delicinha né?

E procurando na internet, esse é o comportamento mesmo e estão sugerindo uma alternativa com índices, mas puts, que bizarro demais:

E agora? Alguém conhece alguma alternativa mais elegante?

@filipedeschamps
Copy link
Owner Author

Bom, não consigo encontrar outra solução a não ser deletar a constraint e substituir por um índice. Fiz da seguinte forma no arquivo de migração:

  // pgm.createIndex(tablename, columns, options)

  await pgm.createIndex('contents', ['owner_id', 'slug', '(deleted_at IS NULL)'], {
    name: 'contents_owner_id_slug_deleted_at_unique_index',
    unique: true,
    where: 'deleted_at IS NULL',
  });

O que gera:

CREATE UNIQUE INDEX "contents_owner_id_slug_deleted_at_unique_index" ON "contents" ("owner_id", "slug", (deleted_at IS NULL)) WHERE deleted_at IS NULL;

E o comportamento de ir fazendo INSERTs em relação ao índice e o deleted_at:

INSERT INTO contents (owner_id, slug, title, body, status, deleted_at) VALUES ('88717cd5-d255-4ada-a235-e8c38bffe173', 'slug', 'title', 'body', 'published', NULL);

-- OK

INSERT INTO contents (owner_id, slug, title, body, status, deleted_at) VALUES ('88717cd5-d255-4ada-a235-e8c38bffe173', 'slug', 'title', 'body', 'published', NULL);

-- NOT OK
-- ERROR:  duplicate key value violates unique constraint "contents_owner_id_slug_deleted_at_unique_index"
-- DETAIL:  Key (owner_id, slug, (deleted_at IS NULL))=(88717cd5-d255-4ada-a235-e8c38bffe173, slug, t) already exists.

INSERT INTO contents (owner_id, slug, title, body, status, deleted_at) VALUES ('88717cd5-d255-4ada-a235-e8c38bffe173', 'slug-sera-deletado', 'title', 'body', 'published', NULL);

-- OK

INSERT INTO contents (owner_id, slug, title, body, status, deleted_at) VALUES ('88717cd5-d255-4ada-a235-e8c38bffe173', 'slug-sera-deletado', 'title', 'body', 'deleted', NULL);

-- NOT OK: ta certo, pois só mudei o `status` para `deleted` sem ter adicionado um `deleted_at`
-- ERROR:  duplicate key value violates unique constraint "contents_owner_id_slug_deleted_at_unique_index"
-- DETAIL:  Key (owner_id, slug, (deleted_at IS NULL))=(88717cd5-d255-4ada-a235-e8c38bffe173, slug-sera-deletado, t) already exists.

INSERT INTO contents (owner_id, slug, title, body, status, deleted_at) VALUES ('88717cd5-d255-4ada-a235-e8c38bffe173', 'slug-sera-deletado', 'title', 'body', 'deleted', NOW());

-- OK: não esbarra mais por conta do `deleted_at`

INSERT INTO contents (owner_id, slug, title, body, status, deleted_at) VALUES ('88717cd5-d255-4ada-a235-e8c38bffe173', 'slug-sera-deletado', 'title', 'body', 'deleted', NOW());

-- OK: não esbarra mais por conta do `deleted_at`

image

O mesmo comportamento se garante se eu tentar sair fazendo UPDATE. Se ele não esbarrar com o índice único, tudo ok... se esbarrar, ele devolve um erro.

@aprendendofelipe
Copy link
Collaborator

Uma alternativa nada semântica seria usar uma data específica do passado no lugar de null e o artigo ser considerado deletado se deleted_at for diferente dessa data, por exemplo, 01/01/2000 00:... Quem aí já trabalhava na área na época do bug do milênio 😅

@filipedeschamps
Copy link
Owner Author

E se eu te falar que sempre preencher uma data foi a primeira alternativa que pensei quando comecei a entender que não daria para resolver usando constraint e null 😂 🤝👍

@aprendendofelipe
Copy link
Collaborator

E se eu te falar que sempre preencher uma data foi a primeira alternativa que pensei quando comecei a entender que não daria para resolver usando constraint e null 😂 🤝👍

Elegante? Nada! Funciona? Perfeitamente... hahaha

@rodrigoKulb
Copy link
Contributor

@filipedeschamps aqui na empresa utilizamos 2 informações para registros apagados
01 - Quando = deleted_at (data)
02 - Quem = idUser (int)
Agora com a funcionalidade liberada para outros membros apagarem artigos de outro usuários, acredito que seria muito importante essa informação até para segurança do portal.

E o campo int = 0 não daria o mesmo problema certo?

@aprendendofelipe
Copy link
Collaborator

aprendendofelipe commented Jun 2, 2022

@rodrigoKulb o problema seria que o mesmo usuário não conseguiria apagar duas vezes a postagem com o mesmo slug e owner_id

Edit para melhor clareza: Esse problema é se a sugestão foi usar o idUser junto de owner_id e slug como unique.

@filipedeschamps
Copy link
Owner Author

Interessantíssimo @rodrigoKulb 👍 no nosso caso o id do usuário é um UUID, então não aceitaria 0, mas não importa porque o índice que estamos usando para controlar o unique não leva em consideração esse campo. Na migration que está no novo PR ele só considera o owner_id, slug e deleted_at.

E acho ótimo anotar esse tipo de informação, mas sugiro não fazer nessa versão (até porque vamos ter tudo isso registrado em logs caso a gente precise investigar). E daí numa próxima interação fazer a anotação de quem editou um conteúdo (não somente deletou), até porque esse "soft delete" é uma "edição de conteúdo" (que altera o status para deleted). Daí devemos anotar todas as atualizações de um conteúdo, inclusive para conseguir mostrar um histórico como o GitHub faz.

@rodrigoKulb
Copy link
Contributor

Edit para melhor clareza: Esse problema é se a sugestão foi usar o idUser junto de owner_id e slug como unique.

@aprendendofelipe verdade! 😅️

id do usuário é um UUID

@filipedeschamps verdade 😅️ show! Seguimos quebrando a cabeça aqui!

@rodrigoKulb
Copy link
Contributor

Só para jogar uma ideia aqui, se utilizarmos o campo:
deleted_at (int) com o Unix Timestamp

Assim poderia utilizar o ZERO sem problemas = Jan 01 1970 00:00:00 GMT+0000

@filipedeschamps
Copy link
Owner Author

@rodrigoKulb gosto de você por estar com a mesma raiva que eu em ter que usar um índice para controlar o unique 😂

Mas apesar dessa ideia tecnicamente ser válida, semanticamente fica estranho o deleted_at ser num formato e os outros campos de data como created_at ou published_at ser em outro formato.

@tembra
Copy link
Contributor

tembra commented Jun 3, 2022

Comentando aqui somente pra registrar a normalidade de se usar unique index em soft deletes: Trabalho há +10 anos com grandes ERPs e todos os que possuem soft delete utilizam desta estratégia. Seja com banco de dados SQL Server, Oracle ou Postgres.

Sempre é criado um índice único contendo o flag do registro de deleção.

Para entendermos melhor o porquê do Postgres permitir múltiplos nulos devemos lembrar da tradução da palavra constraint que significa restrição.

Tendo isto em mente entendemos que a maioria das implementações dos SGBDs utiliza constraints como forma de referenciar alguma coisa, como por exemplo chaves estrangeiras ou valores válidos. Por outro lado sabemos que os índices são utilizados explicitamente para facilitar/agilizar a busca por dados.

Dessa forma agregando agora o conceito de unique entendemos que um valor NULL não impede nenhuma restrição, por isso alguns SGBDs como o Postgres o desconsideram quando falamos de constraints (lembrem: restrições). Não há o que ser restringido se o valor for NULL exceto se a própria coluna não o suportar é claro (mas isso não tem mais a ver com constraints). Por outro lado um índice único deve levar todas as possibilidades (incluindo o NULL) em consideração, pois se trata de como os dados serão organizados.

Ainda sim em alguns SGBDs (como no Postgres) é preciso informar explicitamente que determinada coluna poderá conter o valor NULL e, quando tiver, deve ser indexada de forma única, como feito pelo @filipedeschamps no comentário acima, replicado logo abaixo:

CREATE UNIQUE INDEX "contents_owner_id_slug_deleted_at_unique_index"
ON "contents" ("owner_id", "slug", (deleted_at IS NULL))
WHERE deleted_at IS NULL;

@filipedeschamps
Copy link
Owner Author

Explicação sensacional @tembra e isso me tranquiliza bastante sobre a escolha então de criarmos o índice para controlar isso 🤝

@filipedeschamps
Copy link
Owner Author

Fechado pelo PR #431 🤝

@filipedeschamps
Copy link
Owner Author

Turma, olha o que está por vir na versão 15 do Postgres 😍 https://blog.rustprooflabs.com/2022/07/postgres-15-unique-improvement-with-null

@aprendendofelipe
Copy link
Collaborator

Turma, olha o que está por vir na versão 15 do Postgres

UNIQUE NULLS NOT DISTINCT 😍😍😍

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
back Envolve modificações no backend
Projects
None yet
Development

No branches or pull requests

4 participants