Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
47 changes: 26 additions & 21 deletions apps/central/src/components/form-draft/publish.vue
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

How about modifying focusInput() so that it focuses the <textarea> if the <input> isn't rendered? I.e., focusing whichever form control comes first.

Original file line number Diff line number Diff line change
Expand Up @@ -40,37 +40,27 @@ except according to the terms contained in the LICENSE file.

<hr v-if="draftVersionStringIsDuplicate">
<p v-if="draftVersionStringIsDuplicate">{{ $t('introduction[2]') }}</p>

<p v-if="!draftVersionStringIsDuplicate && !versionConflict">{{ $t('introduction[3]') }}</p>
</div>
<form v-if="draftVersionStringIsDuplicate || versionConflict" @submit.prevent="publish">
<form-group ref="versionString" v-model.trim="versionString"
:placeholder="$t('common.version')" required autocomplete="off"/>
<hr>
<form @submit.prevent="publish">
<form-group v-if="draftVersionStringIsDuplicate || versionConflict" ref="versionString" v-model.trim="versionString" :placeholder="$t('common.version')"
required autocomplete="off"/>
<div class="form-group">
<textarea id="form-draft-publish-note" ref="publishNote" v-model="notes" class="form-control"
:placeholder="$t('placeholder.note')" rows="3"></textarea>
<label for="form-draft-publish-note" class="form-label">{{ $t('field.note') }}</label>
</div>
<p>{{ $t('introduction[3]') }}</p>
<!-- We specify two nearly identical .modal-actions, because here we
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Since notes will always be displayed in the publish modal, I have removed the duplicate modal action buttons.

