Editar o perfil de outro usuário

[Rafatcb]

Mudanças realizadas

1. Uso da permissão update:user:others para editar o nome de usuário e descrição de outro usuário.
2. Criação de evento ao editar o perfil, similar ao que ocorre na edição de um conteúdo.
3. Correção de bug na edição de perfil onde, ao realizar um blur e focus na página /perfil, o nome de usuário e e-mail voltavam aos valores iniciais. Isso ocorre devido ao código do onFocus no hook useUser e foi evitado pela condição !usernameRef.current.value no useEffect em EditProfileForm.
4. Correção de bug de descrição vazia ao recarregar a página (F5). Isso foi corrigido com o setDescription no useEffect em EditProfileForm.

Endpoint afetado: PATCH /api/v1/users/[username]

UI:

Screencast.from.2024-01-29.22-16-25.webm

Permissão (feature)

Apesar de eu ter dito (#1466 (comment)) que poderíamos usar a permissão update:content:others, vi que precisaria modificar a validação em can para comparar não só o resource.owner_id, como também resource.id, e isso só seria validado se estivesse atualizando a descrição. Se outros campos estivessem sendo atualizados, precisaria validar update:user:others (mencionado em ForbiddenError.action). Então deixei a validação com a permissão que já estava no código (mas que ninguém possui hoje, e nem era validada em can).

Se acharem que ainda faz sentido ter toda a modificação mencionada acima para granular as permissões em dois tipos diferentes (modificar qualquer valor do outro usuário e modificar apenas a descrição), posso implementar.

Evento

O evento é para ficar assim mesmo, por enquanto? Só coloquei o básico, como é hoje ao atualizar um conteúdo.

Campos editáveis de outro usuário (UI e API)

Tirei a opção de recuperar a senha da UI (já que esse é outro fluxo), a opção de receber notificações por e-mail e o e-mail em si. Faz sentido?

Não vejo o motivo para um moderador precisar editar as notificações de outro usuário, e o e-mail foi removido porque não é possível ver o e-mail de outro usuário. Não sei se deveria adicionar o password nessa lista de validação no backend também.

Resolve #1466

Tipo de mudança

• Correção de bug
• Nova funcionalidade

Checklist:

• As modificações não geram novos logs de erro ou aviso (warning).
• Eu adicionei testes que provam que a correção ou novo recurso funciona conforme esperado.
• Tanto os novos testes quanto os antigos estão passando localmente.

[vercel]

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name	Status	Preview	Comments	Updated (UTC)
tabnews	✅ Ready (Inspect)	Visit Preview	💬 Add feedback	Feb 9, 2024 1:04am

[aprendendofelipe]

Boa @Rafatcb! 💪💪💪

Eu não revisei tudo da implementação, pois algumas coisas podem mudar dependendo da sua opinião sobre os meus comentários no código e aqui abaixo, além do PR #1618.

Permissões

Algo me diz que a feature update:user:others deveria dar todas as permissões de update:user, mas parece que você tomou o melhor caminho ao começar apenas dando as funcionalidades necessárias. Dito isso, qual cenário você pensou sobre a edição do username de outros usuários?

Mantendo essa forma, onde as permissões não serão as mesmas, então acho que não pode ser feita a unificação em authorization.can. Cada verificação precisa ser feita isoladamente, assim como o filterInput que também será específico para cada caso.

Relacionado a isso, tem uma "falha" no can que é verificar resource apenas se ele for enviado. Isso sempre foi assim, mas apenas quando o método foi refatorado no PR #1427 é que esse comportamento ficou explícito. Ele só deveria retornar true se realmente todos os requisitos para cada feature fossem satisfeitos.

E essa mudança no can exige uma melhoria também no filterInput, que também precisará receber o target em alguns casos.

Eu ia sugerir essa mudanças aqui, mas ia complicar mais do que necessário esse PR, então já fiz no #1618.

E com isso o patchHandler pode chamar uma das duas features diferentes, o que exige primeiro verificar as permissões e depois filtrar o input, como sugeri no comentário no código.

Telas

Acho melhor a tela de edição de outros usuários não ser no mesmo endereço de edição do próprio perfil. Mesmo que seja um formulário muito parecido, e até por ser um formulário muito parecido. Isso aumenta o risco de bugs perigosos pela facilidade de confusão dos desenvolvedores no futuro e também existe a UX ruim para os moderadores, que pode causar uma edição de usuários errados ou do próprio usuário sem querer.

Como o caso principal é de edição da descrição, acho que pode ser na própria página /[username], bastando trocar o visualizador pelo editor de markdown.

Se achar que compensa manter no mesmo endereço, então não acha melhor usar um parâmetro ?username= na URL e buscar os dados atualizados via API ao invés de salvar um cache no localStorage? Também seria bom adicionar um campo mostrando qual é o usuário que está sendo editado. Além de separar a função handleSubmit para cada caso.

[aprendendofelipe]

Seria interessante guardar mais informações sobre o que foi alterado, como um diff simplificado.

Obviamente que no caso da senha é suficiente armazenar apenas a informação de que ela foi alterada.

Sobre o diff da descrição, acho que pode ser resolvido em outro PR, pois isso é interessante também ao editar conteúdos. Então por enquanto poderia só salvar a informação de que a descrição foi alterada.

O que acha?

[Rafatcb]

Eu concordo. Como você acha o melhor formato para armazenar esse dado? Adicionando um updatedFields: ['description'], por exemplo?

E também armazenaria os campos alterados caso o usuário esteja atualizando o próprio perfil, certo? Então poderia ter os outros valores ('notifications', 'username' e 'email').

[aprendendofelipe]

updatedFields é legal.

Não pensei em como guardar qual era o email e username antes da modificação, mas acho importante guardar.

[Rafatcb]

Só o "antigo" ou acha importante o "novo" também? Você enxerga esse modo de salvamento como temporário, no sentido de que depois poderemos mudar para seguir o mesmo padrão da descrição, ou acha melhor já ter algo bem definido e que não vá mudar quando tratarmos o diff da descrição?

Estava refletindo aqui se já valeria a pena manter um campo diff.

---

Edit: Andei pesquisando sobre o assunto e não sei se o diff será tratado pela tabela de events. Talvez nessa tabela valha a pena só armazenar os campos que foram modificados (updatedFields)? Ou, por enquanto, podemos armazenar também os valores antigos e novos para o username e email, já que são campos pequenos, para depois passar para uma tabela de histórico? O evento podendo ser algo na linha de:

event = {
  id: "...",
  diff: {
    username: { previous: "rafael", current: "rafael-2" },
    email: { previous: "rafael@example.com", current: "rafael-tabnews@example.com" }
  },
  updatedFields: ["username", "email"]
}

Algumas referências que li, por enquanto:

1. Stack Overflow - How to keep updates (diffs) of some entity in the database
2. DBA Stack Exchange - How to keep updates (diffs) of some entity in the database
3. DBA Stack Exchange - How does Wikipedia store article diffs (or any site which contains lots of text diffs per record)?
4. Stack Overflow - How does one store history of edits effectively?
5. Stack Overflow PT - Existe alguma estratégia para um banco de dados "desfazível"?

[aprendendofelipe]

Só o "antigo" ou acha importante o "novo" também?

Só o "antigo".

Como o dado novo sempre estará na tabela principal, para recuperar todo o histórico de modificações nós só precisamos buscar o estado "antigo" em cada evento (ou o patch/diff para os textos maiores).

Mas você bem lembrou do caso específico do email, em que existe um estado intermediário na alteração, e isso me lembrou que o token que armazena esse estado intermediário já é suficiente para recuperar o histórico de alteração de emails, só ficando faltando alguma forma de armazenar o primeiro email utilizado no cadastro, pois a ativação é diferente do processo de alteração, mas isso também seria algo para se resolver em outro PR. Então é algo bom não precisar salvar os endereços de email nos eventos.

Você enxerga esse modo de salvamento como temporário, no sentido de que depois poderemos mudar para seguir o mesmo padrão da descrição, ou acha melhor já ter algo bem definido e que não vá mudar quando tratarmos o diff da descrição?

De qualquer forma eu acho que seria temporário, pois deveria haver uma normalização desses dados, talvez criar diferentes tabelas para diferentes tipos de metadados dos eventos. Mas no sentido que você perguntou, acho que não seria temporário, já que não vale a pena armazenar diff/patch para esses campos que não comportam muitos caracteres.

Só que eu acho que essa discussão sobre os eventos pode estar fugindo do foco do PR (culpa minha), e que talvez o único dado útil que a gente iria conseguir salvar o histórico já nesse PR seria o username, mas que não poderá ser modificado com update:user:others, então não tem muita relação com o PR. Fora que é uma discussão que pode ser difícil de se recuperar no futuro se for mantida apenas aqui nesse PR.

Então pode ser suficiente lidarmos apenas com o updatedFields nesse PR. Eu só tentaria não incluir em updatedFields campos que não foram realmente modificados, então mudaria a getEventMetadata para que ela verificasse o que mudou entre targetUser e updatedUser, ao invés de assumir que tudo que veio em secureInputValues foi alterado.

[Rafatcb]

Dito isso, qual cenário você pensou sobre a edição do username de outros usuários?

Houve uma situação em Agosto/2023 que uma pessoa criou a conta com um nome de usuário "inadequado", e criou alguns conteúdos (acredito que eram comentários). Não havia motivo para banir pelos comentários, mas o nome inadequado era um sinal vermelho.

Entrei em contato com o Filipe, ele mandou um e-mail para a pessoa pelo e-mail da conta, e depois de uma semana sem resposta, deu o nuke na conta.

Foi nessa situação que pensei para permitir a edição de nome de usuário. Você acha que deveria permitir alterar apenas a descrição ou faz sentido permitir o username também por causa de situações assim?

Se achar que compensa manter no mesmo endereço, então não acha melhor usar um parâmetro ?username= na URL e buscar os dados atualizados via API ao invés de salvar um cache no localStorage?

Depende da sua resposta para minha pergunta anterior. Acho que se permitir alterar o nome de usuário, complicará um pouco de pensar em como poderia fazer uma UI na própria tela do perfil do usuário, mas vou pensar mais sobre isso. Já tenho uma ideia, só não sei o quão viável seria.

Sobre buscar os dados atualizados na API, eu não havia feito isso porque considerei que o moderador visitou o perfil do usuário e clicou em editar, então já estaria atualizado, mas passar o username na URL é mais amigável e aí o fetch se torna necessário. Você acha que se seguirmos o caminho de editar na página do próprio usuário, precisaria realizar um fetch na hora?

[aprendendofelipe]

Você acha que deveria permitir alterar apenas a descrição ou faz sentido permitir o username também por causa de situações assim?

Sobre um username inadequado, acho que ele se enquadraria em um dos dois casos:

• algo que é grave que merece banimento;
• algo estanho, mas não tão grave, e que daria para dar um prazo para o próprio usuário alterar. Até porque seria muito estranho ter que escolher um nome de usuário para alguém.

Se achar que compensa manter no mesmo endereço, então não acha melhor usar um parâmetro ?username= na URL e buscar os dados atualizados via API ao invés de salvar um cache no localStorage?

Depende da sua resposta para minha pergunta anterior. Acho que se permitir alterar o nome de usuário, complicará um pouco de pensar em como poderia fazer uma UI na própria tela do perfil do usuário, mas vou pensar mais sobre isso. Já tenho uma ideia, só não sei o quão viável seria.

Tem alternativas para fazer na mesma página do usuário, caso essa possibilidade vá ser habilitada.

Sobre buscar os dados atualizados na API, eu não havia feito isso porque considerei que o moderador visitou o perfil do usuário e clicou em editar

Usar o localStorage pode dar problemas caso esteja lidando com mais de um usuário em abas diferentes.

passar o username na URL é mais amigável e aí o fetch se torna necessário.

Isso é uma sugestão se quiser muito usar a mesma URL, mas se não der pra fazer na própria página do usuário, acho que seria melhor criar uma nova página específica para isso.

Você acha que se seguirmos o caminho de editar na página do próprio usuário, precisaria realizar um fetch na hora?

Na própria página não precisa fazer o fetch. Mesmo existindo um pequeno risco de estar vendo um dado já modificado, pois se isso acontecer, provavelmente também estará desatualizado na API, que também tem cache.

[Rafatcb]

Atualizei conforme as revisões.

Testes

Aproveitei para remover o TODO: test with expired session, criando o teste baseado no With expired session do /api/v1/users/get.test.js (acabou mudando apenas o fetch):

tabnews.com.br/tests/integration/api/v1/user/get.test.js

Lines 174 to 212 in 76dfea9

	test('With expired session', async () => {
	jest.useFakeTimers({
	now: new Date(Date.now() - 1000 * 60 * 60 * 24 * 30), // 30 days ago
	advanceTimers: true,
	});
	const defaultUser = await orchestrator.createUser();
	await orchestrator.activateUser(defaultUser);
	const defaultUserSession = await orchestrator.createSession(defaultUser);
	jest.useRealTimers();
	const response = await fetch(`${orchestrator.webserverUrl}/api/v1/user`, {
	method: 'GET',
	headers: {
	cookie: `session_id=${defaultUserSession.token}`,
	},
	});
	const responseBody = await response.json();
	expect(response.status).toEqual(401);
	expect(responseBody.status_code).toEqual(401);
	expect(responseBody.name).toEqual('UnauthorizedError');
	expect(responseBody.message).toEqual('Usuário não possui sessão ativa.');
	expect(responseBody.action).toEqual('Verifique se este usuário está logado.');
	expect(uuidVersion(responseBody.error_id)).toEqual(4);
	expect(uuidVersion(responseBody.request_id)).toEqual(4);
	const parsedCookiesFromGet = authentication.parseSetCookies(response);
	expect(parsedCookiesFromGet.session_id.name).toEqual('session_id');
	expect(parsedCookiesFromGet.session_id.value).toEqual('invalid');
	expect(parsedCookiesFromGet.session_id.maxAge).toEqual(-1);
	expect(parsedCookiesFromGet.session_id.path).toEqual('/');
	expect(parsedCookiesFromGet.session_id.httpOnly).toEqual(true);
	const sessionObject = await orchestrator.findSessionByToken(defaultUserSession.token);
	expect(sessionObject).toBeUndefined();
	});

Evento

Conforme pensei sobre o assunto (compartilhei algumas coisas aqui), optei por salvar os valores em data ao invés de diff para não confundir, já que o que estou salvando não é um formato de diff.

Além disso, do jeito que está, salvará o novo e-mail em data mesmo que ele não tenha sido alterado no banco de dados (porque ainda precisa da confirmação). Faz sentido?

UI

Coloquei um botão para editar a descrição, já que é uma única ação, e aproveitei para também permitir criar a descrição na própria tela.

Optei por não exibir o botão Criar descrição para o moderador no perfil de outro usuário porque não faz sentido criar uma descrição, a menos que tenha apagado a descrição sem querer (o que considero pouco provável). Se necessário, posso adicionar uma confirmação no cenário em que o moderador está atualizando a descrição de outro usuário para vazio.

Tentei seguir o padrão que encontrei em outras telas, mas não encontrei a forma ideal de usar o Flash para feedback nessa tela. Se deixar na mesma posição que está o do nuke, ao aparecer ele empurrará o campo de descrição para baixo, e dependendo da rolagem, não é perceptível que ele apareceu. Se deixar embaixo do campo de descrição, em cima do botão de salvar, no caso de sucesso e uma descrição muito grande, ele não aparecerá na área visível da tela (o usuário precisará realizar a rolagem).

Eu deixei conforme o segundo caso pois acho o melhor, dentre os dois citados.

Screencast.from.2024-02-03.19-44-44.webm

[aprendendofelipe]

@Rafatcb, para testarmos em homologação, já adicionei a update:user:others aos nossos usuários e ao do @filipedeschamps.

Evento

Além disso, do jeito que está, salvará o novo e-mail em data mesmo que ele não tenha sido alterado no banco de dados (porque ainda precisa da confirmação). Faz sentido?

Estou achando melhor nesse PR criar apenas o updatedFields e debater em uma issue como podemos fazer algo mais elaborado para salvas os dados editados nos eventos.

Acho que é um debate importante e que pode ser difícil de ser revisto no futuro se ficar apenas nesse PR, já que ele tem relação apenas indireta com o assunto.

UI

Optei por não exibir o botão Criar descrição para o moderador no perfil de outro usuário porque não faz sentido criar uma descrição, a menos que tenha apagado a descrição sem querer (o que considero pouco provável). Se necessário, posso adicionar uma confirmação no cenário em que o moderador está atualizando a descrição de outro usuário para vazio.

Por precaução, acho que compensa manter o botão Criar descrição para o moderador, mas também é importante a confirmação ao salvar, mesmo que não seja uma descrição vazia. Acho que é importante confirmar que está editando a descrição do usuário "fulanoDeTal".

Tentei seguir o padrão que encontrei em outras telas, mas não encontrei a forma ideal de usar o Flash para feedback nessa tela. Se deixar na mesma posição que está o do nuke, ao aparecer ele empurrará o campo de descrição para baixo, e dependendo da rolagem, não é perceptível que ele apareceu. Se deixar embaixo do campo de descrição, em cima do botão de salvar, no caso de sucesso e uma descrição muito grande, ele não aparecerá na área visível da tela (o usuário precisará realizar a rolagem).

Eu deixei conforme o segundo caso pois acho o melhor, dentre os dois citados.

Acho que pode ficar assim por enquanto, mas acredito que toasts seriam uma boa adição em outro PR.

[aprendendofelipe]

Só o "antigo" ou acha importante o "novo" também?

Só o "antigo".

Como o dado novo sempre estará na tabela principal, para recuperar todo o histórico de modificações nós só precisamos buscar o estado "antigo" em cada evento (ou o patch/diff para os textos maiores).

Mas você bem lembrou do caso específico do email, em que existe um estado intermediário na alteração, e isso me lembrou que o token que armazena esse estado intermediário já é suficiente para recuperar o histórico de alteração de emails, só ficando faltando alguma forma de armazenar o primeiro email utilizado no cadastro, pois a ativação é diferente do processo de alteração, mas isso também seria algo para se resolver em outro PR. Então é algo bom não precisar salvar os endereços de email nos eventos.

Você enxerga esse modo de salvamento como temporário, no sentido de que depois poderemos mudar para seguir o mesmo padrão da descrição, ou acha melhor já ter algo bem definido e que não vá mudar quando tratarmos o diff da descrição?

De qualquer forma eu acho que seria temporário, pois deveria haver uma normalização desses dados, talvez criar diferentes tabelas para diferentes tipos de metadados dos eventos. Mas no sentido que você perguntou, acho que não seria temporário, já que não vale a pena armazenar diff/patch para esses campos que não comportam muitos caracteres.

Só que eu acho que essa discussão sobre os eventos pode estar fugindo do foco do PR (culpa minha), e que talvez o único dado útil que a gente iria conseguir salvar o histórico já nesse PR seria o username, mas que não poderá ser modificado com update:user:others, então não tem muita relação com o PR. Fora que é uma discussão que pode ser difícil de se recuperar no futuro se for mantida apenas aqui nesse PR.

Então pode ser suficiente lidarmos apenas com o updatedFields nesse PR. Eu só tentaria não incluir em updatedFields campos que não foram realmente modificados, então mudaria a getEventMetadata para que ela verificasse o que mudou entre targetUser e updatedUser, ao invés de assumir que tudo que veio em secureInputValues foi alterado.

[Rafatcb]

Realizei as modificações comentadas.

Um problema que percebi que existe é que, se você está num perfil de outro usuário e está com um Flash visível, ao acessar o seu perfil pelo menu, o Flash continua visível. Resolvi isso com um useEffect.

---

Edit:

Estou achando melhor nesse PR criar apenas o updatedFields e debater em uma issue como podemos fazer algo mais elaborado para salvas os dados editados nos eventos.

Você acha melhor criar um issue para isso ou discutir em #461? Para mim, os assuntos parecem bem misturados, sendo ambos sobre um controle de histórico de edições (apenas mudando o campo e tabela).

[aprendendofelipe]

Realizei as modificações comentadas.

Bora pra produção?

Você acha melhor criar um issue para isso ou discutir em #461?

Por enquanto acho que pode ficar na #461, mas se surgir necessidade de separar, a gente cria outro. 🤝

[aprendendofelipe]

Um problema que percebi que existe é que, se você está num perfil de outro usuário e está com um Flash visível, ao acessar o seu perfil pelo menu, o Flash continua visível. Resolvi isso com um useEffect.

Também permanece em modo de edição e com a descrição do usuário anterior, então fiz novas sugestões para correção desse comportamento.

Fiz novos comentários

[aprendendofelipe]

Código e permissões em produção! 🚀🚀🚀