🐛 Fixed error when deleting tag and missing slugs on tags list

no issue

- fixes error that left the confirmation modal in place when deleting a tag by ensuring we return `true` in the task used by the confirm button, if we return the transition object it trips the "failed" state because the `/tags` route aborts and refreshes when transitioning to it
- fixes missing attached posts count in the tag delete confirmation modal by using the correct `tag.count.posts` attribute in the conditional
- fixes missing slugs in the tags list by using the properties on `@tag` rather than expecting a separate `@slug` argument
- replaced the skipped tags acceptance tests with an updated tests that match the recent redesign
This commit is contained in:
Kevin Ansfield 2022-09-07 17:51:46 +01:00
parent 74ecde73db
commit aa53a1c71f
8 changed files with 84 additions and 233 deletions

View File

@ -12,6 +12,7 @@
@value={{this.scratchTag.name}}
@tabindex="1"
@focus-out={{action "setProperty" "name" this.scratchTag.name}}
data-test-input="tag-name"
/>
<span class="error">
<GhErrorMessage @errors={{this.tag.errors}} @property="name" />
@ -63,6 +64,7 @@
@name="slug"
@tabindex="2"
@focus-out={{action "setProperty" "slug" this.scratchTag.slug}}
data-test-input="tag-slug"
/>
<GhUrlPreview @prefix="tag" @slug={{this.scratchTag.slug}} @tagName="p" @classNames="description" />
<GhErrorMessage @errors={{this.activeTag.errors}} @property="slug" />

View File

