From 92123e427ff5098be64be516a6d2c73b618f1ee7 Mon Sep 17 00:00:00 2001 From: Kevin Ansfield Date: Fri, 23 Oct 2015 18:18:39 +0100 Subject: [PATCH] Use tag slugs in URLs for tag management and add front-end edit redirect refs #5845 - adds custom adapter for tags so that `store.queryRecord('tag', {slug: 'tag-slug'})` hits the `/tags/slug/tag-slug` endpoint instead of `/tags/?slug=tag-slug` - updates tag management screens to use tag slugs instead of IDs - adds `/tag/:slug/edit` redirect to front-end --- core/client/app/adapters/tag.js | 17 +++++ core/client/app/router.js | 2 +- core/client/app/routes/settings/tags/tag.js | 6 +- core/client/app/templates/settings/tags.hbs | 2 +- .../tests/acceptance/settings/tags-test.js | 66 +++++++++++++++---- core/server/routes/frontend.js | 3 + core/test/functional/routes/frontend_spec.js | 26 ++++++++ 7 files changed, 108 insertions(+), 14 deletions(-) create mode 100644 core/client/app/adapters/tag.js diff --git a/core/client/app/adapters/tag.js b/core/client/app/adapters/tag.js new file mode 100644 index 0000000000..86b6282ddc --- /dev/null +++ b/core/client/app/adapters/tag.js @@ -0,0 +1,17 @@ +import ApplicationAdapter from 'ghost/adapters/application'; +import Ember from 'ember'; + +const {isBlank} = Ember; + +export default ApplicationAdapter.extend({ + buildURL: function (_modelName, _id, _snapshot, _requestType, query) { + let url = this._super(...arguments); + + if (query && !isBlank(query.slug)) { + url += `slug/${query.slug}/`; + delete query.slug; + } + + return url; + } +}); diff --git a/core/client/app/router.js b/core/client/app/router.js index 6cf2948189..df3504b00b 100644 --- a/core/client/app/router.js +++ b/core/client/app/router.js @@ -44,7 +44,7 @@ Router.map(function () { this.route('settings.general', {path: '/settings/general'}); this.route('settings.tags', {path: '/settings/tags'}, function () { - this.route('tag', {path: ':tag_id'}); + this.route('tag', {path: ':tag_slug'}); this.route('new'); }); this.route('settings.labs', {path: '/settings/labs'}); diff --git a/core/client/app/routes/settings/tags/tag.js b/core/client/app/routes/settings/tags/tag.js index ac75c44f45..3fe60cf5c6 100644 --- a/core/client/app/routes/settings/tags/tag.js +++ b/core/client/app/routes/settings/tags/tag.js @@ -3,7 +3,11 @@ import AuthenticatedRoute from 'ghost/routes/authenticated'; export default AuthenticatedRoute.extend({ model: function (params) { - return this.store.findRecord('tag', params.tag_id); + return this.store.queryRecord('tag', {slug: params.tag_slug}); + }, + + serialize: function (model) { + return {tag_slug: model.get('slug')}; }, // reset the model so that mobile screens react to an empty selectedTag diff --git a/core/client/app/templates/settings/tags.hbs b/core/client/app/templates/settings/tags.hbs index b54f7b074c..14e4f4c8fb 100644 --- a/core/client/app/templates/settings/tags.hbs +++ b/core/client/app/templates/settings/tags.hbs @@ -16,7 +16,7 @@
{{#each tags as |tag|}}
- {{#link-to 'settings.tags.tag' tag.id class="tag-edit-button"}} + {{#link-to 'settings.tags.tag' tag class="tag-edit-button"}} {{tag.name}} /{{tag.slug}}

{{tag.description}}

diff --git a/core/client/tests/acceptance/settings/tags-test.js b/core/client/tests/acceptance/settings/tags-test.js index 872e968f7b..75e99d25f4 100644 --- a/core/client/tests/acceptance/settings/tags-test.js +++ b/core/client/tests/acceptance/settings/tags-test.js @@ -129,6 +129,30 @@ describe('Acceptance: Settings - Tags', function () { return [200, {'Content-Type': 'application/json'}, JSON.stringify(response)]; }); + this.get('/ghost/api/v0.1/tags/slug/tag-two/', function (_request) { + let response = {}; + + response.tag = { + id: 2, + parent: null, + uuid: '0cade0f9-7a3f-4fd1-a80a-3a1ab7028340', + image: '/content/images/2015/10/tag-2.jpg', + name: 'Tag Two', + slug: 'tag-two', + description: 'Description two.', + meta_title: 'Meta Title Two', + meta_description: 'Meta description two.', + created_at: '2015-09-11T09:44:29.871Z', + created_by: 1, + updated_at: '2015-10-19T16:25:07.756Z', + updated_by: 1, + hidden: false, + post_count: 2 + }; + + return [200, {'Content-Type': 'application/json'}, JSON.stringify(response)]; + }); + this.put('/ghost/api/v0.1/tags/2/', function (_request) { let response = {}; @@ -237,7 +261,7 @@ describe('Acceptance: Settings - Tags', function () { andThen(() => { // it redirects to first tag - expect(currentURL(), 'currentURL').to.equal('/settings/tags/1'); + expect(currentURL(), 'currentURL').to.equal('/settings/tags/tag-one'); // it has correct page title expect(document.title, 'page title').to.equal('Settings - Tags - Test Blog'); @@ -253,7 +277,7 @@ describe('Acceptance: Settings - Tags', function () { .to.equal('Tag One'); // it highlights selected tag - expect(find('a[href="/settings/tags/1"]').hasClass('active'), 'highlights selected tag') + expect(find('a[href="/settings/tags/tag-one"]').hasClass('active'), 'highlights selected tag') .to.be.true; // it shows selected tag form @@ -268,10 +292,10 @@ describe('Acceptance: Settings - Tags', function () { andThen(() => { // it navigates to selected tag - expect(currentURL(), 'url after clicking tag').to.equal('/settings/tags/2'); + expect(currentURL(), 'url after clicking tag').to.equal('/settings/tags/tag-two'); // it highlights selected tag - expect(find('a[href="/settings/tags/2"]').hasClass('active'), 'highlights selected tag') + expect(find('a[href="/settings/tags/tag-two"]').hasClass('active'), 'highlights selected tag') .to.be.true; // it shows selected tag form @@ -287,10 +311,10 @@ describe('Acceptance: Settings - Tags', function () { }); // it navigates to previous tag - expect(currentURL(), 'url after keyboard up arrow').to.equal('/settings/tags/1'); + expect(currentURL(), 'url after keyboard up arrow').to.equal('/settings/tags/tag-one'); // it highlights selected tag - expect(find('a[href="/settings/tags/1"]').hasClass('active'), 'selects previous tag') + expect(find('a[href="/settings/tags/tag-one"]').hasClass('active'), 'selects previous tag') .to.be.true; }); @@ -302,10 +326,10 @@ describe('Acceptance: Settings - Tags', function () { }); // it navigates to previous tag - expect(currentURL(), 'url after keyboard down arrow').to.equal('/settings/tags/2'); + expect(currentURL(), 'url after keyboard down arrow').to.equal('/settings/tags/tag-two'); // it highlights selected tag - expect(find('a[href="/settings/tags/2"]').hasClass('active'), 'selects next tag') + expect(find('a[href="/settings/tags/tag-two"]').hasClass('active'), 'selects next tag') .to.be.true; }); @@ -345,14 +369,14 @@ describe('Acceptance: Settings - Tags', function () { andThen(() => { // it redirects to the new tag's URL - expect(currentURL(), 'URL after tag creation').to.equal('/settings/tags/3'); + expect(currentURL(), 'URL after tag creation').to.equal('/settings/tags/tag-three'); // it adds the tag to the list and selects expect(find('.settings-tags .settings-tag').length, 'tag list count after creation') .to.equal(3); expect(find('.settings-tags .settings-tag:last .tag-title').text(), 'new tag list item title') .to.equal('Tag Three'); - expect(find('a[href="/settings/tags/3"]').hasClass('active'), 'highlights new tag') + expect(find('a[href="/settings/tags/tag-three"]').hasClass('active'), 'highlights new tag') .to.be.true; }); @@ -362,7 +386,7 @@ describe('Acceptance: Settings - Tags', function () { andThen(() => { // it redirects to the first tag - expect(currentURL(), 'URL after tag deletion').to.equal('/settings/tags/1'); + expect(currentURL(), 'URL after tag deletion').to.equal('/settings/tags/tag-one'); // it removes the tag from the list expect(find('.settings-tags .settings-tag').length, 'tag list count after deletion') @@ -370,6 +394,26 @@ describe('Acceptance: Settings - Tags', function () { }); }); + it('loads tag via slug when accessed directly', function () { + visit('/settings/tags/tag-two'); + + andThen(() => { + expect(currentURL(), 'URL after direct load').to.equal('/settings/tags/tag-two'); + + // it loads all other tags + expect(find('.settings-tags .settings-tag').length, 'tag list count after direct load') + .to.equal(2); + + // selects tag in list + expect(find('a[href="/settings/tags/tag-two"]').hasClass('active'), 'highlights requested tag') + .to.be.true; + + // shows requested tag in settings pane + expect(find('.tag-settings-pane input[name="name"]').val(), 'loads correct tag into form') + .to.equal('Tag Two'); + }); + }); + it('has infinite scroll pagination of tags list'); }); }); diff --git a/core/server/routes/frontend.js b/core/server/routes/frontend.js index 1bf7feb31e..cb93ff5fc1 100644 --- a/core/server/routes/frontend.js +++ b/core/server/routes/frontend.js @@ -62,6 +62,9 @@ frontendRoutes = function frontendRoutes(middleware) { // Tags tagRouter.route('/').get(frontend.tag); tagRouter.route('/' + routeKeywords.page + '/:page/').get(frontend.tag); + tagRouter.route('/edit?').get(function redirect(req, res) { + res.redirect(subdir + '/ghost/settings/tags/' + req.params.slug + '/'); + }); tagRouter.use(rssRouter); // Authors diff --git a/core/test/functional/routes/frontend_spec.js b/core/test/functional/routes/frontend_spec.js index 5d5b57a1ed..ec9ca19aff 100644 --- a/core/test/functional/routes/frontend_spec.js +++ b/core/test/functional/routes/frontend_spec.js @@ -756,6 +756,32 @@ describe('Frontend Routing', function () { }); }); + describe('Tag edit', function () { + it('should redirect without slash', function (done) { + request.get('/tag/getting-started/edit') + .expect('Location', '/tag/getting-started/edit/') + .expect('Cache-Control', testUtils.cacheRules.year) + .expect(301) + .end(doEnd(done)); + }); + + it('should redirect to tag settings', function (done) { + request.get('/tag/getting-started/edit/') + .expect('Location', '/ghost/settings/tags/getting-started/') + .expect('Cache-Control', testUtils.cacheRules.public) + .expect(302) + .end(doEnd(done)); + }); + + it('should 404 for non-edit parameter', function (done) { + request.get('/tag/getting-started/notedit/') + .expect('Cache-Control', testUtils.cacheRules.private) + .expect(404) + .expect(/Page not found/) + .end(doEnd(done)); + }); + }); + describe('Subdirectory (no slash)', function () { var forkedGhost, request; before(function (done) {