Skip to content

Commit

Permalink
Added audience feedback service and storage (TryGhost#15584)
Browse files Browse the repository at this point in the history
fixes https://github.com/TryGhost/Team/issues/2049
fixes https://github.com/TryGhost/Team/issues/2053

- This adds a new audience feedback package to Ghost. 
- A new members API to give feedback on posts using the `/api/feedback` endpoint.
- Added a new authentication middleware that supports both uuid-based and session based authentication.
  • Loading branch information
SimonBackx authored Oct 11, 2022
1 parent 6ff34fb commit e540344
Show file tree
Hide file tree
Showing 22 changed files with 963 additions and 2 deletions.
6 changes: 6 additions & 0 deletions ghost/audience-feedback/.eslintrc.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
module.exports = {
plugins: ['ghost'],
extends: [
'plugin:ghost/node'
]
};
21 changes: 21 additions & 0 deletions ghost/audience-feedback/README.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,21 @@
# Audience Feedback


## Usage


## Develop

This is a monorepo package.

Follow the instructions for the top-level repo.
1. `git clone` this repo & `cd` into it as usual
2. Run `yarn` to install top-level dependencies.



## Test

- `yarn lint` run just eslint
- `yarn test` run lint and tests

1 change: 1 addition & 0 deletions ghost/audience-feedback/index.js
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
module.exports = require('./lib/audience-feedback');
85 changes: 85 additions & 0 deletions ghost/audience-feedback/lib/AudienceFeedbackController.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,85 @@
const Feedback = require('./Feedback');
const errors = require('@tryghost/errors');
const tpl = require('@tryghost/tpl');

const messages = {
invalidScore: 'Invalid feedback score. Only 1 or 0 is currently allowed.',
postNotFound: 'Post not found.',
memberNotFound: 'Member not found.'
};

/**
* @typedef {object} IFeedbackRepository
* @prop {(feedback: Feedback) => Promise<void>} add
* @prop {(feedback: Feedback) => Promise<void>} edit
* @prop {(postId, memberId) => Promise<Feedback>} get
* @prop {(id: string) => Promise<Post|undefined>} getPostById
*/

class AudienceFeedbackController {
/** @type IFeedbackRepository */
#repository;

/**
* @param {object} deps
* @param {IFeedbackRepository} deps.repository
*/
constructor(deps) {
this.#repository = deps.repository;
}

/**
* Get member from frame
*/
#getMember(frame) {
if (!frame.options?.context?.member?.id) {
// This is an internal server error because authentication should happen outside this service.
throw new errors.InternalServerError({
message: tpl(messages.memberNotFound)
});
}
return frame.options.context.member;
}

async add(frame) {
const data = frame.data.feedback[0];
const postId = data.post_id;
const score = data.score;

if (![0, 1].includes(score)) {
throw new errors.ValidationError({
message: tpl(messages.invalidScore)
});
}

const member = this.#getMember(frame);

const post = await this.#repository.getPostById(postId);
if (!post) {
throw new errors.NotFoundError({
message: tpl(messages.postNotFound)
});
}

const existing = await this.#repository.get(post.id, member.id);
if (existing) {
if (existing.score === score) {
// Don't save so we don't update the updated_at timestamp
return existing;
}
existing.score = score;
await this.#repository.edit(existing);
return existing;
}

const feedback = new Feedback({
memberId: member.id,
postId: post.id,
score
});
await this.#repository.add(feedback);
return feedback;
}
}

module.exports = AudienceFeedbackController;
8 changes: 8 additions & 0 deletions ghost/audience-feedback/lib/AudienceFeedbackService.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
class AudienceFeedbackService {
buildLink() {
// todo
return new URL('https://example.com');
}
}

module.exports = AudienceFeedbackService;
35 changes: 35 additions & 0 deletions ghost/audience-feedback/lib/Feedback.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,35 @@
const ObjectID = require('bson-objectid').default;

module.exports = class Feedback {
/** @type {ObjectID} */
id;
/** @type {number} */
score;
/** @type {ObjectID} */
memberId;
/** @type {ObjectID} */
postId;

constructor(data) {
if (!data.id) {
this.id = new ObjectID();
}

if (typeof data.id === 'string') {
this.id = ObjectID.createFromHexString(data.id);
}

this.score = data.score ?? 0;
if (typeof data.memberId === 'string') {
this.memberId = ObjectID.createFromHexString(data.memberId);
} else {
this.memberId = data.memberId;
}

if (typeof data.postId === 'string') {
this.postId = ObjectID.createFromHexString(data.postId);
} else {
this.postId = data.postId;
}
}
};
5 changes: 5 additions & 0 deletions ghost/audience-feedback/lib/audience-feedback.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
module.exports = {
AudienceFeedbackService: require('./AudienceFeedbackService'),
AudienceFeedbackController: require('./AudienceFeedbackController'),
Feedback: require('./Feedback')
};
29 changes: 29 additions & 0 deletions ghost/audience-feedback/package.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,29 @@
{
"name": "@tryghost/audience-feedback",
"version": "0.0.0",
"repository": "https://github.com/TryGhost/Ghost/tree/main/packages/audience-feedback",
"author": "Ghost Foundation",
"private": true,
"main": "index.js",
"scripts": {
"dev": "echo \"Implement me!\"",
"test:unit": "NODE_ENV=testing c8 --all --reporter text --reporter cobertura mocha './test/**/*.test.js'",
"test": "yarn test:unit",
"lint:code": "eslint *.js lib/ --ext .js --cache",
"lint": "yarn lint:code && yarn lint:test",
"lint:test": "eslint -c test/.eslintrc.js test/ --ext .js --cache"
},
"files": [
"index.js",
"lib"
],
"devDependencies": {
"c8": "7.12.0",
"mocha": "10.0.0",
"should": "13.2.3",
"sinon": "14.0.1"
},
"dependencies": {
"@tryghost/errors": "1.2.17"
}
}
6 changes: 6 additions & 0 deletions ghost/audience-feedback/test/.eslintrc.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
module.exports = {
plugins: ['ghost'],
extends: [
'plugin:ghost/test'
]
};
10 changes: 10 additions & 0 deletions ghost/audience-feedback/test/hello.test.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
// Switch these lines once there are useful utils
// const testUtils = require('./utils');
require('./utils');

