Improve notification management #561

Fusionado

Plume_migration_agent fusionados 4 commits de notifications en master

trinity-1686a comentado

Propietario

fix #472
fix #505

fix #472 fix #505

elegaanz (Migrado desde github.com) revisado

plume-models/src/comments.rs

elegaanz (Migrado desde github.com) comentado

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 desde github.com) revisado

plume-models/src/comments.rs

elegaanz (Migrado desde github.com) comentado

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 revisado

plume-models/src/comments.rs

trinity-1686a comentado

Autor

Propietario

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] comentado (Migrado desde 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 desde github.com) revisado

plume-models/src/comments.rs

elegaanz (Migrado desde github.com) comentado

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

elegaanz (Migrado desde github.com) revisado

src/routes/comments.rs

elegaanz (Migrado desde github.com) comentado

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

elegaanz (Migrado desde github.com) aprobado estos cambios

elegaanz (Migrado desde github.com) dejó un comentario

Seems to work! Thank you! 😊

Seems to work! Thank you! :blush:

Revisores

El Pull Request se ha fusionado como c9070930d2.

También puede ver instrucciones de línea de comandos.

Paso 1:

Desde el repositorio de su proyecto, revisa una nueva rama y prueba los cambios.

git checkout -b notifications master

git pull origin notifications

Paso 2:

Combine los cambios y actualice en Forgejo.

git checkout master

git merge --no-ff notifications

git push origin master

Inicie sesión para unirse a esta conversación.

No hay revisores

Sin Milestone

No asignados

2 participantes

Notificaciones

Fecha de vencimiento

La fecha de vencimiento es inválida o está fuera de rango. Por favor utilice el formato 'aaaa-mm-dd'.

Sin fecha de vencimiento.

Dependencias

No se han establecido dependencias.

Cargando…

Aún no existe contenido.

Etiquetas Hitos

Improve notification management #561

Codecov Report

Revisores

Paso 1:

Paso 2: