Improve notification management #561

Merge aplicado

Plume_migration_agent aplicou merge dos 4 commits de notifications em master

trinity-1686a comentou

Proprietário

fix #472
fix #505

fix #472 fix #505

elegaanz (Migrado de github.com) revisou

plume-models/src/comments.rs

elegaanz (Migrado de github.com) comentou

Shouln't this be .any(...)? Because I'm still getting duplicated notifications.

Shouln't this be `.any(...)`? Because I'm still getting duplicated notifications.

elegaanz (Migrado de github.com) revisou

plume-models/src/comments.rs

elegaanz (Migrado de github.com) comentou

Also, AsActor::is_local is technically broken, since it only does instance_id == 1 which is not always correct (even if it is most of the time (like, unless you ran plm instance new twice, it will probably be correct)).

Maybe we should have a global storing the local instance info somewhere BTW (loaded only once when the app starts)? It would probably avoid a lot of queries to the database, and would allow to fix this issue without changing AsActor definition (but probably in another PR if we ever do it).

Also, `AsActor::is_local` is technically broken, since it only does `instance_id == 1` which is not always correct (even if it is most of the time (like, unless you ran `plm instance new` twice, it will probably be correct)). Maybe we should have a global storing the local instance info somewhere BTW (loaded only once when the app starts)? It would probably avoid a lot of queries to the database, and would allow to fix this issue without changing `AsActor` definition (but probably in another PR if we ever do it).

trinity-1686a revisou

plume-models/src/comments.rs

trinity-1686a comentou

Autor

Proprietário

if all mentioned users are different from this author, send notification. I think it's correct, and when I tested it, it seemed to work.
Edit : My bad, forgot to add some file

if *all* mentioned users are different from this author, send notification. I think it's correct, and when I tested it, it seemed to work. Edit : My bad, forgot to add some file

codecov[bot] comentou (Migrado de github.com)

Codecov Report

Merging #561 into master will decrease coverage by <.01%.
The diff coverage is 54.34%.

@@            Coverage Diff             @@
##           master     #561      +/-   ##
==========================================
- Coverage   35.05%   35.05%   -0.01%     
==========================================
  Files          67       67              
  Lines        7834     7843       +9     
  Branches     1877     1885       +8     
==========================================
+ Hits         2746     2749       +3     
- Misses       4331     4332       +1     
- Partials      757      762       +5

# [Codecov](https://codecov.io/gh/Plume-org/Plume/pull/561?src=pr&el=h1) Report > Merging [#561](https://codecov.io/gh/Plume-org/Plume/pull/561?src=pr&el=desc) into [master](https://codecov.io/gh/Plume-org/Plume/commit/918bda14ececc7f72b5741896c2c7ca8fba114f9?src=pr&el=desc) will **decrease** coverage by `<.01%`. > The diff coverage is `54.34%`. ```diff @@ Coverage Diff @@ ## master #561 +/- ## ========================================== - Coverage 35.05% 35.05% -0.01% ========================================== Files 67 67 Lines 7834 7843 +9 Branches 1877 1885 +8 ========================================== + Hits 2746 2749 +3 - Misses 4331 4332 +1 - Partials 757 762 +5 ```

elegaanz (Migrado de github.com) revisou

plume-models/src/comments.rs

elegaanz (Migrado de github.com) comentou

Oh, I found what is wrong I think: the mention was inserted after the comment in the database.

elegaanz (Migrado de github.com) revisou

src/routes/comments.rs

elegaanz (Migrado de github.com) comentou

OK, I'm too late, you already found it.

elegaanz (Migrado de github.com) aprovou estas alterações

elegaanz (Migrado de github.com) deixou um comentário

Seems to work! Thank you! 😊

Seems to work! Thank you! :blush:

Revisores

O pull request teve merge aplicado como c9070930d2.

Você também pode ver as instruções para a linha de comandos.

Passo 1:

No repositório do seu projeto, crie um novo branch e teste as alterações.

git checkout -b notifications master

git pull origin notifications

Passo 2:

Faça merge das alterações e atualize no Forgejo.

git checkout master

git merge --no-ff notifications

git push origin master

Acesse para participar desta conversação.

Sem revisor

Sem marco

Sem responsável

2 participante(s)

Notificações

Data limite

A data limite é inválida ou está fora do intervalo. Por favor, use o formato 'dd/mm/aaaa'.

Data limite não informada.

Dependências

Nenhuma dependência definida.

Carregando…

Ainda não há conteúdo.

Etiquetas Marcos

Improve notification management #561

Codecov Report

Revisores

Passo 1:

Passo 2: