Resolves #1286: record severity ratings

[aileenzeng]

Resolves #1286:

Earlier, severity ratings were not being recorded when the user hit the enter key too quickly after giving a rating. This was caused by an additional close event handler in ContextMenu.js that sometimes fired before the event handler in Keyboard.js. Severity ratings weren't logged because there is a conditional that checks if the ContextMenu is visible, and it would be marked as 'not visible' from the ContextMenu handler.

The event handler in ContextMenu.js has been removed - we now use the existing one in Keyboard.js.

[misaugstad]

So what is the testing protocol for this? When you say "record severity ratings" does that mean in the problem_severity table or the audit_task_interaction table? Etc.

[aileenzeng]

When I say 'record severity ratings', that refers to interactions that never even make it into the label table.

Testing protocol:

1. Place a label on the screen (w/ a severity option).
2. Give it a severity rating using a keyboard shortcut. Quickly exit out using the enter key.
3. Check the audit_task_interaction table to make sure that events are in happening order. I'd expect to see context menu/severity events in this order:

KeyboardShortcut_Severity_<number>
ContextMenu_ClosePressEnter
KeyboardShortcut_CloseContextMenu

NOTE: sometimes it looks like ContextMenu_ClosePressEnter and KeyboardShortcut_CloseContextMenu get switched up on my end. This is something that I'll look into more with the general interaction logging issues.

I'm borrowing the query that you gave me earlier to do this:

SELECT ACTION, TIMESTAMP, gsv_panorama_id, audit_task.audit_task_id, note
FROM audit_task_interaction
INNER JOIN audit_task ON audit_task.audit_task_id = audit_task_interaction.audit_task_id
WHERE task_start > '2018-06-11 00:00:00.00+00' 
    AND audit_task.audit_task_id= <your audit task id here>
    AND ACTION NOT IN ('LowLevelEvent_click',
                         'LowLevelEvent_keyup',
                         'LowLevelEvent_keydown',
                         'LowLevelEvent_mousemove',
                         'LowLevelEvent_mouseup',
                         'LowLevelEvent_mousedown',
                         'LowLevelEvent_mouseover',
                         'LowLevelEvent_mouseout',
                         'PanoId_Changed',
                         'POV_Changed',
                         'ViewControl_MouseUp',
                         'ViewControl_MouseDown')
ORDER BY TIMESTAMP

4. Check problem_severity to make sure that the severity ratings are being recorded.
   I'm using this query:

SELECT problem_severity.severity, problem_severity.label_id
FROM problem_severity
INNER JOIN sidewalk.label ON problem_severity.label_id = sidewalk.label.label_id
WHERE sidewalk.label.audit_task_id = <your audit task id here>

Try different combinations for rating severity - you can change step 2 by clicking on severity ratings (rather than hitting a key) and change Step 3 by using the Escape key or by clicking the 'OK' button.

[jonfroehlich]

NOTE: sometimes it looks like ContextMenu_ClosePressEnter and KeyboardShortcut_CloseContextMenu get switched up on my end. This is something that I'll look into more with the general interaction logging issues.

I think @shivenbhatt had found a link describing that sometimes Javascript events arrive out-of-order. Could this be related? @shivenbhatt, can you share your findings on this here please?

[aileenzeng]

I realized that I accidentally put these in the wrong order earlier when we used the enter key to close the context menu, but I will look into this more to see if this happens in other places. I fixed this in the most recent commit (7419019).

[AmethystGear]

Here's what I found about JS events getting out of order:

Sometimes when events are made, such as a key press or a click, javascript makes the events in the correct order, with the correct timestamps, but the events are executed in the incorrect order.
for example, let's say that pressing the key 'a' changes the screen color to red and pressing the key 'b' changes the screen color to blue. If the user presses 'a' and then 'b', the screen should end up being blue.
However, if the events execute out of order, then the code for the 'b' key would run first, and the code for the 'a' key would run next. In this case, the screen would end up being red.
Even when events execute out of order, event timestamps are guaranteed to be correct:
https://developer.mozilla.org/en-US/docs/Web/API/Event/timeStamp

The error where events get lost/out of place is documented here: https://stackoverflow.com/questions/11225694/why-are-onkeyup-events-not-firing-in-javascript-game

if the events are getting switched in the log, maybe we could have some sort of log cleaner that occasionally goes through the log and sorts events by their timestamp.

[misaugstad]

When I say 'record severity ratings', that refers to interactions that never even make it into the label table.

And by "make it into the label table", do you mean "make it into the problem_severity" table? Since that is where the severities are recorded in the database. And if it is the case that the actual severity was not being recorded (not just in the interaction log), then the testing instructions should include checking the problem_severity table as well.

[aileenzeng]

@misaugstad Severity ratings are missing from problem_severity as well. Will edit the instructions as well - thanks!

[misaugstad]

Nice! This is working great for closing the context menu via the keyboard (either by hitting enter, esc, or another label type keyboard shortcut), but it doesn't fix it when closing the menu by clicking outside the context menu with the mouse! Any ideas on how to fix that as well?

[aileenzeng]

Looking into this now - it looks like there's something that is causing keyboard events to be slightly delayed? When I click out of the context menu shortly after hitting the 1 key, I get this: (numbers on the right are time stamps):

click: hide context menu, (1535141996999)
1 key pressed, isOpen: false (1535141997100)

[aileenzeng]

@misaugstad Should be ready to go now!

[misaugstad]

Works great, nice work!