search

MSFREACH / msf-reach

Web platform for MSF-REACH
https://msf-reach.org/
4 stars 5 forks source link

check latest notification is actually latest notification #550

Closed matthewberryman closed 6 years ago

mehrdadgit commented 6 years ago

I noticed this bug when I was doing #524 and have fixed it; should be fixed in dev

matthewberryman commented 6 years ago

@mehrdadgit

  1. See https://github.com/MSFREACH/msf-reach/commit/17598a97c312e2feeebd06d48765c8f0b4747d1c#r29902235
  2. There's still a problem for the places where just the latest notification is displayed (popups, top section of events page) caused by my incorrect assumption about ordering of notifications going in, particularly in a multi-user editing scenario. I believe ff14b61d11d9ca140f550bb002abcd643e95b08d in issues/550 branch should address it. If you have time to have a quick look at that and test it out, then that would be helpful :)
mehrdadgit commented 6 years ago

@matthewberryman better to write to common function somewhere to get latest notification from the array of notifications, and without using sort... I will do that.

matthewberryman commented 6 years ago

@mehrdadgit yeah I know, I was just after a quick fix as I have a lot on :/

Can you please do this quickly and then work on #537 urgently.

mehrdadgit commented 6 years ago

Done @matthewberryman I did some tests but please feel free to review https://github.com/MSFREACH/msf-reach/commit/5b0dbd2b2fb5f3f8a2a277ba1746c98a76840542 and let me know if there's any issue :)

matthewberryman commented 6 years ago

Great, thanks @mehrdadgit . I have reviewed and tested and will merge into master.