MSFREACH / msf-reach

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

Errors, duplication on event open and on past response editing #632

Closed matthewberryman closed 6 years ago

matthewberryman commented 6 years ago
  1. I'm getting 400 bad request errors on mission editing on the landing page. {"statusCode":400,"error":"Bad Request","message":"child \"eventId\" fails because [\"eventId\" is required]","validation":{"source":"body","keys":["eventId"]}}. Seems the API (possibly?) and client side code was broken.
  2. Also reports that information entered for one is being copied across past responses (ref. email trail to be sent in a minute).
matthewberryman commented 6 years ago

Just noting that 2. may be a client side thing with Vue.js only (?)

matthewberryman commented 6 years ago

Ok I had a skype session with the user and, noting they are using Safari, Confirmed that (2) is (or was - due to caching) an issue in master branch. Not sure if client side only (i.e. is it not clearing stuff between modals?). Workflow was edit past mission on landing page, make some changes (e.g. description) and then info was duplicated into other past missions.

May be similar issue causing event duplication although couldn't reproduce on test side nor on dev; workflow for that was (a) open event, (b) edit on event page and set status to completed then (c) close event (bottom of event page).

(1) is something separate was seeing in dev branch on dev site, but my feeling is to leave this as one issue to cover testing around past mission editing.

matthewberryman commented 6 years ago

I'm having trouble reproducing the (master branch) issues - be sure to test on Safari especially.

qclin commented 6 years ago

@matthewberryman i tested for (1) saved okay on dev https://github.com/MSFREACH/msf-reach/blob/dev/public/events/mission.html#L737 grabs eventId from the url hash like everywhere else.

matthewberryman commented 6 years ago

@qclin Sorry, my bad, (1) is for landing page - I am seeing this consistently for all missions there.

matthewberryman commented 6 years ago

@qclin @mehrdadgit @talltom any testing on (2) done?

matthewberryman commented 6 years ago

Ok I haven't figured out the duplication on event edit, however I have figured out the duplication (x2 at least) on event creation and close. It's because (a) you can set status to completed and that triggers a close of event (copy to missions) and then also (b) click on close button at bottom of page and that also triggers a close of event (i.e. another copy to missions).

matthewberryman commented 6 years ago

So we need to stick with one way of closing it, make sure date is prompted for correctly (part of #633 ) and then make sure information is copied over correctly (#635 ) and that may also help with some of #633 too although there are some completely separate subissues on #633

matthewberryman commented 6 years ago

Ok, method (a) is copying over types and other info correctly but method (b) is not, so for now I will remove that button and inform users, then we need to tidy up the js code later. That then leaves some of #633 to go.

matthewberryman commented 6 years ago

Just noting (after speaking with @mehrdadgit ) neither of us have been able to repro the mission editing issue so considering that closed. Mission opening (on event closure) duplication fixed in c5a1691

matthewberryman commented 6 years ago

https://github.com/MSFREACH/msf-reach/commit/d96be7aabeb6b1c7893adf5feb5b5147d7d33eb5 allows correct editing (and tested) despite false not being supported according to jquery docs. I think we need a proper fix (the extend seems to be there to fill out details where blank) so something along those lines.

@mehrdadgit

mehrdadgit commented 6 years ago

Thanks @matthewberryman done in above commit and please test further

matthewberryman commented 6 years ago

Thanks. Have tested different use cases and git cherry-picked into master. https://github.com/MSFREACH/msf-reach/commit/d96be7aabeb6b1c7893adf5feb5b5147d7d33eb5 was working but https://github.com/MSFREACH/msf-reach/commit/51167ed4556077139eacdd11a60448a026df7db8 has the advantages of:

  1. Using a documented jquery feature.
  2. Making more sense in terms of logic.
  3. Explains why original case wasn't working (because you need to merge missions and default event into an empty object each time, not default event object that then gets over-written).