NCEAS / metacatui

MetacatUI: A client-side web interface for DataONE data repositories
https://nceas.github.io/metacatui
Apache License 2.0
42 stars 26 forks source link

Save an emergency draft of the EML in local storage so it can be retrieved after an error #216

Closed laurenwalker closed 4 years ago

laurenwalker commented 6 years ago

During our ADC meeting today, we discussed how to display this emergency draft to the user outside of the JS console. Here are some ideas:

If we show the "recovered draft" inside MetacatUI, we probably don't want to display it too prominently - perhaps just as a tab/panel in the user profile settings view. This is because we will have drafts for all editor submissions, even for metadata docs that were successfully saved. It might get confusing if a user sees a "recovered draft" for a metadata doc that was successfully saved.

csjx commented 6 years ago

I have drafts saving to offline storage successfully, but it is not consistent for all changes. Listening to change events on the EML211 model doesn't cover all changes due to sub-models, nor does it cover DataPackage collection changes. During our dev meeting today, Lauren suggested that we consolidate our event handling by creating an EML superclass that all models extend, and moving trickleUpChanges() into the superclass. I will work on this in the feature-emergency-draft branch.

csjx commented 6 years ago

After looking into creating an EMLModule superclass that would provide common event handling for all of the EML* objects, I found that we can't use this approach because the EML211 class would need to subclass both EMLModule and ScienceMetadata for our overall model to work. Backbone doesn't support multiple-inheritance, so this isn't possible. We have to create a superclass further up the chain.

I decided to subclass Backbone.Model with a NestedModel that basically just overrides the default set() method. Any changes to a Backbone model involve set() (even unset() and clear()), so this seemed like a reasonable point to add listeners to attributes that contain sub-models. So, DataONEObject extends NestedModel, as do all other EML* models not directly in the metadata chain (like EMLEntity).

Whenever a model calls set(), we look to see if any properties have direct sub-model values or arrays of sub-models that are instances of Backbone.Model. If they are, we listen for all events on these models as well, and filter by "change*" events. When a change happens, we trigger it on the parent, and then continue with the parent's set() call by calling the superclass set().

This looks to be working well, and every property change for parents and children will cause a draft to be saved (via DataONEObject.handleChange()). I've dealt with some code that is updating array values by reference rather than by value, which bypasses the event model. Now I'm dealing with some circular logic in EMLProject.updateDOM() in conjunction with this new set() behavior. We are getting some StackOverflow errors because we are setting attributes in the midst of building the project/personnel tree. This seems tractable, but will require changes to EMLProject. I'm testing each section of the editor, and dealing with bugs that this change has introduced. One that I haven't tracked down yet is the file upload status bar - it hangs instead of changing to a completed status, so I'll find that. Otherwise, I think the event refactor works pretty well, and that we will eventually be able to remove all trickleUpChange() methods since we now see all changes via NestedModel. This means that any new EML* models should subclass NestedModel, not Backbone.Model. See the overview diagram in https://github.com/NCEAS/metacatui/blob/feature-emergency-drafts/docs/design/editor/editor-design.rst.

laurenwalker commented 5 years ago

Moved to 2.3.0 so that a couple other features can be released beforehand, if possible

csjx commented 5 years ago

Offline draft saving is working well. I've introduced a regression in the status updating of a DataItemView when uploading a new data file, which I likely broke as I've tried to simplify the event triggers to reduce the number of draft saves. I'm tracking that down now, and am testing all inputs to be sure they induce a draft save. I've found some that don't, and have had to change the set() pattern they are using. I've also had to fix issues as I merge master changes in.

mbjones commented 4 years ago

@laurenwalker @csjx This issue just arose for someone again today that caused a lot of frustration with our system. They entered in a lot of attribute metadata for 2 data files. When they tried to submit, they got a red error back saying that there were missing fields. Somehow while inspecting the metadata and trying to find the missing metadata (they reiterated that every red-flagged field was filled in), their browser reloaded, and they lost everything they had entered.

We really need a way to save drafts locally (even when they are invalid), and repopulate the editor with that draft content so this can stop causing such frustration. So I upped the priority on this ticket to high.

amoeba commented 4 years ago

Working on this now, updating over on #1242.