describe('Hello world', function () {
it('Runs a test', function () {
// TODO: Write me!
'hello'.should.eql('hello');
});
});
11 changes: 11 additions & 0 deletions ghost/audience-feedback/test/utils/assertions.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
/**
* Custom Should Assertions
*
* Add any custom assertions to this file.
*/

// Example Assertion
// should.Assertion.add('ExampleAssertion', function () {
// this.params = {operator: 'to be a valid Example Assertion'};
// this.obj.should.be.an.Object;
// });
11 changes: 11 additions & 0 deletions ghost/audience-feedback/test/utils/index.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
/**
* Test Utilities
*
* Shared utils for writing tests
*/

// Require overrides - these add globals for tests
require('./overrides');

// Require assertions - adds custom should assertions
require('./assertions');
10 changes: 10 additions & 0 deletions ghost/audience-feedback/test/utils/overrides.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
// This file is required before any test is run

// Taken from the should wiki, this is how to make should global
// Should is a global in our eslint test config
global.should = require('should').noConflict();
should.extend();

// Sinon is a simple case
// Sinon is a global in our eslint test config
global.sinon = require('sinon');
4 changes: 3 additions & 1 deletion ghost/core/core/boot.js
Original file line number Diff line number Diff line change
Expand Up @@ -288,6 +288,7 @@ async function initServices({config}) {
const memberAttribution = require('./server/services/member-attribution');
const membersEvents = require('./server/services/members-events');
const linkTracking = require('./server/services/link-tracking');
const audienceFeedback = require('./server/services/audience-feedback');

const urlUtils = require('./shared/url-utils');

Expand Down Expand Up @@ -315,7 +316,8 @@ async function initServices({config}) {
apiUrl: urlUtils.urlFor('api', {type: 'admin'}, true)
}),
comments.init(),
linkTracking.init()
linkTracking.init(),
audienceFeedback.init()
]);
debug('End: Services');

Expand Down
23 changes: 23 additions & 0 deletions ghost/core/core/server/api/endpoints/feedback-members.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,23 @@
const feedbackService = require('../../services/audience-feedback');

module.exports = {
docName: 'feedback',

add: {
statusCode: 201,
validation: {
data: {
post_id: {
required: true
},
score: {
required: true
}
}
},
permissions: false,
query(frame) {
return feedbackService.controller.add(frame);
}
}
};
6 changes: 5 additions & 1 deletion ghost/core/core/server/api/endpoints/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -231,5 +231,9 @@ module.exports = {

get commentsMembers() {
return apiFramework.pipeline(require('./comments-members'), localUtils, 'members');
}
},

get feedbackMembers() {
return apiFramework.pipeline(require('./feedback-members'), localUtils, 'members');
}
};
Original file line number Diff line number Diff line change
@@ -0,0 +1,67 @@
module.exports = class FeedbackRepository {
/** @type {object} */
#Member;

/** @type {object} */
#Post;

/** @type {object} */
#MemberFeedback;

/** @type {typeof Object} */
#Feedback;

/**
* @param {object} deps
* @param {object} deps.Member Bookshelf Model
* @param {object} deps.Post Bookshelf Model
* @param {object} deps.MemberFeedback Bookshelf Model
* @param {object} deps.Feedback Feedback object
*/
constructor(deps) {
this.#Member = deps.Member;
this.#Post = deps.Post;
this.#MemberFeedback = deps.MemberFeedback;
this.#Feedback = deps.Feedback;
}

async add(feedback) {
await this.#MemberFeedback.add({
id: feedback.id.toHexString(),
member_id: feedback.memberId.toHexString(),
post_id: feedback.postId.toHexString(),
score: feedback.score
});
}

async edit(feedback) {
await this.#MemberFeedback.edit({
score: feedback.score
}, {
id: feedback.id.toHexString()
});
}

async get(postId, memberId) {
const model = await this.#MemberFeedback.findOne({member_id: memberId, post_id: postId}, {require: false});

if (!model) {
return;
}

return new this.#Feedback({
id: model.id,
memberId: model.get('member_id'),
postId: model.get('post_id'),
score: model.get('score')
});
}

async getMemberByUuid(uuid) {
return await this.#Member.findOne({uuid});
}

async getPostById(id) {
return await this.#Post.findOne({id});
}
};
Loading

0 comments on commit e540344

Please sign in to comment.