From ff14627222cac754d2076df5a17550274405ceed Mon Sep 17 00:00:00 2001 From: John Abrahams Date: Fri, 31 May 2019 10:25:59 -0400 Subject: [PATCH] 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 | 2 +- app/components/display-metadata-keys.js | 9 +- app/components/submission-action-cell.js | 4 + app/components/workflow-basics.js | 48 ++++++- app/components/workflow-files.js | 12 +- app/controllers/submissions/detail.js | 4 +- app/controllers/submissions/new/basics.js | 8 ++ app/models/journal.js | 6 +- app/models/submission.js | 7 +- app/routes/submissions/new.js | 76 ++++++----- app/services/error-handler.js | 10 +- app/services/submission-handler.js | 28 ++-- app/services/workflow.js | 2 +- .../components/submission-action-cell.hbs | 4 +- app/templates/components/workflow-basics.hbs | 13 +- app/templates/submissions/detail.hbs | 8 +- app/templates/submissions/new/basics.hbs | 1 + .../components/workflow-basics-test.js | 124 ++++++++++++++---- .../components/workflow-files-test.js | 57 ++++++-- .../controllers/submissions/detail-test.js | 22 ++-- .../submissions/new/basics-test.js | 11 +- tests/unit/routes/submissions/new-test.js | 96 +++++++++++++- .../unit/services/submission-handler-test.js | 22 +++- 23 files changed, 433 insertions(+), 141 deletions(-) diff --git a/.ember-cli b/.ember-cli index 5b6f4b50d..8363e9503 100644 --- a/.ember-cli +++ b/.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" diff --git a/app/components/display-metadata-keys.js b/app/components/display-metadata-keys.js index b41c979ba..6193612a7 100644 --- a/app/components/display-metadata-keys.js +++ b/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! diff --git a/app/components/submission-action-cell.js b/app/components/submission-action-cell.js index a788a1b58..f82914b91 100644 --- a/app/components/submission-action-cell.js +++ b/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. diff --git a/app/components/workflow-basics.js b/app/components/workflow-basics.js index 223b43f05..995aa1057 100644 --- a/app/components/workflow-basics.js +++ b/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); diff --git a/app/components/workflow-files.js b/app/components/workflow-files.js index a3d1daaaf..8ebabb5a4 100644 --- a/app/components/workflow-files.js +++ b/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); } } } diff --git a/app/controllers/submissions/detail.js b/app/controllers/submissions/detail.js index b435a598e..7ab3f0913 100644 --- a/app/controllers/submissions/detail.js +++ b/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'); + }); } }); } diff --git a/app/controllers/submissions/new/basics.js b/app/controllers/submissions/new/basics.js index 492d78d59..0363456fc 100644 --- a/app/controllers/submissions/new/basics.js +++ b/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() { diff --git a/app/models/journal.js b/app/models/journal.js index 88e14e1d9..de92cebeb 100644 --- a/app/models/journal.js +++ b/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') + }) }); diff --git a/app/models/submission.js b/app/models/submission.js index 7c16972b6..baf0fb02b 100644 --- a/app/models/submission.js +++ b/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'); }) }); diff --git a/app/routes/submissions/new.js b/app/routes/submissions/new.js index 48ec2bf4b..6b06d9357 100644 --- a/app/routes/submissions/new.js +++ b/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; } }); diff --git a/app/services/error-handler.js b/app/services/error-handler.js index 2158ad9bc..72434905f 100644 --- a/app/services/error-handler.js +++ b/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') { diff --git a/app/services/submission-handler.js b/app/services/submission-handler.js index bc6c4d1f6..51bd62e5f 100644 --- a/app/services/submission-handler.js +++ b/app/services/submission-handler.js @@ -77,7 +77,7 @@ export default Service.extend({ }, /** - * _uploadFile - Internal method which adds a file to a submission in the + * _uploadFile - method which adds a file to a submission in the * repository. The bytes of the local file corresponding to the File object * are read and uploaded to the Submission container. The modified File object * is persisted. @@ -86,7 +86,7 @@ export default Service.extend({ * @param {File} file Local file uploaded. * @returns {Promise} Promise resolves to the File object. */ - _uploadFile(sub, file) { + uploadFile(sub, file) { return new Promise((resolve, reject) => { let reader = new FileReader(); @@ -135,13 +135,15 @@ export default Service.extend({ /** * submit - Perform or prepares a submission. Persists the publication, associate * the submission with the saved publication, modify the submission appropriately, - * uploads files, and finally persists the submission with an appropraite event + * and finally persists the submission with an appropraite event * to hold the comment. Repositories of type web-link are removed if submission * is actually submitted. * + * Files are already uploaded during the Files step of the workflow, so they do not + * need to be handled here. + * * @param {Submission} submission * @param {Publication} publication Persisted and associated with Submission. - * @param {Files} files Local files to upload and add to Submission. * @param {String} comment Comment added as to SubmissionEvent. * @returns {Promise} Promise to perform the operation. */ @@ -153,7 +155,7 @@ export default Service.extend({ submission.set('publication', p); return submission.save(); - }).then(s => Promise.all(files.map(f => this._uploadFile(s, f))).then(() => this._finishSubmission(s, comment))); + }).then(s => this._finishSubmission(s, comment)); }, /** @@ -266,17 +268,17 @@ export default Service.extend({ async deleteSubmission(submission) { const result = []; - const pub = submission.get('publication'); - if (pub && pub.hasOwnProperty('destroyRecord')) { + const pub = await submission.get('publication'); + if (pub) { result.push(pub.destroyRecord()); } - // eslint-disable-next-line function-paren-newline - result.push( - this._getFiles(submission.get('id')).then(files => files.map(file => file.destroyRecord())) - ); // eslint-disable-line function-paren-newline + const files = await this._getFiles(submission.get('id')); + if (files) { + result.push(files.map(file => file.destroyRecord())); + } - result.push(submission.destroyRecord()); - return Promise.all(result); + await Promise.all(result); + return submission.destroyRecord(); } }); diff --git a/app/services/workflow.js b/app/services/workflow.js index b7f7a31ff..7f15fcc36 100644 --- a/app/services/workflow.js +++ b/app/services/workflow.js @@ -17,7 +17,7 @@ export default Service.extend({ this.setMaxStep(1); this.setPmcPublisherDeposit(false); this.setFilesTemp([]); - this.setDoiInfo([]); + this.setDoiInfo({}); this.setDefaultRepoLoaded(false); }, getCurrentStep() { diff --git a/app/templates/components/submission-action-cell.hbs b/app/templates/components/submission-action-cell.hbs index dcb045bf3..4d4d6a0b2 100644 --- a/app/templates/components/submission-action-cell.hbs +++ b/app/templates/components/submission-action-cell.hbs @@ -18,8 +18,8 @@ {{#link-to "submissions.detail" record.id class="btn btn-outline-primary"}} Review submission {{/link-to}} -{{else if record.isDraft}} - {{#link-to "submissions.new" (query-params submission=record.id) disabled=true class="btn btn-outline-primary w-100 mb-2"}} +{{else if submissionIsDraft}} + {{#link-to "submissions.new" (query-params submission=record.id) class="btn btn-outline-primary w-100 mb-2"}} Edit {{/link-to}} diff --git a/app/templates/components/workflow-basics.hbs b/app/templates/components/workflow-basics.hbs index 7878deab0..840b850c4 100644 --- a/app/templates/components/workflow-basics.hbs +++ b/app/templates/components/workflow-basics.hbs @@ -51,13 +51,12 @@

