From 86c75aed72d24c372b720a43228416876898d75b Mon Sep 17 00:00:00 2001 From: Kevin Ansfield Date: Mon, 30 Apr 2018 12:29:43 +0100 Subject: [PATCH] =?UTF-8?q?=F0=9F=90=9B=20Fixed=20admin=20URL=20not=20upda?= =?UTF-8?q?ting=20when=20changing=20tag=20slug?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit closes https://github.com/TryGhost/Ghost/issues/9571 - uses `window.replaceState` to update the URL when a tag is saved with a new URL - ensures back button doesn't result in 404 - use `windowProxy` util so that behaviour can be tested --- .../app/controllers/settings/tags/tag.js | 13 ++++++++ ghost/admin/app/controllers/team/user.js | 7 ++-- .../tests/acceptance/settings/tags-test.js | 32 +++++++++++++++++++ 3 files changed, 47 insertions(+), 5 deletions(-) diff --git a/ghost/admin/app/controllers/settings/tags/tag.js b/ghost/admin/app/controllers/settings/tags/tag.js index 1481ac5761..1ec6638c43 100644 --- a/ghost/admin/app/controllers/settings/tags/tag.js +++ b/ghost/admin/app/controllers/settings/tags/tag.js @@ -1,4 +1,5 @@ import Controller, {inject as controller} from '@ember/controller'; +import windowProxy from 'ghost-admin/utils/window-proxy'; import {alias} from '@ember/object/computed'; import {inject as service} from '@ember/service'; @@ -28,6 +29,7 @@ export default Controller.extend({ _saveTagProperty(propKey, newValue) { let tag = this.get('tag'); + let isNewTag = tag.get('isNew'); let currentValue = tag.get(propKey); if (newValue) { @@ -46,6 +48,17 @@ export default Controller.extend({ tag.save().then((savedTag) => { // replace 'new' route with 'tag' route this.replaceRoute('settings.tags.tag', savedTag); + + // update the URL if the slug changed + if (propKey === 'slug' && !isNewTag) { + let currentPath = window.location.hash; + + let newPath = currentPath.split('/'); + newPath[newPath.length - 1] = savedTag.get('slug'); + newPath = newPath.join('/'); + + windowProxy.replaceState({path: newPath}, '', newPath); + } }).catch((error) => { if (error) { this.get('notifications').showAPIError(error, {key: 'tag.save'}); diff --git a/ghost/admin/app/controllers/team/user.js b/ghost/admin/app/controllers/team/user.js index f0dddf0060..e8a8b20237 100644 --- a/ghost/admin/app/controllers/team/user.js +++ b/ghost/admin/app/controllers/team/user.js @@ -435,17 +435,14 @@ export default Controller.extend({ } try { - let currentPath, - newPath; - user = yield user.save({format: false}); // If the user's slug has changed, change the URL and replace // the history so refresh and back button still work if (slugChanged) { - currentPath = window.location.hash; + let currentPath = window.location.hash; - newPath = currentPath.split('/'); + let newPath = currentPath.split('/'); newPath[newPath.length - 1] = user.get('slug'); newPath = newPath.join('/'); diff --git a/ghost/admin/tests/acceptance/settings/tags-test.js b/ghost/admin/tests/acceptance/settings/tags-test.js index 4d49fa04ed..8bbab9eaa5 100644 --- a/ghost/admin/tests/acceptance/settings/tags-test.js +++ b/ghost/admin/tests/acceptance/settings/tags-test.js @@ -3,6 +3,7 @@ import $ from 'jquery'; import destroyApp from '../../helpers/destroy-app'; import startApp from '../../helpers/start-app'; import wait from 'ember-test-helpers/wait'; +import windowProxy from 'ghost-admin/utils/window-proxy'; import {Response} from 'ember-cli-mirage'; import {afterEach, beforeEach, describe, it} from 'mocha'; import {authenticateSession, invalidateSession} from 'ghost-admin/tests/helpers/ember-simple-auth'; @@ -77,13 +78,25 @@ describe('Acceptance: Settings - Tags', function () { }); describe('when logged in', function () { + let newLocation, originalReplaceState; + beforeEach(function () { let role = server.create('role', {name: 'Administrator'}); server.create('user', {roles: [role]}); + originalReplaceState = windowProxy.replaceState; + windowProxy.replaceState = function (params, title, url) { + newLocation = url; + }; + newLocation = undefined; + return authenticateSession(application); }); + afterEach(function () { + windowProxy.replaceState = originalReplaceState; + }); + it('it renders, can be navigated, can edit, create & delete tags', async function () { let tag1 = server.create('tag'); let tag2 = server.create('tag'); @@ -267,6 +280,25 @@ describe('Acceptance: Settings - Tags', function () { .to.equal('internal'); }); + it('updates the URL when slug changes', async function () { + server.createList('tag', 2); + + await visit('/settings/tags/tag-1'); + + // second wait is needed for the vertical-collection to settle + await wait(); + + expect(currentURL(), 'URL after direct load').to.equal('/settings/tags/tag-1'); + + // update the slug + await fillIn('.tag-settings-pane input[name="slug"]', 'test'); + await triggerEvent('.tag-settings-pane input[name="slug"]', 'blur'); + + // 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'); + }); + it('redirects to 404 when tag does not exist', async function () { server.get('/tags/slug/unknown/', function () { return new Response(404, {'Content-Type': 'application/json'}, {errors: [{message: 'Tag not found.', errorType: 'NotFoundError'}]});