🐛 Added missing history logs for post/page bulk actions (#16734)

no issue

The post/page bulk actions weren't logged in the history log / actions
table.

This change adds support for logging bulk actions.
- New `addActions` static method on models. It creates an action log in
the database for multiple models at once. If only one model was edited,
deleted or added, it will fallback to `addAction`
- `addAction` can also be called statically now
- `actionName` option is now supported when using `addActions`,
`addAction`, and as a result also in all bulk manipulation methods, and
CRUD methods. This allows you to replace the default '5 posts edited'
into something more specific like '5 posts featured'
- Fixed support for null resource_id in the parse-history-event helper
- Removed the default 'published' status requirement when using
Post.findOne for internal queries.
This commit is contained in:
Simon Backx 2023-05-05 09:45:36 +02:00 committed by GitHub
parent 5d392bbe57
commit fbed93b866
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
6 changed files with 202 additions and 76 deletions

View File

@ -20,32 +20,34 @@
<div>
<div class="gh-history-description">
<span>
{{capitalize-first-letter ev.action}}:
{{capitalize-first-letter ev.action}}{{#unless ev.isBulkAction}}:{{/unless}}
</span>
{{#if ev.contextResource}}
<span>
<span>{{capitalize-first-letter ev.contextResource.first}}</span>
{{#if (not-eq ev.contextResource.first ev.contextResource.second)}}
<code>({{ev.contextResource.second}})</code>
{{/if}}
</span>
{{else if (or ev.original.resource.title ev.original.resource.name ev.original.context.primary_name)}}
{{#if ev.linkTarget}}
{{#if ev.linkTarget.models}}
<LinkTo @route={{ev.linkTarget.route}} @models={{ev.linkTarget.models}} class="permalink fw6">
{{or ev.original.resource.title ev.original.resource.name}}
</LinkTo>
{{#unless ev.isBulkAction}}
{{#if ev.contextResource}}
<span>
<span>{{capitalize-first-letter ev.contextResource.first}}</span>
{{#if (not-eq ev.contextResource.first ev.contextResource.second)}}
<code>({{ev.contextResource.second}})</code>
{{/if}}
</span>
{{else if (or ev.original.resource.title ev.original.resource.name ev.original.context.primary_name)}}
{{#if ev.linkTarget}}
{{#if ev.linkTarget.models}}
<LinkTo @route={{ev.linkTarget.route}} @models={{ev.linkTarget.models}} class="permalink fw6">
{{or ev.original.resource.title ev.original.resource.name}}
</LinkTo>
{{else}}
<LinkTo @route={{ev.linkTarget.route}} class="permalink fw6">
{{or ev.original.resource.title ev.original.resource.name}}
</LinkTo>
{{/if}}
{{else}}
<LinkTo @route={{ev.linkTarget.route}} class="permalink fw6">
{{or ev.original.resource.title ev.original.resource.name}}
</LinkTo>
<span>{{or ev.original.resource.title ev.original.resource.name ev.original.context.primary_name}}</span>
{{/if}}
{{else}}
<span>{{or ev.original.resource.title ev.original.resource.name ev.original.context.primary_name}}</span>
<span class="midlightgrey">(unknown)</span>
{{/if}}
{{else}}
<span class="midlightgrey">(unknown)</span>
{{/if}}
{{/unless}}
<span class="gh-history-name">
<span class="midgrey">&ndash; by </span>

View File

@ -24,7 +24,8 @@ export default class ParseHistoryEvent extends Helper {
actor,
actorIcon,
actorLinkTarget,
original: ev
original: ev,
isBulkAction: !!ev.context.count
};
}
}
@ -88,7 +89,7 @@ function getLinkTarget(ev) {
switch (ev.resource_type) {
case 'page':
case 'post':
if (!ev.resource.id) {
if (!ev.resource || !ev.resource.id) {
return null;
}
@ -103,7 +104,7 @@ function getLinkTarget(ev) {
models: [resourceType, ev.resource.id]
};
case 'integration':
if (!ev.resource.id) {
if (!ev.resource || !ev.resource.id) {
return null;
}
@ -112,7 +113,7 @@ function getLinkTarget(ev) {
models: [ev.resource.id]
};
case 'offer':
if (!ev.resource.id) {
if (!ev.resource || !ev.resource.id) {
return null;
}
@ -121,7 +122,7 @@ function getLinkTarget(ev) {
models: [ev.resource.id]
};
case 'tag':
if (!ev.resource.slug) {
if (!ev.resource || !ev.resource.slug) {
return null;
}
@ -135,7 +136,7 @@ function getLinkTarget(ev) {
models: null
};
case 'user':
if (!ev.resource.slug) {
if (!ev.resource || !ev.resource.slug) {
return null;
}
@ -181,7 +182,19 @@ function getAction(ev) {
}
}
return `${resourceType} ${ev.event}`;
let action = ev.event;
if (ev.event === 'edited') {
if (ev.context.action_name) {
action = ev.context.action_name;
}
}
if (ev.context.count && ev.context.count > 1) {
return `${ev.context.count} ${resourceType}s ${action}`;
}
return `${resourceType} ${action}`;
}
function getContextResource(ev) {

View File

@ -6,6 +6,54 @@ const logging = require('@tryghost/logging');
* @param {import('bookshelf')} Bookshelf
*/
module.exports = function (Bookshelf) {
const insertAction = (data, options) => {
// CASE: model does not support action for target event
if (!data) {
return;
}
const insert = (action) => {
Bookshelf.model('Action')
.add(action, {autoRefresh: false})
.catch((err) => {
if (_.isArray(err)) {
err = err[0];
}
logging.error(new errors.InternalServerError({
err
}));
});
};
if (options.transacting) {
options.transacting.once('committed', (committed) => {
if (!committed) {
return;
}
insert(data);
});
} else {
insert(data);
}
};
// We need this addAction accessible from the static model and instances
const addAction = (model, event, options) => {
if (!model.wasChanged()) {
return;
}
// CASE: model does not support actions at all
if (!model.getAction) {
return;
}
const data = model.getAction(event, options);
insertAction(data, options);
};
Bookshelf.Model = Bookshelf.Model.extend({
/**
* Constructs data to be stored in the database with info
@ -33,7 +81,9 @@ module.exports = function (Bookshelf) {
return;
}
let context = {};
let context = {
action_name: options.actionName
};
if (this.actionsExtraContext && Array.isArray(this.actionsExtraContext)) {
for (const c of this.actionsExtraContext) {
@ -74,48 +124,73 @@ module.exports = function (Bookshelf) {
*
* We could embed adding actions more nicely in the future e.g. plugin.
*/
addAction: (model, event, options) => {
if (!model.wasChanged()) {
addAction
}, {
addAction,
async addActions(event, ids, options) {
if (ids.length === 1) {
// We want to store an event for a single model in the actions table
// This is so we can include the name
const model = await this.findOne({[options.column ?? 'id']: ids[0]}, {require: true, transacting: options.transacting, context: {internal: true}});
this.addAction(model, event, options);
return;
}
// CASE: model does not support actions at all
if (!model.getAction) {
const existingAction = this.getBulkAction(event, ids.length, options);
insertAction(existingAction, options);
},
/**
* Constructs data to be stored in the database with info
* on particular actions
*/
getBulkAction(event, count, options) {
const actor = this.prototype.getActor(options);
// @NOTE: we ignore internal updates (`options.context.internal`) for now
if (!actor) {
return;
}
const existingAction = model.getAction(event, options);
// CASE: model does not support action for target event
if (!existingAction) {
if (!this.prototype.actionsCollectCRUD) {
return;
}
const insert = (action) => {
Bookshelf.model('Action')
.add(action, {autoRefresh: false})
.catch((err) => {
if (_.isArray(err)) {
err = err[0];
}
let resourceType = this.prototype.actionsResourceType;
logging.error(new errors.InternalServerError({
err
}));
});
if (typeof resourceType === 'function') {
resourceType = resourceType.bind(this)();
}
if (!resourceType) {
return;
}
let context = {
count,
action_name: options.actionName
};
if (options.transacting) {
options.transacting.once('committed', (committed) => {
if (!committed) {
return;
}
insert(existingAction);
});
} else {
insert(existingAction);
if (this.getBulkActionExtraContext && typeof this.getBulkActionExtraContext === 'function') {
context = {
...context,
...this.getBulkActionExtraContext.bind(this)(options)
};
}
const data = {
event,
resource_id: null,
resource_type: resourceType,
actor_id: actor.id,
actor_type: actor.type
};
if (context && Object.keys(context).length) {
data.context = context;
}
return data;
}
});
};

View File

@ -60,7 +60,7 @@ async function editSingle(knex, table, id, options) {
if (options.transacting) {
k = k.transacting(options.transacting);
}
await k.where('id', id).update(options.data);
await k.where(options.column ?? 'id', id).update(options.data);
}
async function editMultiple(knex, table, chunk, options) {
@ -68,7 +68,7 @@ async function editMultiple(knex, table, chunk, options) {
if (options.transacting) {
k = k.transacting(options.transacting);
}
await k.whereIn('id', chunk).update(options.data);
await k.whereIn(options.column ?? 'id', chunk).update(options.data);
}
async function delSingle(knex, table, id, options) {
@ -112,23 +112,44 @@ module.exports = function (Bookshelf) {
return insert(Bookshelf.knex, tableName, data, options);
},
bulkEdit: async function bulkEdit(data, tableName, options = {}) {
/**
*
* @param {*} ids
* @param {*} tableName
* @param {object} options
* @param {object} [options.data] Data change you want to apply to the rows
* @param {string} [options.column] Update the rows where this column equals the ids (defaults to 'id')
* @returns
*/
bulkEdit: async function bulkEdit(ids, tableName, options = {}) {
tableName = tableName || this.prototype.tableName;
return await edit(Bookshelf.knex, tableName, data, options);
const result = await edit(Bookshelf.knex, tableName, ids, options);
if (result.successful > 0 && tableName === this.prototype.tableName) {
await this.addActions('edited', ids, options);
}
return result;
},
/**
*
* @param {string[]} data List of ids to delete
* @param {string[]} ids List of ids to delete
* @param {*} tableName
* @param {Object} [options]
* @param {string} [options.column] Delete the rows where this column equals the ids in `data` (defaults to 'id')
* @returns
*/
bulkDestroy: async function bulkDestroy(data, tableName, options = {}) {
bulkDestroy: async function bulkDestroy(ids, tableName, options = {}) {
tableName = tableName || this.prototype.tableName;
return await del(Bookshelf.knex, tableName, data, options);
if (tableName === this.prototype.tableName) {
// Needs to happen before, otherwise we cannot fetch the names of the deleted items
await this.addActions('deleted', ids, options);
}
return await del(Bookshelf.knex, tableName, ids, options);
}
});
};

View File

@ -1129,6 +1129,16 @@ Post = ghostBookshelf.Model.extend({
return filter;
}
}, {
getBulkActionExtraContext: function (options) {
if (options && options.filter && options.filter.includes('type:page')) {
return {
type: 'page'
};
}
return {
type: 'post'
};
},
allowedFormats: ['mobiledoc', 'lexical', 'html', 'plaintext'],
orderDefaultOptions: function orderDefaultOptions() {
@ -1236,9 +1246,11 @@ Post = ghostBookshelf.Model.extend({
* **See:** [ghostBookshelf.Model.findOne](base.js.html#Find%20One)
*/
findOne: function findOne(data = {}, options = {}) {
// @TODO: remove when we drop v0.1
if (!options.filter && !data.status) {
data.status = 'published';
if (!options.context || !options.context.internal) {
// @TODO: remove when we drop v0.1
if (!options.filter && !data.status) {
data.status = 'published';
}
}
if (data.status === 'all') {

View File

@ -70,13 +70,13 @@ class PostsService {
async bulkEdit(data, options) {
if (data.action === 'unpublish') {
return await this.#updatePosts({status: 'draft'}, {filter: this.#mergeFilters('status:published', options.filter)});
return await this.#updatePosts({status: 'draft'}, {filter: this.#mergeFilters('status:published', options.filter), context: options.context, actionName: 'unpublished'});
}
if (data.action === 'feature') {
return await this.#updatePosts({featured: true}, {filter: options.filter});
return await this.#updatePosts({featured: true}, {filter: options.filter, context: options.context, actionName: 'featured'});
}
if (data.action === 'unfeature') {
return await this.#updatePosts({featured: false}, {filter: options.filter});
return await this.#updatePosts({featured: false}, {filter: options.filter, context: options.context, actionName: 'unfeatured'});
}
if (data.action === 'access') {
if (!['public', 'members', 'paid', 'tiers'].includes(data.meta.visibility)) {
@ -93,7 +93,7 @@ class PostsService {
}
tiers = data.meta.tiers;
}
return await this.#updatePosts({visibility: data.meta.visibility, tiers}, {filter: options.filter});
return await this.#updatePosts({visibility: data.meta.visibility, tiers}, {filter: options.filter, context: options.context});
}
if (data.action === 'addTag') {
if (!Array.isArray(data.meta.tags)) {
@ -113,7 +113,7 @@ class PostsService {
});
}
}
return await this.#bulkAddTags({tags: data.meta.tags}, {filter: options.filter});
return await this.#bulkAddTags({tags: data.meta.tags}, {filter: options.filter, context: options.context});
}
throw new errors.IncorrectUsageError({
message: tpl(messages.unsupportedBulkAction)
@ -125,6 +125,8 @@ class PostsService {
* @param {string[]} data.tags - Array of tag ids to add to the post
* @param {object} options
* @param {string} options.filter - An NQL Filter
* @param {object} options.context
* @param {object} [options.transacting]
*/
async #bulkAddTags(data, options) {
if (!options.transacting) {
@ -139,7 +141,7 @@ class PostsService {
// Create tags that don't exist
for (const tag of data.tags) {
if (!tag.id) {
const createdTag = await this.models.Tag.add(tag, {transacting: options.transacting});
const createdTag = await this.models.Tag.add(tag, {transacting: options.transacting, context: options.context});
tag.id = createdTag.id;
}
}
@ -162,6 +164,7 @@ class PostsService {
}, []);
await options.transacting('posts_tags').insert(postTags);
await this.models.Post.addActions('edited', postRows.map(p => p.id), options);
return {
successful: postRows.length,
@ -236,7 +239,7 @@ class PostsService {
// Posts and emails
await this.models.Post.bulkDestroy(deleteEmailIds, 'emails', {transacting: options.transacting, throwErrors: true});
return await this.models.Post.bulkDestroy(deleteIds, 'posts', {transacting: options.transacting, throwErrors: true});
return await this.models.Post.bulkDestroy(deleteIds, 'posts', {...options, throwErrors: true});
}
async export(frame) {
@ -268,8 +271,8 @@ class PostsService {
}
const result = await this.models.Post.bulkEdit(editIds, 'posts', {
...options,
data,
transacting: options.transacting,
throwErrors: true
});