{{input - class=doiClass - value=publication.doi - keyUp=(action "lookupDOI") - mouseUp=(action "lookupDOI") - id="doi" - placeholder="Leave blank if your manuscript or article has not yet been assigned a DOI" - readonly=submission.id + class=doiClass + value=publication.doi + keyUp=(action "lookupDOI") + mouseUp=(action "lookupDOI") + id="doi" + placeholder="Leave blank if your manuscript or article has not yet been assigned a DOI" }}
{{if (and publication.doi (not isValidDOI)) 'Please provide a valid DOI'}} diff --git a/app/templates/submissions/detail.hbs b/app/templates/submissions/detail.hbs index b50711d27..14869d00b 100644 --- a/app/templates/submissions/detail.hbs +++ b/app/templates/submissions/detail.hbs @@ -67,12 +67,12 @@ Finish submission {{/link-to}} {{else if model.sub.isDraft}} - {{#link-to 'submissions.new' (query-params submission=model.sub.id) disabled=true class="btn btn-outline-primary text-right"}} + {{#link-to 'submissions.new' (query-params submission=model.sub.id) class="btn btn-outline-primary text-right"}} Edit submission {{/link-to}} - {{#link-to (action 'deleteSubmission' model.sub) class="btn btn-outline-danger text-right"}} + {{/if}}
@@ -236,8 +236,6 @@ {{file.description}} - {{else}} -
Loading files...
{{/each}} diff --git a/app/templates/submissions/new/basics.hbs b/app/templates/submissions/new/basics.hbs index 5c29d83da..6dfa4fee6 100644 --- a/app/templates/submissions/new/basics.hbs +++ b/app/templates/submissions/new/basics.hbs @@ -9,6 +9,7 @@ publication=publication preLoadedGrant=preLoadedGrant doiInfo=doiInfo + journal=journal flaggedFields=flaggedFields validateTitle=(action "validateTitle") validateJournal=(action "validateJournal") diff --git a/tests/integration/components/workflow-basics-test.js b/tests/integration/components/workflow-basics-test.js index af5b640a6..859ca798b 100644 --- a/tests/integration/components/workflow-basics-test.js +++ b/tests/integration/components/workflow-basics-test.js @@ -1,7 +1,7 @@ import { setupRenderingTest } from 'ember-qunit'; import hbs from 'htmlbars-inline-precompile'; import { module, test } from 'qunit'; -import { click, fillIn, triggerKeyEvent, render } from '@ember/test-helpers'; +import { fillIn, render, waitUntil } from '@ember/test-helpers'; import { run } from '@ember/runloop'; module('Integration | Component | workflow basics', (hooks) => { @@ -58,7 +58,8 @@ module('Integration | Component | workflow basics', (hooks) => { doi: '1234/4321', issue: 1, title: 'Quantitative profiling of carbonyl metabolites directly in crude biological extracts using chemoselective tagging and nanoESI-FTMS', - volume: '143' + volume: '143', + journal: Ember.Object.create({ journalName: 'moo-title' }) }) }); }, @@ -73,18 +74,6 @@ module('Integration | Component | workflow basics', (hooks) => { } }); - const mockNlmta = Ember.Service.extend({ - getNlmtaFromIssn() { - return { - nlmta: 'nl-moo-ta', - 'issn-map': { - '0003-2654': { 'pub-type': 'print' }, - '1364-5528': { 'pub-type': 'electronic' } - } - }; - } - }); - const mockStore = Ember.Service.extend({ query(type, query) { return Promise.resolve(Ember.A()); @@ -102,32 +91,99 @@ module('Integration | Component | workflow basics', (hooks) => { this.owner.unregister('service:doi'); this.owner.register('service:doi', mockDoiService); - this.owner.unregister('service:nlmta'); - this.owner.register('service:nlmta', mockNlmta); - this.owner.unregister('service:store'); this.owner.register('service:store', mockStore); }); }); - // eslint-disable-next-line prefer-arrow-callback - test('it renders', async function (assert) { + test('lookupDOI should set doiInfo and publication', async function (assert) { + this.set('validateTitle', () => assert.ok(true)); + await render(hbs`{{workflow-basics submission=submission + publication=publication preLoadedGrant=preLoadedGrant + doiInfo=doiInfo + flaggedFields=flaggedFields + validateTitle=(action validateTitle) + validateJournal=(action validateJournal) + validateSubmitterEmail=(action validateSubmitterEmail) + next=(action loadNext)}}`); + + // Add a DOI to UI + await fillIn('#doi', '1234/4321'); + + assert.equal(this.get('doiInfo').DOI, '10.1039/c7an01256j'); + assert.equal(this.get('publication.doi'), '1234/4321'); + assert.equal(this.get('publication.issue'), '1'); + }); + + /** + * Test case: + * - Draft submission + * - Publication has a DOI + * + * Expect: + * DOI, title, and journal should be added to the UI inputs when things settle + */ + test('Info is filled in when a submission is provided with a publication and DOI', async function (assert) { + const publication = this.get('publication'); + const submission = this.get('submission'); + + submission.set('publication', publication); + + await render(hbs`{{workflow-basics + submission=submission publication=publication + preLoadedGrant=preLoadedGrant doiInfo=doiInfo flaggedFields=flaggedFields validateTitle=(action validateTitle) validateJournal=(action validateJournal) validateSubmitterEmail=(action validateSubmitterEmail) next=(action loadNext)}}`); - assert.ok(true); + assert.ok(this.element); + + // await waitUntil(() => new Promise(resolve => setTimeout(() => { debugger; resolve(); }, 1000))); + // 2 inputs, 1 textarea + const inputs = this.element.querySelectorAll('input'); + const title = this.element.querySelector('textarea'); + + assert.equal(inputs.length, 2, 'There should be two input elements'); + assert.ok(title, 'No "title" textarea element found'); + + assert.ok(title.textLength > 0, 'No title value found'); + inputs.forEach((inp) => { + const msg = `No value found for input "${inp.parentElement.parentElement.querySelector('label').textContent}"`; + assert.ok(!!inp.value, msg); + }); }); - test('lookupDOI should set doiInfo and publication', async function (assert) { - // this.set('lookupDOI', () => assert.ok(true)); - this.set('validateTitle', () => assert.ok(true)); + /** + * Test this by first adding metadata to the submission with some fields containing values that + * differ from equivalent fields defined in the mock DOI service. + */ + test('Draft submission metadata is not overwritten by DOI data', async function (assert) { + // First override the DOI service to ensure that it isn't called + this.set('doiService', { + resolveDOI: () => assert.ok(false, 'resolveDOI should not be called'), + formatDOI: () => asssert.ok(false, 'formatDOI should not be called'), + isValidDOI: () => assert.ok(false, 'isValidDOI should not be called') + }); + + const pub = Ember.Object.create({ + doi: 'ThisIsADOI', + title: 'Moo title', + journal: Ember.Object.create({ journalName: 'Moo Journal' }) + }); + this.set('publication', pub); + + // Add metadata to submission + const submission = Ember.Object.create({ + publication: pub, + metadata: JSON.stringify({ title: 'You better use this' }) + }); + this.set('submission', submission); await render(hbs`{{workflow-basics submission=submission @@ -139,12 +195,24 @@ module('Integration | Component | workflow basics', (hooks) => { validateJournal=(action validateJournal) validateSubmitterEmail=(action validateSubmitterEmail) next=(action loadNext)}}`); + assert.ok(this.element); - // Add a DOI to UI - await fillIn('#doi', '1234/4321'); + // Check values of various inputs + const inputs = this.element.querySelectorAll('input'); + const title = this.element.querySelector('textarea'); - assert.equal(this.get('doiInfo').DOI, '10.1039/c7an01256j'); - assert.equal(this.get('publication.doi'), '1234/4321'); - assert.equal(this.get('publication.issue'), '1'); + assert.equal(inputs.length, 2, 'There should be two input elements'); + assert.ok(title, 'No "title" textarea element found'); + + assert.ok(title.textLength > 0, 'No title value found'); + inputs.forEach((inp) => { + const msg = `No value found for input "${inp.parentElement.parentElement.querySelector('label').textContent}"`; + assert.ok(!!inp.value, msg); + }); + + // Check submission metadata + const md = JSON.parse(this.get('submission.metadata')); + assert.ok(md, 'no metadata found'); + assert.equal(md.title, 'You better use this', 'Unexpected metadata!'); }); }); diff --git a/tests/integration/components/workflow-files-test.js b/tests/integration/components/workflow-files-test.js index cae64e2fd..475824b09 100644 --- a/tests/integration/components/workflow-files-test.js +++ b/tests/integration/components/workflow-files-test.js @@ -1,11 +1,12 @@ import { setupRenderingTest } from 'ember-qunit'; import hbs from 'htmlbars-inline-precompile'; import { module, test } from 'qunit'; +import { run } from '@ember/runloop'; module('Integration | Component | workflow files', (hooks) => { setupRenderingTest(hooks); - test('it renders', function (assert) { + hooks.beforeEach(function () { let submission = Ember.Object.create({ repositories: [], grants: [] @@ -17,13 +18,53 @@ module('Integration | Component | workflow files', (hooks) => { this.set('newFiles', newFiles); this.set('loadPrevious', (actual) => {}); this.set('loadNext', (actual) => {}); + }); + + /** + * Intent is to fake the file input element to make it look like a file is present, + * then test the behavior of #getFiles(). + * + * In this case, submission-handler#uploadFiles() should be called once for each + * mock file + */ + test('Files should upload immediately', function (assert) { + assert.expect(6); + + this.owner.register('service:store', Ember.Service.extend({ + createRecord: () => Promise.resolve() + })); + this.owner.register('service:submission-handler', Ember.Service.extend({ + uploadFile: (submission, file) => { + assert.ok(submission); + assert.ok(file); + + assert.equal(file.get('name'), 'Fake-file-name'); + assert.equal(file.get('mimeType'), 'plain'); + assert.deepEqual(file.get('_file'), { size: 100, name: 'Fake-file-name', type: 'text/plain' }); + } + })); - this.render(hbs`{{workflow-files - submission=submission - previouslyUploadedFiles=files - newFiles=newFiles - next=(action loadNext) - back=(action loadPrevious)}}`); - assert.ok(true); + run(() => { + const component = this.owner.lookup('component:workflow-files'); + + // Crappily mock this function on the component so we don't have to mess with + // the 'document' object... + component.set('_getFilesElement', () => { + assert.ok(true); + return ({ + files: [ + { + size: 100, + name: 'Fake-file-name', + type: 'text/plain' + } + ] + }); + }); + component.set('newFiles', Ember.A()); + component.set('submission', Ember.Object.create()); + + component.send('getFiles'); + }); }); }); diff --git a/tests/unit/controllers/submissions/detail-test.js b/tests/unit/controllers/submissions/detail-test.js index f7c75b62c..399263baf 100644 --- a/tests/unit/controllers/submissions/detail-test.js +++ b/tests/unit/controllers/submissions/detail-test.js @@ -10,27 +10,27 @@ module('Unit | Controller | submissions/detail', (hooks) => { assert.ok(controller); }); - test('delete action should trigger destroy on model object', function (assert) { - assert.expect(2); + test('delete action should call the submission handler', function (assert) { + assert.expect(3); // Mock the global SweetAlert object to always return immediately swal = () => Promise.resolve({ value: 'moo' }); - const submission = { - get() { - return undefined; - }, - destroyRecord() { - assert.ok(true); - return Promise.resolve(); - } - }; + const submission = Ember.Object.create(); const controller = this.owner.lookup('controller:submissions/detail'); assert.ok(controller, 'controller not found'); + controller.set('submissionHandler', Ember.Object.create({ + deleteSubmission() { + assert.ok(true); + return Promise.resolve(); + } + })); + controller.set('transitionToRoute', () => assert.ok(true)); + controller.send('deleteSubmission', submission); }); }); diff --git a/tests/unit/controllers/submissions/new/basics-test.js b/tests/unit/controllers/submissions/new/basics-test.js index 7705696d4..2f95db15d 100644 --- a/tests/unit/controllers/submissions/new/basics-test.js +++ b/tests/unit/controllers/submissions/new/basics-test.js @@ -159,7 +159,7 @@ module('Unit | Controller | submissions/new/basics', (hooks) => { }); test('check validateAndLoadTab accepts complete information', function (assert) { - assert.expect(10); + assert.expect(11); let subSaved = false; @@ -179,6 +179,10 @@ module('Unit | Controller | submissions/new/basics', (hooks) => { title: 'Test publication title', journal: { id: 'journal:id' + }, + save() { + assert.ok(true); + return Promise.resolve(); } }); let model = { @@ -210,7 +214,7 @@ module('Unit | Controller | submissions/new/basics', (hooks) => { * action is sent to the controller. */ test('make sure submission is saved', function (assert) { - assert.expect(1); + assert.expect(2); const controller = this.owner.lookup('controller:submissions/new/basics'); const model = { @@ -218,7 +222,8 @@ module('Unit | Controller | submissions/new/basics', (hooks) => { title: 'This is the moo-iest', journal: Ember.Object.create({ id: 'journal:id' - }) + }), + save: () => Promise.resolve(assert.ok(true)) }), newSubmission: Ember.Object.create({ save: () => Promise.resolve(assert.ok(true)) diff --git a/tests/unit/routes/submissions/new-test.js b/tests/unit/routes/submissions/new-test.js index 61f2bbbbf..c4f5bdfa9 100644 --- a/tests/unit/routes/submissions/new-test.js +++ b/tests/unit/routes/submissions/new-test.js @@ -4,9 +4,97 @@ import { setupTest } from 'ember-qunit'; module('Unit | Route | submissions/new', (hooks) => { setupTest(hooks); - // Replace this with your real tests. - test('it exists', function (assert) { - let service = this.owner.lookup('route:submissions/new'); - assert.ok(service); + /* + * Ended up mocking the Store in each test (instead of beforeEach()) in order to put + * test assertions in store calls + */ + hooks.beforeEach(function () { + this.set('journal', Ember.Object.create({ journalName: 'International Moonthly' })); + this.set('publication', Ember.Object.create({ + title: 'Test Publication', + journal: this.get('journal') + })); + this.set('submission', Ember.Object.create({ + submissionStatus: 'draft', + publication: this.get('publication'), + metadata: '{ "moo": "This is a moo" }' + })); + }); + + test('fresh submission returned by model() when no ID is provided', async function (assert) { + assert.expect(4); + + const route = this.owner.lookup('route:submissions/new'); + + route.set('store', { + createRecord(type, data) { + switch (type) { + case 'publication': + assert.ok(true); + return Promise.resolve(Ember.Object.create({ title: 'MockMoo' })); + case 'submission': // Submission - fall through to default + assert.ok(true); + // eslint-disable-next-line no-fallthrough + default: + return Promise.resolve(Ember.Object.create(data)); + } + }, + findRecord: (type, id) => Promise.resolve(Ember.Object.create()), + query: () => Promise.resolve(Ember.A()) + }); + + const result = await route.model({}); + + assert.ok(result, 'no model found'); + assert.notOk(result.newSubmission.get('publication'), 'There should be no publication on this submission'); + }); + + /** + * Test case: submission ID is included in URL, grant ID NOT included in URL + * + * Expect that createRecord and findRecord are each called once and that + */ + test('The mock submission returned from model() when it\'s ID is included', async function (assert) { + assert.expect(6); + + const mockSub = this.get('submission'); + + const route = this.owner.lookup('route:submissions/new'); + + route.set('store', { + createRecord(type, data) { + switch (type) { + case 'publication': + assert.ok(false, 'should not create a publication'); + return Promise.resolve(Ember.Object.create({ title: 'MockMoo' })); + default: + assert.ok(false, `unexpected 'createRecord' type found: ${type}`); + return Promise.resolve(Ember.Object.create(data)); + } + }, + findRecord(type, id) { + switch (type) { + case 'submission': + assert.ok(true); + return Promise.resolve(mockSub); + default: + assert.ok(false, `unexpected 'createRecord' type found: ${type}`); + return Promise.resolve(Ember.Object.create()); + } + }, + query: () => Promise.resolve(Ember.A()) + }); + route.set('workflow', { + setDoiInfo(data) { + assert.ok(data); + assert.equal(data.moo, 'This is a moo'); + } + }); + + const result = await route.model({ submission: 'moo' }); + + assert.ok(result, 'no model found'); + assert.ok(result.newSubmission.get('publication'), 'There should be a publication on this submission'); + assert.equal(result.newSubmission.get('publication.title'), 'Test Publication'); }); }); diff --git a/tests/unit/services/submission-handler-test.js b/tests/unit/services/submission-handler-test.js index 526b35efd..2504eb74b 100644 --- a/tests/unit/services/submission-handler-test.js +++ b/tests/unit/services/submission-handler-test.js @@ -286,7 +286,12 @@ module('Unit | Service | submission-handler', (hooks) => { * Associated files should be queried from the Store */ test('delete submission', function (assert) { - assert.expect(4); + assert.expect(6); + + function destroyRecord() { + assert.ok(true); + return Promise.resolve(); + } const service = this.owner.lookup('service:submission-handler'); @@ -294,17 +299,20 @@ module('Unit | Service | submission-handler', (hooks) => { query(type, query) { assert.equal(type, 'file', 'Unexpected store query found'); assert.ok(query); - return Promise.resolve(Ember.A()); + return Promise.resolve(Ember.A([ + Ember.Object.create({ + destroyRecord + }) + ])); } })); const sub = Ember.Object.create({ - // publication: Ember.Object.create() - destroyRecord() { - assert.ok(true); - return Promise.resolve(); - } + destroyRecord }); + sub.set('publication', Ember.Object.create({ + destroyRecord + })); const result = service.deleteSubmission(sub); result.then(() => {