want the Proceed button to be a submit button (which means that browsers
will do some basic form validation when it is clicked). -->
<div class="modal-actions">
<button type="button" class="btn btn-link"
:aria-disabled="awaitingResponse" @click="$emit('hide')">
{{ $t('action.cancel') }}
</button>
<button type="submit" class="btn btn-primary"
:aria-disabled="awaitingResponse">
<button type="submit" class="btn btn-primary" :aria-disabled="awaitingResponse">
{{ $t('action.proceed') }} <spinner :state="awaitingResponse"/>
</button>
</div>
</form>
<div v-else class="modal-actions">
<button type="button" class="btn btn-link" :aria-disabled="awaitingResponse"
@click="$emit('hide')">
{{ $t('action.cancel') }}
</button>
<button type="button" class="btn btn-primary"
:aria-disabled="awaitingResponse" @click="publish">
{{ $t('action.proceed') }} <spinner :state="awaitingResponse"/>
</button>
</div>
</template>
</modal>
</template>
Expand Down Expand Up @@ -111,6 +101,7 @@ export default {
data() {
return {
versionString: '',
notes: '',
// versionConflict is used in a scenario where a user tries to
// publish a form that conflicts with a form/version combo probably
// found in the trash. This component doesn't have access to trashed
Expand Down Expand Up @@ -139,14 +130,21 @@ export default {
},
watch: {
state(state) {
if (state) this.versionString = this.formDraft.version;
if (state) {
this.versionString = this.formDraft.version;
this.notes = '';
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could you add a test or modify an existing test to assert that the notes field is reset? There's a related test in form-draft/publish.spec.js for it('resets the input if the modal is hidden'

}
}
},
methods: {
focusInput() {
if (this.draftVersionStringIsDuplicate) this.$refs.versionString.focus();
else this.$refs.publishNote.focus();
},
publish() {
const headers = {};
if (this.notes !== '')
headers['X-Action-Notes'] = encodeURIComponent(this.notes);
this.request({
method: 'POST',
url: apiPaths.publishFormDraft(
Expand All @@ -156,6 +154,7 @@ export default {
? { version: this.versionString }
: null
),
headers,
fulfillProblem: (problem) => problem.code === 409.6
})
.then(({ data }) => {
Expand Down Expand Up @@ -198,6 +197,12 @@ export default {
"newProperties": "Publishing this draft will create {count} property. | Publishing this draft will create {count} properties.",
"problem": {
"409_6": "The version name of this Draft conflicts with a past version of this Form or a deleted Form. Please use the field below to change it to something new or upload a new Form definition."
},
"field": {
"note": "Notes"
},
"placeholder": {
"note": "Add optional publishing notes…"
}
}
}
Expand Down
4 changes: 2 additions & 2 deletions apps/central/test/components/form-attachment/list.spec.js
Original file line number Diff line number Diff line change
Expand Up @@ -1140,7 +1140,7 @@ describe('FormAttachmentList', () => {
.complete()
.request(async (app) => {
await app.get('#form-edit-publish-button').trigger('click');
return app.get('#form-draft-publish .btn-primary').trigger('click');
return app.get('#form-draft-publish form').trigger('submit');
})
.respondWithData(() => {
testData.extendedFormDrafts.publish(-1);
Expand Down Expand Up @@ -1184,7 +1184,7 @@ describe('FormAttachmentList', () => {
.complete()
.request(async (app) => {
await app.get('#form-edit-publish-button').trigger('click');
return app.get('#form-draft-publish .btn-primary').trigger('click');
return app.get('#form-draft-publish form').trigger('submit');
})
.respondWithData(() => {
testData.extendedFormDrafts.publish(-1);
Expand Down
80 changes: 56 additions & 24 deletions apps/central/test/components/form-draft/publish.spec.js
Original file line number Diff line number Diff line change
Expand Up @@ -109,7 +109,7 @@ describe('FormDraftPublish', () => {
modal.findAll('form p').length.should.equal(1);
});

it('does not show input if version string of draft is different', async () => {
it('does not show version input if version string of draft is different', async () => {
testData.extendedForms.createPast(1);
testData.extendedFormVersions.createPast(1, {
version: 'v2',
Expand All @@ -118,15 +118,17 @@ describe('FormDraftPublish', () => {
const modal = mount(FormDraftPublish, mountOptions());
await modal.setProps({ state: true });
modal.find('input').exists().should.be.false;
modal.findAll('.modal-introduction p').length.should.equal(3);
modal.findAll('.modal-introduction p').length.should.equal(2);
modal.findAll('form p').length.should.equal(1);
});

it('does not show the input for a form without a published version', async () => {
it('does not show the version input for a form without a published version', async () => {
testData.extendedForms.createPast(1, { draft: true });
const modal = mount(FormDraftPublish, mountOptions());
await modal.setProps({ state: true });
modal.find('input').exists().should.be.false;
modal.findAll('.modal-introduction p').length.should.equal(3);
modal.findAll('.modal-introduction p').length.should.equal(2);
modal.findAll('form p').length.should.equal(1);
});

it('focuses the input', async () => {
Expand Down Expand Up @@ -164,6 +166,30 @@ describe('FormDraftPublish', () => {
});
});

describe('publish notes', () => {
it('focuses the textarea if version input is not shown', async () => {
testData.extendedForms.createPast(1);
testData.extendedFormVersions.createPast(1, { version: 'v2', draft: true });
const modal = mount(FormDraftPublish, mountOptions({
attachTo: document.body
}));
await modal.setProps({ state: true });
modal.get('textarea').should.be.focused();
});

it('resets the input if the modal is hidden', async () => {
testData.extendedForms.createPast(1);
testData.extendedFormVersions.createPast(1, { version: 'v2', draft: true });
const modal = mount(FormDraftPublish, mountOptions());
await modal.setProps({ state: true });
const input = modal.get('textarea');
await input.setValue('fixed validation error');
await modal.setProps({ state: false });
await modal.setProps({ state: true });
input.element.value.should.equal('');
});
});

describe('standard button things', () => {
it('implements things if the version string input is shown', () => {
testData.extendedForms.createPast(1);
Expand All @@ -178,18 +204,6 @@ describe('FormDraftPublish', () => {
modal: true
});
});

it('implements things if the version string input is not shown', () => {
testData.extendedForms.createPast(1, { draft: true });
return mockHttp()
.mount(FormDraftPublish, mountOptions())
.afterResponses(modal => modal.setProps({ state: true }))
.testStandardButton({
button: '.btn-primary',
disabled: ['.btn-link'],
modal: true
});
});
});

describe('request', () => {
Expand All @@ -199,7 +213,7 @@ describe('FormDraftPublish', () => {
.mount(FormDraftPublish, mountOptions())
.request(async (modal) => {
await modal.setProps({ state: true });
return modal.get('.btn-primary').trigger('click');
return modal.get('form').trigger('submit');
})
.beforeEachResponse((_, { method, url }) => {
method.should.equal('POST');
Expand Down Expand Up @@ -249,7 +263,7 @@ describe('FormDraftPublish', () => {
.mount(FormDraftPublish, mountOptions())
.request(async (modal) => {
await modal.setProps({ state: true });
return modal.get('.btn-primary').trigger('click');
return modal.get('form').trigger('submit');
})
.beforeEachResponse((_, { url }) => {
url.should.equal('/v1/projects/1/forms/f/draft/publish');
Expand Down Expand Up @@ -283,7 +297,7 @@ describe('FormDraftPublish', () => {
.mount(FormDraftPublish, mountOptions())
.request(async (modal) => {
await modal.setProps({ state: true });
return modal.get('#form-draft-publish .btn-primary').trigger('click');
return modal.get('#form-draft-publish form').trigger('submit');
})
.respondWithProblem({
code: 409.17,
Expand All @@ -309,7 +323,7 @@ describe('FormDraftPublish', () => {
.mount(FormDraftPublish, mountOptions())
.request(async (modal) => {
await modal.setProps({ state: true });
return modal.get('#form-draft-publish .btn-primary').trigger('click');
return modal.get('form').trigger('submit');
})
.respondWithProblem(409.6)
.afterResponse(modal => {
Expand All @@ -329,13 +343,14 @@ describe('FormDraftPublish', () => {
.mount(FormDraftPublish, mountOptions())
.request(async (modal) => {
await modal.setProps({ state: true });
return modal.get('#form-draft-publish .btn-primary').trigger('click');
return modal.get('form').trigger('submit');
})
.respondWithProblem(500.1)
.afterResponse(modal => {
modal.should.alert('danger');
modal.find('input').exists().should.be.false;
modal.findAll('.modal-introduction p').length.should.equal(3);
modal.findAll('.modal-introduction p').length.should.equal(2);
modal.findAll('form p').length.should.equal(1);
});
});

Expand All @@ -355,7 +370,7 @@ describe('FormDraftPublish', () => {
.complete()
.request(async (app) => {
await app.get('#form-edit-publish-button').trigger('click');
return app.get('#form-draft-publish .btn-primary').trigger('click');
return app.get('#form-draft-publish form').trigger('submit');
})
.modify(respondToPublish);
};
Expand All @@ -369,6 +384,23 @@ describe('FormDraftPublish', () => {
{ url: '/v1/projects/1/forms/f/dataset-diff' }
]));

it('sends X-Action-Notes header when notes are provided', () => {
testData.extendedForms.createPast(1, { draft: true });
return load('/projects/1/forms/f/draft')
.complete()
.request(async (app) => {
await app.get('#form-edit-publish-button').trigger('click');
await app.get('#form-draft-publish-note').setValue('Version 1 release notes');
return app.get('#form-draft-publish form').trigger('submit');
})
.beforeEachResponse((_, { url, headers }) => {
if (url === '/v1/projects/1/forms/f/draft/publish') {
headers['X-Action-Notes'].should.equal(encodeURIComponent('Version 1 release notes'));
}
})
.modify(respondToPublish);
});

it('hides the modal', () =>
publish()
.afterResponses(app => {
Expand Down Expand Up @@ -422,7 +454,7 @@ describe('FormDraftPublish', () => {
.complete()
.request(async (app) => {
await app.get('#form-edit-publish-button').trigger('click');
return app.get('#form-draft-publish .btn-primary').trigger('click');
return app.get('#form-draft-publish form').trigger('submit');
})
.modify(respondToPublish)
.complete()
Expand Down
2 changes: 1 addition & 1 deletion apps/central/test/components/form/edit.spec.js
Original file line number Diff line number Diff line change
Expand Up @@ -55,7 +55,7 @@ describe('FormEdit', () => {
})
.request(async (app) => {
await app.get('#form-edit-publish-button').trigger('click');
return app.get('#form-draft-publish .btn-primary').trigger('click');
return app.get('#form-draft-publish form').trigger('submit');
})
.respondWithData(() => {
testData.extendedFormDrafts.publish(-1);
Expand Down
11 changes: 11 additions & 0 deletions apps/central/transifex/strings_en.json
Original file line number Diff line number Diff line change
Expand Up @@ -3324,6 +3324,17 @@
"409_6": {
"string": "The version name of this Draft conflicts with a past version of this Form or a deleted Form. Please use the field below to change it to something new or upload a new Form definition."
}
},
"field": {
"note": {
"string": "Notes",
"developer_comment": "This is the text of a form field."
}
},
"placeholder": {
"note": {
"string": "Add optional publishing notes…"
}
}
},
"FormDraftQrPanel": {
Expand Down