@ -1,17 +1,17 @@
<li class="gh-list-row gh-tags-list-item" ...attributes>
<LinkTo @route="tag" @model={{@tag}} class="gh-list-data gh-tag-list-title gh-list-cellwidth-70" title="Edit tag">
<h3 class="gh-tag-list-name">
<h3 class="gh-tag-list-name" data-test-tag-name>
{{@tag.name}}
</h3>
{{#if @tag.description}}
<p class="ma0 pa0 f8 midgrey gh-tag-list-description">
<p class="ma0 pa0 f8 midgrey gh-tag-list-description" data-test-tag-description>
{{@tag.description}}
</p>
{{/if}}
</LinkTo>
<LinkTo @route="tag" @model={{@tag}} class="gh-list-data middarkgrey f8 gh-tag-list-slug gh-list-cellwidth-10" title="Edit tag">
<span title="{{@slug}}">{{@slug}}</span>
<LinkTo @route="tag" @model={{@tag}} class="gh-list-data middarkgrey f8 gh-tag-list-slug gh-list-cellwidth-10" title="Edit tag" data-test-tag-slug>
<span title="{{@tag.slug}}">{{@tag.slug}}</span>
</LinkTo>
{{#if @tag.count.posts}}

View File

@ -4,13 +4,13 @@
<a class="close" href="" role="button" title="Close" {{action "closeModal"}}>{{svg-jar "close"}}<span class="hidden">Close</span></a>
<div class="modal-body">
{{#if this.tag.post_count}}
<span class="red">This tag is attached to {{this.tag.count.posts}} {{this.postInflection}}.</span>
{{#if this.tag.count.posts}}
<span class="red">This tag is attached to <span data-test-text="posts-count">{{this.tag.count.posts}} {{this.postInflection}}</span>.</span>
{{/if}}
You're about to delete "<strong>{{this.tag.name}}</strong>". This is permanent! We warned you, k?
</div>
<div class="modal-footer">
<button class="gh-btn" type="button" {{action "closeModal"}}><span>Cancel</span></button>
<GhTaskButton @buttonText="Delete" @successText="Deleted" @task={{this.deleteTag}} @class="gh-btn gh-btn-red gh-btn-icon" />
<button class="gh-btn" type="button" {{action "closeModal"}} data-test-button="cancel"><span>Cancel</span></button>
<GhTaskButton @buttonText="Delete" @successText="Deleted" @task={{this.deleteTag}} @class="gh-btn gh-btn-red gh-btn-icon" data-test-button="confirm" />
</div>

View File

@ -4,6 +4,9 @@ import {computed} from '@ember/object';
import {task} from 'ember-concurrency';
export default ModalComponent.extend({
attributeBindings: ['dataTestModal:data-test-modal'],
dataTestModal: 'confirm-delete-tag',
// Allowed actions
confirm: () => {},

View File

@ -45,7 +45,8 @@ export default class TagController extends Controller {
deleteTag() {
return this.tag.destroyRecord().then(() => {
this.set('showDeleteTagModal', false);
return this.transitionToRoute('tags');
this.router.transitionTo('tags');
return true;
}, (error) => {
return this.notifications.showAPIError(error, {key: 'tag.delete'});
});

View File

@ -19,7 +19,7 @@
{{#unless this.tag.isNew}}
<div>
<button type="button" class="gh-btn gh-btn-red gh-btn-icon" {{on "click" (action "openDeleteTagModal")}}>
<button type="button" class="gh-btn gh-btn-red gh-btn-icon" {{on "click" (action "openDeleteTagModal")}} data-test-button="delete-tag">
<span>Delete tag</span>
</button>
</div>

View File

@ -3,8 +3,8 @@
<h2 class="gh-canvas-title" data-test-screen-title>Tags</h2>
<section class="view-actions">
<div class="gh-contentfilter gh-btn-group">
<button class="gh-btn {{if (eq this.type "public") "gh-btn-group-selected"}}" type="button" {{action "changeType" "public"}}><span>Public tags</span></button>
<button class="gh-btn {{if (eq this.type "internal") "gh-btn-group-selected"}}" type="button" {{action "changeType" "internal"}}><span>Internal tags</span></button>
<button class="gh-btn {{if (eq this.type "public") "gh-btn-group-selected"}}" type="button" {{action "changeType" "public"}} data-test-tags-nav="public" data-test-active={{eq this.type "public"}}><span>Public tags</span></button>
<button class="gh-btn {{if (eq this.type "internal") "gh-btn-group-selected"}}" type="button" {{action "changeType" "internal"}} data-test-tags-nav="internal" data-test-active={{eq this.type "internal"}}><span>Internal tags</span></button>
</div>
<LinkTo @route="tag.new" class="gh-btn gh-btn-primary"><span>New tag</span></LinkTo>
</section>
@ -20,7 +20,7 @@
<div class="gh-list-header gh-list-cellwidth-10"></div>
</li>
<VerticalCollection @items={{this.sortedTags}} @key="id" @containerSelector=".gh-main" @estimateHeight={{60}} @bufferSize={{20}} as |tag|>
<GhTagsListItem @tag={{tag}} data-test-tag-id={{tag.id}} />
<GhTagsListItem @tag={{tag}} data-test-tag={{tag.id}} />
</VerticalCollection>
{{else}}
<li class="no-posts-box">

View File

@ -10,34 +10,7 @@ import {setupMirage} from 'ember-cli-mirage/test-support';
import {timeout} from 'ember-concurrency';
import {visit} from '../../helpers/visit';
// Grabbed from keymaster's testing code because Ember's `keyEvent` helper
// is for some reason not triggering the events in a way that keymaster detects:
// https://github.com/madrobby/keymaster/blob/master/test/keymaster.html#L31
const modifierMap = {
16: 'shiftKey',
18: 'altKey',
17: 'ctrlKey',
91: 'metaKey'
};
let keydown = function (code, modifiers, el) {
let event = document.createEvent('Event');
event.initEvent('keydown', true, true);
event.keyCode = code;
if (modifiers && modifiers.length > 0) {
for (let i in modifiers) {
event[modifierMap[modifiers[i]]] = true;
}
}
(el || document).dispatchEvent(event);
};
let keyup = function (code, el) {
let event = document.createEvent('Event');
event.initEvent('keyup', true, true);
event.keyCode = code;
(el || document).dispatchEvent(event);
};
describe.skip('Acceptance: Tags', function () {
describe('Acceptance: Tags', function () {
let hooks = setupApplicationTest();
setupMirage(hooks);
@ -88,17 +61,18 @@ describe.skip('Acceptance: Tags', function () {
windowProxy.replaceState = originalReplaceState;
});
it('it renders, can be navigated, can edit, create & delete tags', async function () {
let tag1 = this.server.create('tag');
let tag2 = this.server.create('tag');
it('lists public and internal tags separately', async function () {
this.server.create('tag', {name: 'B - Third', slug: 'third'});
this.server.create('tag', {name: 'Z - Last', slug: 'last'});
this.server.create('tag', {name: '!A - Second', slug: 'second'});
this.server.create('tag', {name: 'A - First', slug: 'first'});
this.server.create('tag', {name: '#one', slug: 'hash-one', visibility: 'internal'});
this.server.create('tag', {name: '#two', slug: 'hash-two', visibility: 'internal'});
await visit('/tags');
await visit('tags');
// it redirects to first tag
// expect(currentURL(), 'currentURL').to.equal(`/tags/${tag1.slug}`);
// it doesn't redirect to first tag
expect(currentURL(), 'currentURL').to.equal('/tags');
// it loads tags list
expect(currentURL(), 'currentURL').to.equal('tags');
// it has correct page title
expect(document.title, 'page title').to.equal('Tags - Test Blog');
@ -107,197 +81,84 @@ describe.skip('Acceptance: Tags', function () {
expect(find('[data-test-nav="tags"]'), 'highlights nav menu item')
.to.have.class('active');
// it lists all tags
expect(findAll('.tags-list .gh-tags-list-item').length, 'tag list count')
.to.equal(2);
let tag = find('.tags-list .gh-tags-list-item');
expect(tag.querySelector('.gh-tag-list-name').textContent, 'tag list item title')
.to.equal(tag1.name);
// it defaults to public tags
expect(find('[data-test-tags-nav="public"]')).to.have.attr('data-test-active');
expect(find('[data-test-tags-nav="internal"]')).to.not.have.attr('data-test-active');
// it highlights selected tag
// expect(find(`a[href="/ghost/tags/${tag1.slug}"]`), 'highlights selected tag')
// .to.have.class('active');
// it lists all public tags
expect(findAll('[data-test-tag]'), 'public tag list count')
.to.have.length(4);
await visit(`/tags/${tag1.slug}`);
// tags are in correct order
let tags = findAll('[data-test-tag]');
// second wait is needed for the tag details to settle
expect(tags[0].querySelector('[data-test-tag-name]')).to.have.trimmed.text('A - First');
expect(tags[1].querySelector('[data-test-tag-name]')).to.have.trimmed.text('!A - Second');
expect(tags[2].querySelector('[data-test-tag-name]')).to.have.trimmed.text('B - Third');
expect(tags[3].querySelector('[data-test-tag-name]')).to.have.trimmed.text('Z - Last');
// it shows selected tag form
// expect(find('.tag-settings-pane h4').textContent, 'settings pane title')
// .to.equal('Tag settings');
expect(find('.gh-tag-basic-settings-form input[name="name"]').value, 'loads correct tag into form')
.to.equal(tag1.name);
// can switch to internal tags
await click('[data-test-tags-nav="internal"]');
// click the second tag in the list
// let tagEditButtons = findAll('.tag-edit-button');
// await click(tagEditButtons[tagEditButtons.length - 1]);
// it navigates to selected tag
// expect(currentURL(), 'url after clicking tag').to.equal(`/tags/${tag2.slug}`);
// it highlights selected tag
// expect(find(`a[href="/ghost/tags/${tag2.slug}"]`), 'highlights selected tag')
// .to.have.class('active');
// it shows selected tag form
// expect(find('.tag-settings-pane input[name="name"]').value, 'loads correct tag into form')
// .to.equal(tag2.name);
// simulate up arrow press
run(() => {
keydown(38);
keyup(38);
});
await settled();
// it navigates to previous tag
expect(currentURL(), 'url after keyboard up arrow').to.equal(`/tags/${tag1.slug}`);
// it highlights selected tag
// expect(find(`a[href="/ghost/tags/${tag1.slug}"]`), 'selects previous tag')
// .to.have.class('active');
// simulate down arrow press
run(() => {
keydown(40);
keyup(40);
});
await settled();
// it navigates to previous tag
expect(currentURL(), 'url after keyboard down arrow').to.equal(`/tags/${tag2.slug}`);
// it highlights selected tag
// expect(find(`a[href="/ghost/tags/${tag2.slug}"]`), 'selects next tag')
// .to.have.class('active');
// trigger save
await fillIn('.tag-settings-pane input[name="name"]', 'New Name');
await blur('.tag-settings-pane input[name="name"]');
// extra timeout needed for Travis - sometimes it doesn't update
// quick enough and an extra wait() call doesn't help
await timeout(100);
// check we update with the data returned from the server
let tags = findAll('.settings-tags .settings-tag');
tag = tags[0];
expect(tag.querySelector('.tag-title').textContent, 'tag list updates on save')
.to.equal('New Name');
expect(find('.tag-settings-pane input[name="name"]').value, 'settings form updates on save')
.to.equal('New Name');
// start new tag
await click('.view-actions .gh-btn-green');
// it navigates to the new tag route
expect(currentURL(), 'new tag URL').to.equal('/tags/new');
// it displays the new tag form
expect(find('.tag-settings-pane h4').textContent, 'settings pane title')
.to.equal('New tag');
// all fields start blank
findAll('.tag-settings-pane input, .tag-settings-pane textarea').forEach(function (elem) {
expect(elem.value, `input field for ${elem.getAttribute('name')}`)
.to.be.empty;
});
// save new tag
await fillIn('.tag-settings-pane input[name="name"]', 'New tag');
await blur('.tag-settings-pane input[name="name"]');
// extra timeout needed for FF on Linux - sometimes it doesn't update
// quick enough, especially on Travis, and an extra wait() call
// doesn't help
await timeout(100);
// it redirects to the new tag's URL
expect(currentURL(), 'URL after tag creation').to.equal('/tags/new-tag');
// it adds the tag to the list and selects
tags = findAll('.settings-tags .settings-tag');
tag = tags[1]; // second tag in list due to alphabetical ordering
expect(tags.length, 'tag list count after creation')
.to.equal(3);
// new tag will be second in the list due to alphabetical sorting
expect(findAll('.settings-tags .settings-tag')[1].querySelector('.tag-title').textContent.trim(), 'new tag list item title');
expect(tag.querySelector('.tag-title').textContent, 'new tag list item title')
.to.equal('New tag');
expect(find('a[href="/ghost/tags/new-tag"]'), 'highlights new tag')
.to.have.class('active');
// delete tag
await click('.settings-menu-delete-button');
await click('.fullscreen-modal .gh-btn-red');
// it redirects to the first tag
expect(currentURL(), 'URL after tag deletion').to.equal(`/tags/${tag1.slug}`);
// it removes the tag from the list
expect(findAll('.settings-tags .settings-tag').length, 'tag list count after deletion')
.to.equal(2);
expect(findAll('[data-test-tag]'), 'internal tag list count').to.have.length(2);
});
// TODO: Unskip and fix
// skipped because it was failing most of the time on Travis
// see https://github.com/TryGhost/Ghost/issues/8805
it.skip('loads tag via slug when accessed directly', async function () {
this.server.createList('tag', 2);
it('can edit tags', async function () {
const tag = this.server.create('tag', {name: 'To be edited', slug: 'to-be-edited'});
await visit('/tags/tag-1');
await visit('tags');
await click(`[data-test-tag="${tag.id}"] [data-test-tag-name]`);
expect(currentURL(), 'URL after direct load').to.equal('/tags/tag-1');
expect(currentURL()).to.equal('/tags/to-be-edited');
// it loads all other tags
expect(findAll('.settings-tags .settings-tag').length, 'tag list count after direct load')
.to.equal(2);
expect(find('[data-test-input="tag-name"]')).to.have.value('To be edited');
expect(find('[data-test-input="tag-slug"]')).to.have.value('to-be-edited');
// selects tag in list
expect(find('a[href="/ghost/tags/tag-1"]').classList.contains('active'), 'highlights requested tag')
.to.be.true;
await fillIn('[data-test-input="tag-name"]', 'New tag name');
await fillIn('[data-test-input="tag-slug"]', 'new-tag-slug');
await click('[data-test-button="save"]');
// shows requested tag in settings pane
expect(find('.tag-settings-pane input[name="name"]').value, 'loads correct tag into form')
.to.equal('Tag 1');
const savedTag = this.server.db.tags.find(tag.id);
expect(savedTag.name, 'saved tag name').to.equal('New tag name');
expect(savedTag.slug, 'saved tag slug').to.equal('new-tag-slug');
await click('[data-test-link="tags-back"]');
const tagListItem = find('[data-test-tag]');
expect(tagListItem.querySelector('[data-test-tag-name]')).to.have.trimmed.text('New tag name');
expect(tagListItem.querySelector('[data-test-tag-slug]')).to.have.trimmed.text('new-tag-slug');
});
it('shows the internal tag label', async function () {
this.server.create('tag', {name: '#internal-tag', slug: 'hash-internal-tag', visibility: 'internal'});
it('can delete tags', async function () {
const tag = this.server.create('tag', {name: 'To be edited', slug: 'to-be-edited'});
this.server.create('post', {tags: [tag]});
await visit('tags/');
await visit('tags');
await click(`[data-test-tag="${tag.id}"] [data-test-tag-name]`);
expect(currentURL()).to.equal('/tags/hash-internal-tag');
await click('[data-test-button="delete-tag"]');
expect(findAll('.settings-tags .settings-tag').length, 'tag list count')
.to.equal(1);
const tagModal = '[data-test-modal="confirm-delete-tag"]';
let tag = find('.settings-tags .settings-tag');
expect(find(tagModal)).to.exist;
expect(find(`${tagModal} [data-test-text="posts-count"]`))
.to.have.trimmed.text('1 post');
expect(tag.querySelectorAll('.label.label-blue').length, 'internal tag label')
.to.equal(1);
await click(`${tagModal} [data-test-button="confirm"]`);
expect(tag.querySelector('.label.label-blue').textContent.trim(), 'internal tag label text')
.to.equal('internal');
expect(find(tagModal)).to.not.exist;
expect(currentURL()).to.equal('/tags');
expect(findAll('[data-test-tag]')).to.have.length(0);
});
it('updates the URL when slug changes', async function () {
this.server.createList('tag', 2);
it('can load tag via slug in url', async function () {
const tag = this.server.create('tag', {name: 'To be edited', slug: 'to-be-edited'});
await visit('/tags/tag-1');
await visit('tags/to-be-edited');
expect(currentURL()).to.equal('tags/to-be-edited');
expect(currentURL(), 'URL after direct load').to.equal('/tags/tag-1');
// update the slug
await fillIn('.tag-settings-pane input[name="slug"]', 'test');
await blur('.tag-settings-pane input[name="slug"]');
// tests don't have a location.hash so we can only check that the
// slug portion is updated correctly
expect(newLocation, 'URL after slug change').to.equal('test');
expect(find('[data-test-input="tag-name"]')).to.have.value('To be edited');
expect(find('[data-test-input="tag-slug"]')).to.have.value('to-be-edited');
});
it('redirects to 404 when tag does not exist', async function () {
@ -310,21 +171,5 @@ describe.skip('Acceptance: Tags', function () {
expect(currentRouteName()).to.equal('error404');
expect(currentURL()).to.equal('/tags/unknown');
});
it('sorts tags correctly', async function () {
this.server.create('tag', {name: 'B - Third', slug: 'third'});
this.server.create('tag', {name: 'Z - Last', slug: 'last'});
this.server.create('tag', {name: '#A - Second', slug: 'second'});
this.server.create('tag', {name: 'A - First', slug: 'first'});
await visit('tags');
let tags = findAll('[data-test-tag]');
expect(tags[0].querySelector('[data-test-name]').textContent.trim()).to.equal('A - First');
expect(tags[1].querySelector('[data-test-name]').textContent.trim()).to.equal('#A - Second');
expect(tags[2].querySelector('[data-test-name]').textContent.trim()).to.equal('B - Third');
expect(tags[3].querySelector('[data-test-name]').textContent.trim()).to.equal('Z - Last');
});
});
});