Reenable draft edit (#941)

* Editing draft submissions mostly works now

* Force journal to be added to doiInfo for draft submissions

* Submission details should now display data for draft submissions

* Prevent explicit journal lookup when DOI is present

* Fix tests

* Cleanup dead property

* Add more tests for submission workflow

* Prevent workflow-basic from overwriting existing submission metadata

* Cleaned up new submission route model.

* Fix deletion of publication in submission handler.

* Disable live reload since it does not work in our docker environment.

* Closes #926. Ignore spurious TransitionAborted errors.

* For safety, await when getting submission publication in model.

* Correct new submission model test.

* Persist files as soon as a user uploads

.ember-cli

@@ -6,7 +6,7 @@
     Setting `disableAnalytics` to true will prevent any data from being sent.
   */
   "disableAnalytics": false,
-
+  "liveReload": false,
   "testPort": 4200,
   "watcher": "polling"
 

app/components/display-metadata-keys.js

@@ -6,7 +6,14 @@ export default Component.extend({
   ignoreList: ['agent_information', 'external-submissions'],
 
   metadata: Ember.computed('submission', function () {
-    return JSON.parse(this.get('submission.metadata'));
+    try {
+      return JSON.parse(this.get('submission.metadata'));
+    } catch (e) {
+      /**
+       * We just need to handle the case where 'submission.metadata' does not exist.
+       */
+      return {};
+    }
   }),
 
   // TODO: could be changed to get the real label from relevant metadata schema!

app/components/submission-action-cell.js

@@ -15,6 +15,10 @@ export default Component.extend({
     return this.get('currentUser.user.id') === this.get('record.submitter.id');
   }),
 
+  submissionIsDraft: Ember.computed('record', function () {
+    return this.get('record.isDraft');
+  }),
+
   actions: {
     /**
      * Delete the specified submission record from Fedora.

app/components/workflow-basics.js

@@ -1,12 +1,24 @@
 import Component from '@ember/component';
 import { inject as service } from '@ember/service';
 
-
+/**
+ * IMPL NOTE:
+ * An interesting thing to consider: say a user goes through the submission workflow and ends up filling
+ * out some custom values in the metadata fields presented to them. The user then leaves the workflow
+ * leaving us with a 'draft' submission that can be resumed later. When we re-enter the submission
+ * workflow, we don't want to clobber the user-entered metadata.
+ *
+ * If submission metadata exists, it means the user at least made it past the metadata details step
+ * in the workflow. In this case, we should trust this metadata over data from the DOI service. Since
+ * data from the DOI is used to seed the metadata step, we can be sure that this data already exists
+ * in the submission metadata.
+ */
 export default Component.extend({
   store: service('store'),
   workflow: service('workflow'),
   currentUser: service('current-user'),
   doiService: service('doi'),
+  metadataService: service('metadata-blob'),
 
   inFlight: false,
   isShowingUserSearchModal: false,
@@ -32,7 +44,32 @@ export default Component.extend({
   },
   didInsertElement() {
     this._super(...arguments);
-    this.send('lookupDOI');
+    /**
+     * See IMPL NOTE above regarding the existance of submission metadata
+     */
+    try {
+      /**
+       * Set workflow doiInfo because of some weird timing between `routes/submissions/new/index#beforeModel`
+       * and `routes/submissions/new#model()` causing the doiInfo in 'workflow' to reset after it is
+       * defined by the incoming submission
+       */
+      this.get('workflow').setDoiInfo(JSON.parse(this.get('submission.metadata')));
+    } catch (e) {
+      /**
+       * Either 'metadata' is missing or malformed
+       */
+      this.send('lookupDOI');
+      /**
+       * If a Journal object exists in the model, then we have loaded an existing Submission
+       * with a Publication and a Journal. We need to make sure that this journal makes it
+       * into 'doiInfo' so it can be used in later steps.
+       *
+       * Only do this if there is no publication DOI, as the DOI lookup will cover this case.
+       */
+      if (!this.get('publication.doi') && this.get('journal')) {
+        this.send('selectJournal', this.get('journal'));
+      }
+    }
   },
   isValidDOI: Ember.computed('publication.doi', function () {
     return this.get('doiService').isValidDOI(this.get('publication.doi'));
@@ -146,9 +183,12 @@ export default Component.extend({
     /** Sets the selected journal for the current publication.
      * @param journal {DS.Model} The journal
      */
-    async selectJournal(journal) {
+    selectJournal(journal) {
       let doiInfo = this.get('doiInfo');
-      doiInfo = { 'journal-title': journal.get('journalName'), ISSN: journal.get('issns') };
+      // Formats metadata and adds journal metadata
+      let metadata = this.get('doiService').doiToMetadata(doiInfo, journal);
+      metadata['journal-title'] = journal.get('journalName');
+      doiInfo = this.get('metadataService').mergeBlobs(doiInfo, metadata);
 
       this.set('doiInfo', doiInfo);
 

app/components/workflow-files.js

@@ -4,7 +4,13 @@ import { inject as service } from '@ember/service';
 export default Component.extend({
   store: service('store'),
   workflow: service('workflow'),
+  submitter: service('submission-handler'),
   currentUser: service('current-user'),
+
+  _getFilesElement() {
+    return document.getElementById('file-multiple-input');
+  },
+
   actions: {
     deleteExistingFile(file) {
       swal({
@@ -30,7 +36,8 @@ export default Component.extend({
       });
     },
     getFiles() {
-      const uploads = document.getElementById('file-multiple-input');
+      const uploads = this._getFilesElement();
+
       if ('files' in uploads) {
         if (uploads.files.length !== 0) {
           for (let i = 0; i < uploads.files.length; i++) {
@@ -51,6 +58,9 @@ export default Component.extend({
                 newFile.set('fileRole', 'manuscript');
               }
               this.get('newFiles').pushObject(newFile);
+
+              // Immediately upload file
+              this.get('submitter').uploadFile(this.get('submission'), newFile);
             }
           }
         }

app/controllers/submissions/detail.js

@@ -422,7 +422,9 @@ export default Controller.extend({
         showCancelButton: true
       }).then((result) => {
         if (result.value) {
-          this.get('submissionHandler').deleteSubmission(submission);
+          this.get('submissionHandler').deleteSubmission(submission).then(() => {
+            this.transitionToRoute('submissions');
+          });
         }
       });
     }

app/controllers/submissions/new/basics.js

@@ -8,6 +8,7 @@ export default Controller.extend({
   publication: alias('model.publication'),
   preLoadedGrant: alias('model.preLoadedGrant'),
   submissionEvents: alias('model.submissionEvents'),
+  journal: alias('model.journal'),
   parent: Ember.inject.controller('submissions.new'),
 
   // these errors start as false since you don't want to immediately have all fields turn red
@@ -96,6 +97,13 @@ export default Controller.extend({
           }
         }
       }
+
+      // After validation, we can save the publication to the Submission
+      const publication = this.get('publication');
+      publication.save().then(() => {
+        this.set('submission.publication', publication);
+      });
+
       this.send('loadTab', gotoRoute);
     },
     validateTitle() {

app/models/journal.js

@@ -15,9 +15,5 @@ export default DS.Model.extend({
   }),
   isMethodB: Ember.computed('pmcParticipation', function () {
     return this.get('pmcParticipation') ? this.get('pmcParticipation').toLowerCase() === 'b' : false;
-  }),
-
-  // TODO MUST REMOVE
-  // Artifact of incomplete data - once Journal data is updated this must be removed
-  name: DS.attr('string')
+  })
 });

app/models/submission.js

@@ -7,7 +7,7 @@ export default DS.Model.extend({
   }),
   submittedDate: DS.attr('date'),
   source: DS.attr('string', { defaultValue: 'pass' }),
-  metadata: DS.attr('string', { defaultValue: '[]' }), // Stringified JSON
+  metadata: DS.attr('string'), // Stringified JSON
   submitted: DS.attr('boolean', { defaultValue: false }),
   submissionStatus: DS.attr('string'),
   submitterName: DS.attr('string'),
@@ -82,8 +82,7 @@ export default DS.Model.extend({
    * @returns {boolean} is this a draft submission?
    */
   isDraft: Ember.computed('submitted', 'submissionStatus', function () {
-    // TODO: after model update, we can just check if submission status === 'draft'
-    // return this.get('record.submissinoStatus') === 'draft';
-    return !this.get('submitted') && !this.get('submissionStatus');
+    return this.get('submissionStatus') === 'draft';
+    // return !this.get('submitted') && !this.get('submissionStatus');
   })
 });

app/routes/submissions/new.js

@@ -17,16 +17,18 @@ export default CheckSessionRoute.extend({
     }
   },
 
-  // Return a promise to load count objects starting at offsefrom of given type.
+  // Return a promise to load count objects starting at offset from of given type.
   loadObjects(type, offset, count) {
     return this.get('store').query(type, { query: { match_all: {} }, from: offset, size: count });
   },
-  model(params) {
+
+  async model(params) {
     let preLoadedGrant = null;
     let newSubmission = null;
     let submissionEvents = null;
     let files = null;
-    let publication = this.get('store').createRecord('publication');
+    let publication = null;
+    let journal = null;
 
     if (params.grant) {
       preLoadedGrant = this.get('store').findRecord('grant', params.grant);
@@ -36,43 +38,53 @@ export default CheckSessionRoute.extend({
     const policies = this.loadObjects('policy', 0, 500);
 
     if (params.submission) {
-      return this.get('store').findRecord('submission', params.submission).then((sub) => {
-        newSubmission = this.get('store').findRecord('submission', params.submission);
-        publication = sub.get('publication');
-        submissionEvents = this.get('store').query('submissionEvent', {
-          sort: [
-            { performedDate: 'asc' }
-          ],
-          query: {
-            term: {
-              submission: sub.get('id')
-            }
-          }
-        });
-        files = this.get('store').query('file', {
+      // Operating on existing submission
+
+      newSubmission = await this.get('store').findRecord('submission', params.submission);
+      publication = await newSubmission.get('publication');
+      journal = publication.get('journal');
+
+      submissionEvents = this.get('store').query('submissionEvent', {
+        sort: [
+          { performedDate: 'asc' }
+        ],
+        query: {
           term: {
-            submission: sub.get('id')
+            submission: newSubmission.get('id')
           }
-        });
-        return Ember.RSVP.hash({
-          repositories,
-          newSubmission,
-          submissionEvents,
-          publication,
-          policies,
-          preLoadedGrant,
-          files
-        });
+        }
+      });
+
+      files = this.get('store').query('file', {
+        term: {
+          submission: newSubmission.get('id')
+        }
+      });
+
+      // Also seed workflow.doiInfo with metadata from the Submission
+      const metadata = newSubmission.get('metadata');
+      if (metadata) {
+        this.get('workflow').setDoiInfo(JSON.parse(metadata));
+      }
+    } else {
+      // Starting a new submission
+
+      publication = this.get('store').createRecord('publication');
+
+      newSubmission = this.get('store').createRecord('submission', {
+        submissionStatus: 'draft'
       });
     }
-    newSubmission = this.get('store').createRecord('submission');
-    const h = Ember.RSVP.hash({
+
+    return Ember.RSVP.hash({
       repositories,
       newSubmission,
+      submissionEvents,
       publication,
       policies,
-      preLoadedGrant
+      preLoadedGrant,
+      files,
+      journal
     });
-    return h;
   }
 });

app/services/error-handler.js

@@ -6,6 +6,12 @@ import ENV from 'pass-ember/config/environment';
 
 export default Service.extend({
   handleError(error) {
+    if (error.name == 'TransitionAborted') {
+      // Ignore what seems to be spurious transition errors.
+      // See https://github.com/emberjs/ember.js/issues/12505
+      return;
+    }
+
     console.log(`Handling error: ${JSON.stringify(error.message)}`);
 
     // Error stack is non-standard. Show if available.
@@ -16,9 +22,7 @@ export default Service.extend({
       console.log(error);
     }
 
-    if (error.name == 'TransitionAborted') {
-      // Ignore
-    } else if (error.message === 'shib302') {
+    if (error.message === 'shib302') {
       // Sent from the session checker to indicate the session has timed out.
       this.handleSessionTimeout(error);
     } else if (error.message === 'didNotLoadData') {