From 3bbe8c8c7adc760b51b87aa8650e0eae68ce7fb1 Mon Sep 17 00:00:00 2001
From: Kevin Ansfield <kevin@ghost.org>
Date: Tue, 1 Oct 2024 18:16:23 +0100
Subject: [PATCH] Fixed bugs in post's change-access context menu flow (#21173)

ref https://github.com/TryGhost/Ghost/commit/86d61304b11218c4a9786bd7afbd65675ddf0b2e
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
---
 .../components/posts-list/context-menu.hbs    |  22 ++--
 .../app/components/posts-list/context-menu.js |  12 +-
 .../posts-list/modals/edit-posts-access.js    |   7 +-
 ghost/admin/mirage/config/posts.js            |  26 ++++-
 ghost/admin/mirage/models/post.js             |   3 +-
 ghost/admin/mirage/serializers/post.js        |   1 +
 ghost/admin/tests/acceptance/content-test.js  | 110 ++++++++++++++----
 7 files changed, 133 insertions(+), 48 deletions(-)

diff --git a/ghost/admin/app/components/posts-list/context-menu.hbs b/ghost/admin/app/components/posts-list/context-menu.hbs
index 6646408e0a..cb60862df8 100644
--- a/ghost/admin/app/components/posts-list/context-menu.hbs
+++ b/ghost/admin/app/components/posts-list/context-menu.hbs
@@ -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>
diff --git a/ghost/admin/app/components/posts-list/context-menu.js b/ghost/admin/app/components/posts-list/context-menu.js
index e689acbd42..5ff09f7c2b 100644
--- a/ghost/admin/app/components/posts-list/context-menu.js
+++ b/ghost/admin/app/components/posts-list/context-menu.js
@@ -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
diff --git a/ghost/admin/app/components/posts-list/modals/edit-posts-access.js b/ghost/admin/app/components/posts-list/modals/edit-posts-access.js
index 3cc9b94846..9369a14eac 100644
--- a/ghost/admin/app/components/posts-list/modals/edit-posts-access.js
+++ b/ghost/admin/app/components/posts-list/modals/edit-posts-access.js
@@ -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;
diff --git a/ghost/admin/mirage/config/posts.js b/ghost/admin/mirage/config/posts.js
index 916869416b..3733795160 100644
--- a/ghost/admin/mirage/config/posts.js
+++ b/ghost/admin/mirage/config/posts.js
@@ -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: []
+                }
+            }
+        };
     });
 }
diff --git a/ghost/admin/mirage/models/post.js b/ghost/admin/mirage/models/post.js
index 46ea92d1fa..cb97fe8ed1 100644
--- a/ghost/admin/mirage/models/post.js
+++ b/ghost/admin/mirage/models/post.js
@@ -5,5 +5,6 @@ export default Model.extend({
     authors: hasMany('user'),
     email: belongsTo(),
     newsletter: belongsTo(),
-    postRevisions: hasMany()
+    postRevisions: hasMany(),
+    tiers: hasMany()
 });
diff --git a/ghost/admin/mirage/serializers/post.js b/ghost/admin/mirage/serializers/post.js
index 2985c0073f..0db130d37e 100644
--- a/ghost/admin/mirage/serializers/post.js
+++ b/ghost/admin/mirage/serializers/post.js
@@ -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');
diff --git a/ghost/admin/tests/acceptance/content-test.js b/ghost/admin/tests/acceptance/content-test.js
index 54a3309cda..e4eb0e6e6a 100644
--- a/ghost/admin/tests/acceptance/content-test.js
+++ b/ghost/admin/tests/acceptance/content-test.js
@@ -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');