Improve notification management #561

Samengevoegd
Plume_migration_agent heeft 4 commits samengevoegd van notifications naar master 5 jaren geleden
Eigenaar

fix #472
fix #505

fix #472 fix #505
elegaanz (Gemigreerd van github.com) beoordeeld 5 jaren geleden
elegaanz (Gemigreerd van github.com) reageerde 5 jaren geleden

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 (Gemigreerd van github.com) beoordeeld 5 jaren geleden
elegaanz (Gemigreerd van github.com) reageerde 5 jaren geleden

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 beoordeeld 5 jaren geleden
Poster
Eigenaar

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] reageerde 5 jaren geleden (Gemigreerd van 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 (Gemigreerd van github.com) beoordeeld 5 jaren geleden
elegaanz (Gemigreerd van github.com) reageerde 5 jaren geleden

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

Oh, I found what is wrong I think: the mention was inserted after the comment in the database.
elegaanz (Gemigreerd van github.com) beoordeeld 5 jaren geleden
elegaanz (Gemigreerd van github.com) reageerde 5 jaren geleden

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

OK, I'm too late, you already found it.
elegaanz (Gemigreerd van github.com) heeft deze veranderingen 5 jaren geleden goedgekeurd
elegaanz (Gemigreerd van github.com) heeft een reactie achtergelaten

Seems to work! Thank you! 😊

Seems to work! Thank you! :blush:

Reviewers

De pull request is samengevoegd als c9070930d2.
Je kunt ook command line instructies bekijken.

Stap 1:

Vanuit het project, check een branch uit en test de veranderingen.
git checkout -b notifications master
git pull origin notifications

Stap 2:

Voeg de wijzigingen samen en update ze op Forgejo.
git checkout master
git merge --no-ff notifications
git push origin master
Log in om deel te nemen aan deze discussie.
Geen beoordelaars
Geen mijlpaal
Niet toegewezen
2 deelnemers
Notificaties
Vervaldatum
De deadline is ongeldig of buiten bereik. Gebruik het formaat 'jjjj-mm-dd'.

Geen vervaldatum ingesteld.

Afhankelijkheden

Geen afhankelijkheden ingesteld.

Referentie: Plume/Plume#561
Laden…
Er is nog geen inhoud.