Infrastructure/ghost
0
Fork 0
mirror of https://github.com/TryGhost/Ghost.git synced 2025-01-20 22:42:53 -05:00
Code Issues Activity

Fixed bugs in post's change-access context menu flow (#21173)

Browse source 
ref 86d61304b1
ref https://linear.app/tryghost/issue/ONC-323

- added `tracked()` to our proxy model object properties
  - fixes default data always showing when opening the modal
- fixed data push after completing modal
- `post.tiers` is set up as an attribute in Admin rather than a relationship
- fixes incorrect tiers list showing when the change access modal is opened again after changing access before the post is re-fetched from the API
- fixed flash of failure button state when saving modal changes
- expanded tests to cover tiers selection
This commit is contained in:
Kevin Ansfield 2024-10-01 18:16:23 +01:00 • committed by GitHub
parent 8aaac5abe1
commit 3bbe8c8c7a
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
7 changed files with 133 additions and 48 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

22
ghost/admin/app/components/posts-list/context-menu.hbs
View file

@ -1,29 +1,29 @@
<ul class="gh-posts-context-menu dropdown-menu dropdown-triangle-top-left">
<ul class="gh-posts-context-menu dropdown-menu dropdown-triangle-top-left" data-test-post-context-menu>
{{#if this.canUnpublishSelection}}
{{#if this.canCopySelection}}
<li>
<button class="mr2" type="button" {{on "click" this.copyPostLink}}>
<button class="mr2" type="button" {{on "click" this.copyPostLink}} data-test-button="copy-link">
<span>{{svg-jar "link"}}Copy link to post</span>
</button>
</li>
{{/if}}
<li>
<button class="mr2" type="button" {{on "click" this.unpublishPosts}}>
<button class="mr2" type="button" {{on "click" this.unpublishPosts}} data-test-button="unpublish">
<span>{{svg-jar "undo"}}Unpublish</span>
</button>
</li>
{{else}}
{{#if this.canCopySelection}}
<li>
<button class="mr2" type="button" {{on "click" this.copyPreviewLink}}>
<button class="mr2" type="button" {{on "click" this.copyPreviewLink}} data-test-button="copy-preview">
<span>{{svg-jar "link"}}Copy preview link</span>
</button>
</li>
{{/if}}
{{#if this.canUnscheduleSelection}}
<li>
<button class="mr2" type="button" {{on "click" this.unschedulePosts}}>
<button class="mr2" type="button" {{on "click" this.unschedulePosts}} data-test-button="unschedule">
<span>{{svg-jar "undo"}}Unschedule</span>
</button>
</li>
@ -32,26 +32,26 @@
{{#if this.canFeatureSelection}}
{{#if this.shouldFeatureSelection }}
<li>
<button class="mr2" type="button" {{on "click" this.featurePosts}}>
<button class="mr2" type="button" {{on "click" this.featurePosts}} data-test-button="feature">
<span>{{svg-jar "star" class="mb1 star"}}Feature</span>
</button>
</li>
{{else}}
<li>
<button class="mr2" type="button" {{on "click" this.unfeaturePosts}}>
<button class="mr2" type="button" {{on "click" this.unfeaturePosts}} data-test-button="unfeature">
<span>{{svg-jar "star" class="mb1"}}Unfeature</span>
</button>
</li>
{{/if}}
{{/if}}
<li>
<button class="mr2" type="button" {{on "click" this.addTagToPosts}}>
<button class="mr2" type="button" {{on "click" this.addTagToPosts}} data-test-button="add-tag">
<span>{{svg-jar "tag"}}Add a tag</span>
</button>
</li>
{{#if this.membersUtils.isMembersEnabled}}
<li>
<button class="mr2" type="button" {{on "click" this.editPostsAccess}}>
<button class="mr2" type="button" {{on "click" this.editPostsAccess}} data-test-button="change-access">
<span>{{svg-jar "lock"}}Change access</span>
</button>
</li>
@ -59,13 +59,13 @@
{{#if this.session.user.isAdmin}}
{{#if this.canCopySelection}}
<li>
<button class="mr2" type="button" {{on "click" this.copyPosts}}>
<button class="mr2" type="button" {{on "click" this.copyPosts}} data-test-button="duplicate">
<span>{{svg-jar "duplicate"}}Duplicate</span>
</button>
</li>
{{/if}}
<li>
<button class="mr2" type="button" {{on "click" this.deletePosts}}>
<button class="mr2" type="button" {{on "click" this.deletePosts}} data-test-button="delete">
<span class="red">{{svg-jar "trash"}}Delete</span>
</button>
</li>

12
ghost/admin/app/components/posts-list/context-menu.js
View file

@ -348,7 +348,7 @@ export default class PostsContextMenu extends Component {
}
return filterNql.queryJSON(model.serialize({includeId: true}));
});
// Deleteobjects method from infintiymodel is broken for all models except the first page, so we cannot use this
// Deleteobjects method from infinitymodel is broken for all models except the first page, so we cannot use this
this.infinity.replace(this.selectionList.infinityModel[key], remainingModels);
}
@ -369,12 +369,8 @@ export default class PostsContextMenu extends Component {
id: post.id,
type: this.type,
attributes: {
visibility
},
relationships: {
links: {
data: tiers
}
visibility,
tiers // tiers is a weird one, it's set up as an attribute but represents a relationship
}
}
});
@ -384,6 +380,8 @@ export default class PostsContextMenu extends Component {
this.updateFilteredPosts();
close();
return true;
}
@task

7
ghost/admin/app/components/posts-list/modals/edit-posts-access.js
View file

@ -8,7 +8,10 @@ import {tracked} from '@glimmer/tracking';
const PostValidatorProxy = EmberObject.extend(ValidationEngine, {
validationType: 'post',
isNew: false // required for our visibility and tiers validations to work
isNew: false, // required for our visibility and tiers validations to work
visibility: tracked(),
tiers: tracked()
});
export default class EditPostsAccessModal extends Component {
@ -27,7 +30,7 @@ export default class EditPostsAccessModal extends Component {
setup() {
if (this.selectionList.first && this.selectionList.isSingle) {
this.post.visibility = this.selectionList.first.visibility;
this.post.tiers = this.selectionList.first.tiers;
this.post.tiers = this.selectionList.first.tiers || [];
} else {
// Use default
this.post.visibility = this.settings.defaultContentVisibility;

26
ghost/admin/mirage/config/posts.js
View file

@ -125,10 +125,10 @@ export default function mockPosts(server) {
return posts.create(attrs);
});
server.put('/posts/bulk/', function ({tags}, {requestBody}) {
server.put('/posts/bulk/', function ({posts, tags}, {queryParams, requestBody}) {
const bulk = JSON.parse(requestBody).bulk;
const action = bulk.action;
// const ids = extractFilterParam('id', queryParams.filter);
const ids = extractFilterParam('id', queryParams.filter);
if (action === 'addTag') {
// create tag so we have an id from the server
@ -144,5 +144,27 @@ export default function mockPosts(server) {
// const postsToUpdate = posts.find(ids);
// getting the posts is fine, but within this we CANNOT manipulate them (???) not even iterate with .forEach
}
if (action === 'access') {
const postsToUpdate = posts.find(ids);
postsToUpdate.models.forEach((post) => {
post.visibility = bulk.meta.visibility;
post.tierIds = bulk.meta.tiers.map(tier => tier.id);
post.save();
});
}
return {
bulk: {
meta: {
errors: [],
stats: {
successful: ids.length,
unsuccessful: 0
},
unsuccessfulData: []
}
}
};
});
}

3
ghost/admin/mirage/models/post.js
View file

@ -5,5 +5,6 @@ export default Model.extend({
authors: hasMany('user'),
email: belongsTo(),
newsletter: belongsTo(),
postRevisions: hasMany()
postRevisions: hasMany(),
tiers: hasMany()
});

1
ghost/admin/mirage/serializers/post.js
View file

@ -11,6 +11,7 @@ export default BaseSerializer.extend({
// embedded records that are included by default in the API
includes.add('tags');
includes.add('authors');
includes.add('tiers');
// clean up some things that mirage doesn't understand
includes.delete('authorsRoles');

110
ghost/admin/tests/acceptance/content-test.js
View file

@ -109,6 +109,9 @@ describe('Acceptance: Posts / Pages', function () {
let admin, editor, publishedPost, scheduledPost, draftPost, authorPost;
beforeEach(async function () {
this.server.loadFixtures('settings');
this.server.loadFixtures('tiers');
let adminRole = this.server.create('role', {name: 'Administrator'});
admin = this.server.create('user', {roles: [adminRole]});
let editorRole = this.server.create('role', {name: 'Editor'});
@ -448,44 +451,50 @@ describe('Acceptance: Posts / Pages', function () {
expect(JSON.parse(lastRequest.requestBody).bulk.action, 'add tag request action').to.equal('addTag');
});
it('can change access', async function () {
it('cannot change access when members is disabled', async function () {
await visit('/posts');
const settingsService = this.owner.lookup('service:settings');
await settingsService.set('membersEnabled', false);
// get all posts
const posts = findAll('[data-test-post-id]');
expect(posts.length, 'all posts count').to.equal(4);
const postThreeContainer = posts[2].parentElement; // draft post
const postFourContainer = posts[3].parentElement; // published post
const postThreeContainer = posts[2].parentElement; // published post
const postFourContainer = posts[3].parentElement; // author post
await click(postThreeContainer, {metaKey: ctrlOrCmd === 'command', ctrlKey: ctrlOrCmd === 'ctrl'});
await click(postFourContainer, {metaKey: ctrlOrCmd === 'command', ctrlKey: ctrlOrCmd === 'ctrl'});
expect(postFourContainer.getAttribute('data-selected'), 'postFour selected').to.exist;
expect(postThreeContainer.getAttribute('data-selected'), 'postThree selected').to.exist;
await triggerEvent(postFourContainer, 'contextmenu');
let contextMenu = find('.gh-posts-context-menu'); // this is a <ul> element
expect(contextMenu, 'context menu').to.exist;
let buttons = contextMenu.querySelectorAll('button');
let changeAccessButton = findButton('Change access', buttons);
expect(changeAccessButton, 'change access button').not.to.exist;
expect(find('[data-test-post-context-menu]'), 'context menu').to.exist;
expect(find('[data-test-post-context-menu] [data-test-button="change-access"]'), 'change access button').not.to.exist;
});
it('can change access', async function () {
await visit('/posts');
const settingsService = this.owner.lookup('service:settings');
await settingsService.set('membersEnabled', true);
await triggerEvent(postFourContainer, 'contextmenu');
contextMenu = find('.gh-posts-context-menu'); // this is a <ul> element
expect(contextMenu, 'context menu').to.exist;
buttons = contextMenu.querySelectorAll('button');
changeAccessButton = findButton('Change access', buttons);
let posts = findAll('[data-test-post-id]');
let postThreeContainer = posts[2].parentElement; // published post
let postFourContainer = posts[3].parentElement; // author post
await click(postThreeContainer, {metaKey: ctrlOrCmd === 'command', ctrlKey: ctrlOrCmd === 'ctrl'});
await click(postFourContainer, {metaKey: ctrlOrCmd === 'command', ctrlKey: ctrlOrCmd === 'ctrl'});
await triggerEvent(postFourContainer, 'contextmenu');
let contextMenu = find('.gh-posts-context-menu'); // this is a <ul> element
let buttons = contextMenu.querySelectorAll('button');
let changeAccessButton = findButton('Change access', buttons);
expect(changeAccessButton, 'change access button').to.exist;
await click(changeAccessButton);
const changeAccessModal = find('[data-test-modal="edit-posts-access"]');
const selectElement = changeAccessModal.querySelector('select');
let changeAccessModal = find('[data-test-modal="edit-posts-access"]');
let selectElement = changeAccessModal.querySelector('select');
await fillIn(selectElement, 'members');
await click('[data-test-button="confirm"]');
@ -494,6 +503,20 @@ describe('Acceptance: Posts / Pages', function () {
expect(lastRequest.queryParams.filter, 'change access request id').to.equal(`id:['${publishedPost.id}','${authorPost.id}']`);
expect(JSON.parse(lastRequest.requestBody).bulk.action, 'change access request action').to.equal('access');
// ensure modal matches the new state when accessed again
// NOTE: we only show the selected visibility/tiers state for single selections
await click(postThreeContainer, {metaKey: ctrlOrCmd === 'command', ctrlKey: ctrlOrCmd === 'ctrl'});
postFourContainer = findAll('[data-test-post-id]')[3].parentElement; // published post
await triggerEvent(postFourContainer, 'contextmenu');
contextMenu = find('.gh-posts-context-menu'); // this is a <ul> element
buttons = contextMenu.querySelectorAll('button');
changeAccessButton = findButton('Change access', buttons);
await click(changeAccessButton);
changeAccessModal = find('[data-test-modal="edit-posts-access"]');
selectElement = changeAccessModal.querySelector('select');
expect(selectElement, 'access select value after changing').to.have.value('members');
await click(changeAccessModal.querySelector('[data-test-button="cancel"]'));
// ensure creating new posts still works
// (we had a bug where newly created records in the store had `isNew: false` set meaning any saves failed
// because Ember Data attempted a PUT with no id)
@ -504,6 +527,43 @@ describe('Acceptance: Posts / Pages', function () {
expect(this.server.db.posts.length, 'posts count after new post save').to.equal(5);
});
it('can change access with custom tiers', async function () {
await visit('/posts');
const settingsService = this.owner.lookup('service:settings');
await settingsService.set('membersEnabled', true);
const postContainer = findAll('[data-test-post-id]')[2].parentElement; // published post
await triggerEvent(postContainer, 'contextmenu');
await click('[data-test-post-context-menu] [data-test-button="change-access"]');
const modalSelector = '[data-test-modal="edit-posts-access"]';
const tiersSelector = `${modalSelector} [data-test-visibility-segment-select]`;
expect(find(tiersSelector)).not.to.exist;
await fillIn(`${modalSelector} select`, 'tiers');
expect(find(tiersSelector)).to.exist;
expect(findAll(`${tiersSelector} [data-test-visibility-segment-option]`)).to.have.length(0);
await clickTrigger(tiersSelector);
await selectChoose(tiersSelector, 'Default Tier');
await click(`${modalSelector} [data-test-button="confirm"]`);
// check API request
let [lastRequest] = this.server.pretender.handledRequests.slice(-1);
expect(lastRequest.queryParams.filter, 'change access request id').to.equal(`id:['${publishedPost.id}']`);
expect(JSON.parse(lastRequest.requestBody).bulk.action, 'change access request action').to.equal('access');
expect(JSON.parse(lastRequest.requestBody).bulk.meta.visibility, 'change access request visibility').to.equal('tiers');
expect(JSON.parse(lastRequest.requestBody).bulk.meta.tiers[0].id, 'change access request tier').to.equal(this.server.schema.tiers.findBy({slug: 'default-tier'}).id);
// check correct data is shown when re-accessing change access modal
await triggerEvent(postContainer, 'contextmenu');
await click('[data-test-post-context-menu] [data-test-button="change-access"]');
expect(find(`${modalSelector} select`).value).to.equal('tiers');
expect(findAll(`${tiersSelector} [data-test-visibility-segment-option]`)).to.have.length(1);
expect(find(`${tiersSelector} [data-test-visibility-segment-option]`).textContent.trim()).to.equal('Default Tier');
});
it('can unpublish', async function () {
await visit('/posts');
@ -673,7 +733,7 @@ describe('Acceptance: Posts / Pages', function () {
});
it('can navigate to custom views', async function () {
this.server.create('setting', {
this.server.schema.settings.findBy({key: 'shared_views'}).update({
group: 'site',
key: 'shared_views',
value: JSON.stringify([{
@ -688,10 +748,10 @@ describe('Acceptance: Posts / Pages', function () {
await visit('/posts');
// nav bar contains default + custom views
expect(find('[data-test-nav-custom="posts-Drafts"]')).to.exist;
expect(find('[data-test-nav-custom="posts-Scheduled"]')).to.exist;
expect(find('[data-test-nav-custom="posts-Published"]')).to.exist;
expect(find('[data-test-nav-custom="posts-My posts"]')).to.exist;
expect(find('[data-test-nav-custom="posts-Drafts"]'), 'drafts nav').to.exist;
expect(find('[data-test-nav-custom="posts-Scheduled"]'), 'scheduled nav').to.exist;
expect(find('[data-test-nav-custom="posts-Published"]'), 'published nav').to.exist;
expect(find('[data-test-nav-custom="posts-My posts"]'), 'my posts nav').to.exist;
// screen has default title and sidebar is showing inactive custom view
expect(find('[data-test-screen-title]')).to.have.rendered.trimmed.text('Posts');