OpenSourceArk/Ghost
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

✨ Improved performance loading posts & pages in admin (#20646)

Browse Source 
ref 8ea1dfb
ref https://linear.app/tryghost/issue/ONC-111

* undid the reversion for the performance improvements
* built upon new tests for the posts list functionality in admin,
including right click actions
* added tests for pages view in Admin

This was reverted because it broke the Pages list view in Admin, which
is a thin extension of the Posts functionality in admin (route &
controller). That has been fixed and tests added.

This was originally reverted because the changes to improve loading
response times broke right click (bulk) actions in the posts list. This
was not caught because it turned out we had near-zero test coverage of
that part of the codebase. Test coverage has been expanded for the posts
list, and while not comprehensive, is a much better place for us to be
in.
This commit is contained in:
Steve Larson 2024-07-29 11:19:28 -05:00 committed by GitHub
parent a109b255f0
commit c61c42ce1d
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
12 changed files with 834 additions and 629 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

2
ghost/admin/app/components/gh-context-menu.js
View File

@ -1,5 +1,5 @@
import Component from '@glimmer/component';
import SelectionList from '../utils/selection-list';
import SelectionList from './posts-list/selection-list';
import {action} from '@ember/object';
import {inject as service} from '@ember/service';
import {task} from 'ember-concurrency';

39
ghost/admin/app/components/posts-list/context-menu.js
View File

@ -216,11 +216,14 @@ export default class PostsContextMenu extends Component {
yield this.performBulkDestroy();
this.notifications.showNotification(this.#getToastMessage('deleted'), {type: 'success'});
const remainingModels = this.selectionList.infinityModel.content.filter((model) => {
return !deletedModels.includes(model);
});
// Deleteobjects method from infintiymodel is broken for all models except the first page, so we cannot use this
this.infinity.replace(this.selectionList.infinityModel, remainingModels);
for (const key in this.selectionList.infinityModel) {
const remainingModels = this.selectionList.infinityModel[key].content.filter((model) => {
return !deletedModels.includes(model);
});
// Deleteobjects method from infintiymodel is broken for all models except the first page, so we cannot use this
this.infinity.replace(this.selectionList.infinityModel[key], remainingModels);
}
this.selectionList.clearSelection({force: true});
return true;
}
@ -247,9 +250,7 @@ export default class PostsContextMenu extends Component {
}
}
// Remove posts that no longer match the filter
this.updateFilteredPosts();
return true;
}
@ -282,14 +283,16 @@ export default class PostsContextMenu extends Component {
]
});
const remainingModels = this.selectionList.infinityModel.content.filter((model) => {
if (!updatedModels.find(u => u.id === model.id)) {
return true;
}
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
this.infinity.replace(this.selectionList.infinityModel, remainingModels);
for (const key in this.selectionList.infinityModel) {
const remainingModels = this.selectionList.infinityModel[key].content.filter((model) => {
if (!updatedModels.find(u => u.id === model.id)) {
return true;
}
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
this.infinity.replace(this.selectionList.infinityModel[key], remainingModels);
}
this.selectionList.clearUnavailableItems();
}
@ -386,8 +389,10 @@ export default class PostsContextMenu extends Component {
const data = result[this.type === 'post' ? 'posts' : 'pages'][0];
const model = this.store.peekRecord(this.type, data.id);
// Update infinity list
this.selectionList.infinityModel.content.unshiftObject(model);
// Update infinity draft posts content - copied posts are always drafts
if (this.selectionList.infinityModel.draftInfinityModel) {
this.selectionList.infinityModel.draftInfinityModel.content.unshiftObject(model);
}
// Show notification
this.notifications.showNotification(this.#getToastMessage('duplicated'), {type: 'success'});

41
ghost/admin/app/components/posts-list/list.hbs
View File

@ -1,14 +1,39 @@
<MultiList::List @model={{@list}} class="posts-list gh-list {{unless @model "no-posts"}} feature-memberAttribution" as |list| >
{{#each @model as |post|}}
<list.item @id={{post.id}} class="gh-posts-list-item-group">
<PostsList::ListItem
@post={{post}}
data-test-post-id={{post.id}}
/>
</list.item>
{{!-- always order as scheduled, draft, remainder --}}
{{#if (or @model.scheduledInfinityModel (or @model.draftInfinityModel @model.publishedAndSentInfinityModel))}}
{{#if @model.scheduledInfinityModel}}
{{#each @model.scheduledInfinityModel as |post|}}
<list.item @id={{post.id}} class="gh-posts-list-item-group">
<PostsList::ListItem
@post={{post}}
data-test-post-id={{post.id}}
/>
</list.item>
{{/each}}
{{/if}}
{{#if (and @model.draftInfinityModel (or (not @model.scheduledInfinityModel) (and @model.scheduledInfinityModel @model.scheduledInfinityModel.reachedInfinity)))}}
{{#each @model.draftInfinityModel as |post|}}
<list.item @id={{post.id}} class="gh-posts-list-item-group">
<PostsList::ListItem
@post={{post}}
data-test-post-id={{post.id}}
/>
</list.item>
{{/each}}
{{/if}}
{{#if (and @model.publishedAndSentInfinityModel (and (or (not @model.scheduledInfinityModel) @model.scheduledInfinityModel.reachedInfinity) (or (not @model.draftInfinityModel) @model.draftInfinityModel.reachedInfinity)))}}
{{#each @model.publishedAndSentInfinityModel as |post|}}
<list.item @id={{post.id}} class="gh-posts-list-item-group">
<PostsList::ListItem
@post={{post}}
data-test-post-id={{post.id}}
/>
</list.item>
{{/each}}
{{/if}}
{{else}}
{{yield}}
{{/each}}
{{/if}}
</MultiList::List>
{{!-- The currently selected item or items are passed to the context menu --}}

98
ghost/admin/app/utils/selection-list.js → ghost/admin/app/components/posts-list/selection-list.js
View File

@ -18,7 +18,11 @@ export default class SelectionList {
#clearOnNextUnfreeze = false;
constructor(infinityModel) {
this.infinityModel = infinityModel ?? {content: []};
this.infinityModel = infinityModel ?? {
draftInfinityModel: {
content: []
}
};
}
freeze() {
@ -41,7 +45,12 @@ export default class SelectionList {
* Returns an NQL filter for all items, not the selection
*/
get allFilter() {
return this.infinityModel.extraParams?.filter ?? '';
const models = this.infinityModel;
// grab filter from the first key in the infinityModel object (they should all be identical)
for (const key in models) {
return models[key].extraParams?.allFilter ?? '';
}
return '';
}
/**
@ -81,10 +90,13 @@ export default class SelectionList {
* Keep in mind that when using CMD + A, we don't have all items in memory!
*/
get availableModels() {
const models = this.infinityModel;
const arr = [];
for (const item of this.infinityModel.content) {
if (this.isSelected(item.id)) {
arr.push(item);
for (const key in models) {
for (const item of models[key].content) {
if (this.isSelected(item.id)) {
arr.push(item);
}
}
}
return arr;
@ -102,7 +114,13 @@ export default class SelectionList {
if (!this.inverted) {
return this.selectedIds.size;
}
return Math.max((this.infinityModel.meta?.pagination?.total ?? 0) - this.selectedIds.size, 1);
const models = this.infinityModel;
let total;
for (const key in models) {
total += models[key].meta?.pagination?.total;
}
return Math.max((total ?? 0) - this.selectedIds.size, 1);
}
isSelected(id) {
@ -147,9 +165,12 @@ export default class SelectionList {
clearUnavailableItems() {
const newSelection = new Set();
for (const item of this.infinityModel.content) {
if (this.selectedIds.has(item.id)) {
newSelection.add(item.id);
const models = this.infinityModel;
for (const key in models) {
for (const item of models[key].content) {
if (this.selectedIds.has(item.id)) {
newSelection.add(item.id);
}
}
}
this.selectedIds = newSelection;
@ -181,37 +202,40 @@ export default class SelectionList {
// todo
let running = false;
for (const item of this.infinityModel.content) {
// Exlusing the last selected item
if (item.id === this.lastSelectedId || item.id === id) {
if (!running) {
running = true;
const models = this.infinityModel;
for (const key in models) {
for (const item of this.models[key].content) {
// Exlusing the last selected item
if (item.id === this.lastSelectedId || item.id === id) {
if (!running) {
running = true;
// Skip last selected on its own
if (item.id === this.lastSelectedId) {
continue;
}
} else {
// Still include id
if (item.id === id) {
this.lastShiftSelectionGroup.add(item.id);
if (this.inverted) {
this.selectedIds.delete(item.id);
} else {
this.selectedIds.add(item.id);
// Skip last selected on its own
if (item.id === this.lastSelectedId) {
continue;
}
}
break;
}
}
} else {
// Still include id
if (item.id === id) {
this.lastShiftSelectionGroup.add(item.id);
if (running) {
this.lastShiftSelectionGroup.add(item.id);
if (this.inverted) {
this.selectedIds.delete(item.id);
} else {
this.selectedIds.add(item.id);
if (this.inverted) {
this.selectedIds.delete(item.id);
} else {
this.selectedIds.add(item.id);
}
}
break;
}
}
if (running) {
this.lastShiftSelectionGroup.add(item.id);
if (this.inverted) {
this.selectedIds.delete(item.id);
} else {
this.selectedIds.add(item.id);
}
}
}
}

2
ghost/admin/app/controllers/pages.js
View File

@ -40,4 +40,4 @@ export default class PagesController extends PostsController {
openEditor(page) {
this.router.transitionTo('lexical-editor.edit', 'page', page.get('id'));
}
}
}

10
ghost/admin/app/controllers/posts.js
View File

@ -1,5 +1,5 @@
import Controller from '@ember/controller';
import SelectionList from 'ghost-admin/utils/selection-list';
import SelectionList from 'ghost-admin/components/posts-list/selection-list';
import {DEFAULT_QUERY_PARAMS} from 'ghost-admin/helpers/reset-query-params';
import {action} from '@ember/object';
import {inject} from 'ghost-admin/decorators/inject';
@ -85,14 +85,6 @@ export default class PostsController extends Controller {
Object.assign(this, DEFAULT_QUERY_PARAMS.posts);
}
get postsInfinityModel() {
return this.model;
}
get totalPosts() {
return this.model.meta?.pagination?.total ?? 0;
}
get showingAll() {
const {type, author, tag, visibility} = this;

56
ghost/admin/app/routes/posts.js
View File

@ -1,4 +1,5 @@
import AuthenticatedRoute from 'ghost-admin/routes/authenticated';
import RSVP from 'rsvp';
import {action} from '@ember/object';
import {assign} from '@ember/polyfills';
import {isBlank} from '@ember/utils';
@ -39,43 +40,54 @@ export default class PostsRoute extends AuthenticatedRoute {
model(params) {
const user = this.session.user;
let queryParams = {};
let filterParams = {tag: params.tag, visibility: params.visibility};
let paginationParams = {
perPageParam: 'limit',
totalPagesParam: 'meta.pagination.pages'
};
// type filters are actually mapping statuses
assign(filterParams, this._getTypeFilters(params.type));
if (params.type === 'featured') {
filterParams.featured = true;
}
// authors and contributors can only view their own posts
if (user.isAuthor) {
// authors can only view their own posts
filterParams.authors = user.slug;
} else if (user.isContributor) {
// Contributors can only view their own draft posts
filterParams.authors = user.slug;
// filterParams.status = 'draft';
// otherwise we need to filter by author if present
} else if (params.author) {
filterParams.authors = params.author;
}
let filter = this._filterString(filterParams);
if (!isBlank(filter)) {
queryParams.filter = filter;
}
if (!isBlank(params.order)) {
queryParams.order = params.order;
}
let perPage = this.perPage;
let paginationSettings = assign({perPage, startingPage: 1}, paginationParams, queryParams);
const filterStatuses = filterParams.status;
let queryParams = {allFilter: this._filterString({...filterParams})}; // pass along the parent filter so it's easier to apply the params filter to each infinity model
let models = {};
return this.infinity.model(this.modelName, paginationSettings);
if (filterStatuses.includes('scheduled')) {
let scheduledInfinityModelParams = {...queryParams, order: params.order || 'published_at desc', filter: this._filterString({...filterParams, status: 'scheduled'})};
models.scheduledInfinityModel = this.infinity.model(this.modelName, assign({perPage, startingPage: 1}, paginationParams, scheduledInfinityModelParams));
}
if (filterStatuses.includes('draft')) {
let draftInfinityModelParams = {...queryParams, order: params.order || 'updated_at desc', filter: this._filterString({...filterParams, status: 'draft'})};
models.draftInfinityModel = this.infinity.model(this.modelName, assign({perPage, startingPage: 1}, paginationParams, draftInfinityModelParams));
}
if (filterStatuses.includes('published') || filterStatuses.includes('sent')) {
let publishedAndSentInfinityModelParams;
if (filterStatuses.includes('published') && filterStatuses.includes('sent')) {
publishedAndSentInfinityModelParams = {...queryParams, order: params.order || 'published_at desc', filter: this._filterString({...filterParams, status: '[published,sent]'})};
} else {
publishedAndSentInfinityModelParams = {...queryParams, order: params.order || 'published_at desc', filter: this._filterString({...filterParams, status: filterStatuses.includes('published') ? 'published' : 'sent'})};
}
models.publishedAndSentInfinityModel = this.infinity.model(this.modelName, assign({perPage, startingPage: 1}, paginationParams, publishedAndSentInfinityModelParams));
}
return RSVP.hash(models);
}
// trigger a background load of all tags and authors for use in filter dropdowns
@ -120,6 +132,12 @@ export default class PostsRoute extends AuthenticatedRoute {
};
}
/**
* Returns an object containing the status filter based on the given type.
*
* @param {string} type - The type of filter to generate (draft, published, scheduled, sent).
* @returns {Object} - An object containing the status filter.
*/
_getTypeFilters(type) {
let status = '[draft,scheduled,published,sent]';

23
ghost/admin/app/templates/pages.hbs
View File

@ -28,7 +28,7 @@
<section class="view-container content-list">
<PostsList::List
@model={{this.postsInfinityModel}}
@model={{@model}}
@list={{this.selectionList}}
>
<li class="no-posts-box">
@ -41,7 +41,7 @@
</LinkTo>
{{else}}
<h4>No pages match the current filter</h4>
<LinkTo @route="pages" @query={{hash type=null author=null tag=null}} class="gh-btn">
<LinkTo @route="pages" @query={{hash visibility=null type=null author=null tag=null}} class="gh-btn">
<span>Show all pages</span>
</LinkTo>
{{/if}}
@ -49,11 +49,26 @@
</li>
</PostsList::List>
{{!-- only show one infinity loader wheel at a time - always order as scheduled, draft, remainder --}}
{{#if @model.scheduledInfinityModel}}
<GhInfinityLoader
@infinityModel={{this.postsInfinityModel}}
@infinityModel={{@model.scheduledInfinityModel}}
@scrollable=".gh-main"
@triggerOffset={{1000}} />
</section>
{{/if}}
{{#if (and @model.draftInfinityModel (or (not @model.scheduledInfinityModel) (and @model.scheduledInfinityModel @model.scheduledInfinityModel.reachedInfinity)))}}
<GhInfinityLoader
@infinityModel={{@model.draftInfinityModel}}
@scrollable=".gh-main"
@triggerOffset={{1000}} />
{{/if}}
{{#if (and @model.publishedAndSentInfinityModel (and (or (not @model.scheduledInfinityModel) @model.scheduledInfinityModel.reachedInfinity) (or (not @model.draftInfinityModel) @model.draftInfinityModel.reachedInfinity)))}}
<GhInfinityLoader
@infinityModel={{@model.publishedAndSentInfinityModel}}
@scrollable=".gh-main"
@triggerOffset={{1000}} />
{{/if}}
</section>
{{outlet}}
</section>

23
ghost/admin/app/templates/posts.hbs
View File

@ -30,7 +30,7 @@
<section class="view-container content-list">
<PostsList::List
@model={{this.postsInfinityModel}}
@model={{@model}}
@list={{this.selectionList}}
>
<li class="no-posts-box" data-test-no-posts-box>
@ -43,7 +43,7 @@
</LinkTo>
{{else}}
<h4>No posts match the current filter</h4>
<LinkTo @route="posts" @query={{hash type=null author=null tag=null}} class="gh-btn" data-test-link="show-all">
<LinkTo @route="posts" @query={{hash type=null author=null tag=null visibility=null}} class="gh-btn" data-test-link="show-all">
<span>Show all posts</span>
</LinkTo>
{{/if}}
@ -51,11 +51,26 @@
</li>
</PostsList::List>
{{!-- only show one infinity loader wheel at a time - always order as scheduled, draft, remainder --}}
{{#if @model.scheduledInfinityModel}}
<GhInfinityLoader
@infinityModel={{this.postsInfinityModel}}
@infinityModel={{@model.scheduledInfinityModel}}
@scrollable=".gh-main"
@triggerOffset={{1000}} />
</section>
{{/if}}
{{#if (and @model.draftInfinityModel (or (not @model.scheduledInfinityModel) (and @model.scheduledInfinityModel @model.scheduledInfinityModel.reachedInfinity)))}}
<GhInfinityLoader
@infinityModel={{@model.draftInfinityModel}}
@scrollable=".gh-main"
@triggerOffset={{1000}} />
{{/if}}
{{#if (and @model.publishedAndSentInfinityModel (and (or (not @model.scheduledInfinityModel) @model.scheduledInfinityModel.reachedInfinity) (or (not @model.draftInfinityModel) @model.draftInfinityModel.reachedInfinity)))}}
<GhInfinityLoader
@infinityModel={{@model.publishedAndSentInfinityModel}}
@scrollable=".gh-main"
@triggerOffset={{1000}} />
{{/if}}
</section>
{{outlet}}
</section>

1
ghost/admin/mirage/config/pages.js
View File

@ -37,7 +37,6 @@ export default function mockPages(server) {
return pages.create(attrs);
});
// TODO: handle authors filter
server.get('/pages/', function ({pages}, {queryParams}) {
let {filter, page, limit} = queryParams;

25
ghost/admin/mirage/config/posts.js
View File

@ -23,7 +23,6 @@ function extractTags(postAttrs, tags) {
});
}
// TODO: handle authors filter
export function getPosts({posts}, {queryParams}) {
let {filter, page, limit} = queryParams;
@ -31,15 +30,27 @@ export function getPosts({posts}, {queryParams}) {
limit = +limit || 15;
let statusFilter = extractFilterParam('status', filter);
let authorsFilter = extractFilterParam('authors', filter);
let visibilityFilter = extractFilterParam('visibility', filter);
let collection = posts.all().filter((post) => {
let matchesStatus = true;
let matchesAuthors = true;
let matchesVisibility = true;
if (!isEmpty(statusFilter)) {
matchesStatus = statusFilter.includes(post.status);
}
return matchesStatus;
if (!isEmpty(authorsFilter)) {
matchesAuthors = authorsFilter.includes(post.authors.models[0].slug);
}
if (!isEmpty(visibilityFilter)) {
matchesVisibility = visibilityFilter.includes(post.visibility);
}
return matchesStatus && matchesAuthors && matchesVisibility;
});
return paginateModelCollection('posts', collection, page, limit);
@ -59,7 +70,6 @@ export default function mockPosts(server) {
return posts.create(attrs);
});
// TODO: handle authors filter
server.get('/posts/', getPosts);
server.get('/posts/:id/', function ({posts}, {params}) {
@ -100,6 +110,13 @@ export default function mockPosts(server) {
posts.find(ids).destroy();
});
server.post('/posts/:id/copy/', function ({posts}, {params}) {
let post = posts.find(params.id);
let attrs = post.attrs;
return posts.create(attrs);
});
server.put('/posts/bulk/', function ({tags}, {requestBody}) {
const bulk = JSON.parse(requestBody).bulk;
const action = bulk.action;
@ -115,7 +132,7 @@ export default function mockPosts(server) {
tags.create(tag);
}
});
// TODO: update the actual posts in the mock db
// TODO: update the actual posts in the mock db if wanting to write tests where we navigate around (refresh model)
// const postsToUpdate = posts.find(ids);
// getting the posts is fine, but within this we CANNOT manipulate them (???) not even iterate with .forEach
}

1143
ghost/admin/tests/acceptance/content-test.js
View File

File diff suppressed because it is too large Load Diff