Feature/import sessions

[rebeccamadsen]

Resolves #235. There is a new option in the File menu to import a session. Also uses the session panel on the dashboard overview as a drop point. Can import multiple graphml files, and accommodates graphml files with multiple sessions (as long as they all match the same protocol).

Matches the import formatting currently in NC (using the uuids for variables in a session, not the inverted version with variable names), so dashboard views of the variables don't display the correct data. This is true of sessions exported from master in NC as well, so this is a separate issue, or perhaps addressed when we update the network-exporters reference #287.

Sometimes we refer to "cases" and sometimes "sessions"...should we pick one for user facing, or is that interchangeable for users?

[jthrilly]

This is working wonderfully - really nice work! I haven't dug into the implementation yet, but here are some thoughts on the user-facing aspects of this:

Drop zone

I'm not sure about the drop zone - it feels awkward having it in the session box. I know I didn't give you direction on this, but seeing it work now has made me realize that I think we should have a full app overlay (just like like slack) which handles both protocols and files being dropped. The overlay can cover the entire "overview" area of the app (everything but the sidebar), and should work even when there are no protocols installed yet (to handle dropping protocol files).

It should be easy to set up using framer-motion (not sure if we have talked through this yet, but Steve and I are using it pretty extensively now, and really enjoying it - let me know if you want me to help you get this set up).

If you want to do something slightly simpler, you could leave the current protocol drop zone in the sidebar, and just have the session drop zone in the overview. Up to you.

Small bug

I found a small issue with the recent sessions panel. If I have three sessions (case IDs "A", "B", and "C") and I delete "A", when I import it again the recent sessions list shows "2 of 3" and omits "C". Refreshing the app shows all three sessions.

Layout cordinates with screen space values

I realized there will likely be an issue with layout coordinates if the NC client is set to use screen space coordinates. When Server is updated to the new network exporters, if a user applies this option again, the coordinates for existing sessions that used it will be messed up. For the sake of simplicity I am going to remove this option from NC, and keep it in Server. However, we do still need a plan for what to do when layout values outside of 0-1 are encountered, since this will still be the case if the user tries to re-import case data that has been exported by Server with this option.

We could either:

1. Change the way the screen space coords feature works in network exporters, so that it creates a new string based variable that can always be encoded and doesnt change the layout variable itself
2. Refuse to import those values. Either drop the variable, or the session.
3. Try to re-normalize by asking the user to enter screen width and height in a prompt.

I am in favor of (1).

User feedback

When importing multiple sessions an error in one will cancel the whole process. I think we should import all the sessions we can, and then show a warning dialog at the end that not all sessions could be imported (give details of which).

Related to this is user feedback - can we improve it? With a 10mb graphml, there is about a second pause where nothing visibly happens. This is probably acceptable, but maybe some sort of progress overlay would be a good idea. Errors should be error dialogs and have more meaningful info (For example "associated protocol does not exist on this server" - which protocol? Same with the complete toasts - how many sessions were imported?).

[jthrilly]

Just a note to say probably don't update this against master until we have resolved the above, since the changes in master will reset the app data.

[wwqrd]

I'm not sure I understand:

Change the way the screen space coords feature works in network exporters, so that it creates a new string based variable that can always be encoded and doesnt change the layout variable itself

Could you give an example of what that would look like?

[jthrilly]

Could you give an example of what that would look like?

Sure, its super simple.

Instead of having:

{
  layout: {
    x: "1467", // Formerly 0.9
    y: "355", // Formerly 0.234
  }
}

You have:

{
  layout: {
    x: 0.9,
    y: 0.234,
  },
  layoutScreenSpaceX: "1467",
  layoutScreenSpaceY: "355",
}

The point is that right now we modify the layout variable which is a one way process if the parameters for the screen size aren't known (which they won't be if all you have is the file that resulted).

I'm suggesting changing it so we always keep the existing normalized layout variable, but optionally add a screen space layout variable that is stored as two string variables. These will be encoded "as-is" as non codebook variables.

[rebeccamadsen]

Will look at the small bug and the drop zone. I like this proposal on taking the whole overview space -- I hadn't thought of that; I wasn't super satisfied with the other ideas I explored.

Nominally an error in one session should cancel that file, but any other files dropped at the same time will still attempt to load their session(s). If it behaved differently for you, that's a bug. I can tweak that behaviour, and attempt to insert in the database one session at a time instead of sending all the sessions for the file together to the backend. I think I assumed that an issue with the file would need to be dealt with at the file level and not the session level, but can see that may not be true.

I'll work on enriching the feedback at the end. I hadn't done a progress indicator because I figured the app should still be usable, but can definitely switch that up -- that's cool. So we should use dialogs instead of the app messages for all feedback, or only if there were errors? Should the dialogs be preferred for importing from NC as well as the graphml? Right now it's using the same mechanism to insert both, and to send app messages back from both.

With the layout issue -- I prefer option 1 as well. I'd hate to drop the session or the variables, and re-normalizing seems prone to error (some users may not know the screen size?). What would the graphml look like? Are the string values still useful in graphml? Is that a network-exporters only change or do I need some action here to prep for that?

[jthrilly]

Nominally an error in one session should cancel that file,

I think this is the difference - what happens if its a "file" with multiple sessions? Does that cancel the whole file? If so, that's what I would like to change.

So we should use dialogs instead of the app messages for all feedback, or only if there were errors?

The way I'm thinking of it is that the "toast" notifications are designed to be self-dismissing. Errors are something that requires active dismissal, whereas notifications that something finished or succeeded aren't. Does that make sense? So I would use the toast for successes, and the error for failure. This is similar to how entity resolution works.

Should the dialogs be preferred for importing from NC as well as the graphml?

As far as I was aware, sending data from NC didn't display anything visible in Server. If that's changed with this, it should use the same mechanism for notifying the user too (cool feature if that's what you mean!).

With the layout issue -- I prefer option 1 as well. I'd hate to drop the session or the variables, and re-normalizing seems prone to error (some users may not know the screen size?). What would the graphml look like? Are the string values still useful in graphml? Is that a network-exporters only change or do I need some action here to prep for that?

I think this is a network-exporters only change, unless you added some logic to deal specifically with storing layout variables. At a minimum I suppose you should already be (1) detecting that a given variable is a layout variable, (2) recreating the x/y object representing the layout variable. So i guess that can all stay the same.

The graphML will just look as though there are two additional non-codebook variables (just like if they came from external data), which I believe will just pass through everything.

[rebeccamadsen]

This is ready for another look.

[rebeccamadsen]

Oh except the layout coordinates -- do we need a separate issue for network-exporters to track that, or is it being addressed somewhere else?

[jthrilly]

Oh except the layout coordinates -- do we need a separate issue for network-exporters to track that, or is it being addressed somewhere else?

We need a separate issue for that!

[jthrilly]

Feature wise, this is working well.

This is the feedback that I am currently working on addressing myself:

• [x] Large numbers of files (both successfully imported and with errors) cause visual glitches. Success causes overflowing toast, and failure causes overflowing dialog.
• [x] Dropzone mouse over icon causes dropzone to visully glitch
• [x] Loading spinner during import and no user feedback
• [x] Error messages not formatted and not descriptive. "This server already contains this session"
• [x] Success messages not formaatted and not descriptive "[number] from [protocol name]