From e540344ef215bebba899d2160e95fbfe5cc27885 Mon Sep 17 00:00:00 2001 From: Simon Backx Date: Tue, 11 Oct 2022 16:32:28 +0200 Subject: [PATCH] Added audience feedback service and storage (#15584) 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. --- ghost/audience-feedback/.eslintrc.js | 6 + ghost/audience-feedback/README.md | 21 ++ ghost/audience-feedback/index.js | 1 + .../lib/AudienceFeedbackController.js | 85 ++++++ .../lib/AudienceFeedbackService.js | 8 + ghost/audience-feedback/lib/Feedback.js | 35 +++ .../lib/audience-feedback.js | 5 + ghost/audience-feedback/package.json | 29 ++ ghost/audience-feedback/test/.eslintrc.js | 6 + ghost/audience-feedback/test/hello.test.js | 10 + .../test/utils/assertions.js | 11 + ghost/audience-feedback/test/utils/index.js | 11 + .../audience-feedback/test/utils/overrides.js | 10 + ghost/core/core/boot.js | 4 +- .../server/api/endpoints/feedback-members.js | 23 ++ ghost/core/core/server/api/endpoints/index.js | 6 +- .../audience-feedback/FeedbackRepository.js | 67 +++++ .../services/audience-feedback/index.js | 28 ++ .../server/services/members/middleware.js | 40 +++ ghost/core/core/server/web/members/app.js | 12 + .../__snapshots__/feedback.test.js.snap | 265 ++++++++++++++++ .../test/e2e-api/members/feedback.test.js | 282 ++++++++++++++++++ 22 files changed, 963 insertions(+), 2 deletions(-) create mode 100644 ghost/audience-feedback/.eslintrc.js create mode 100644 ghost/audience-feedback/README.md create mode 100644 ghost/audience-feedback/index.js create mode 100644 ghost/audience-feedback/lib/AudienceFeedbackController.js create mode 100644 ghost/audience-feedback/lib/AudienceFeedbackService.js create mode 100644 ghost/audience-feedback/lib/Feedback.js create mode 100644 ghost/audience-feedback/lib/audience-feedback.js create mode 100644 ghost/audience-feedback/package.json create mode 100644 ghost/audience-feedback/test/.eslintrc.js create mode 100644 ghost/audience-feedback/test/hello.test.js create mode 100644 ghost/audience-feedback/test/utils/assertions.js create mode 100644 ghost/audience-feedback/test/utils/index.js create mode 100644 ghost/audience-feedback/test/utils/overrides.js create mode 100644 ghost/core/core/server/api/endpoints/feedback-members.js create mode 100644 ghost/core/core/server/services/audience-feedback/FeedbackRepository.js create mode 100644 ghost/core/core/server/services/audience-feedback/index.js create mode 100644 ghost/core/test/e2e-api/members/__snapshots__/feedback.test.js.snap create mode 100644 ghost/core/test/e2e-api/members/feedback.test.js diff --git a/ghost/audience-feedback/.eslintrc.js b/ghost/audience-feedback/.eslintrc.js new file mode 100644 index 0000000000..c9c1bcb522 --- /dev/null +++ b/ghost/audience-feedback/.eslintrc.js @@ -0,0 +1,6 @@ +module.exports = { + plugins: ['ghost'], + extends: [ + 'plugin:ghost/node' + ] +}; diff --git a/ghost/audience-feedback/README.md b/ghost/audience-feedback/README.md new file mode 100644 index 0000000000..0e850548bd --- /dev/null +++ b/ghost/audience-feedback/README.md @@ -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 + diff --git a/ghost/audience-feedback/index.js b/ghost/audience-feedback/index.js new file mode 100644 index 0000000000..d92ff20209 --- /dev/null +++ b/ghost/audience-feedback/index.js @@ -0,0 +1 @@ +module.exports = require('./lib/audience-feedback'); diff --git a/ghost/audience-feedback/lib/AudienceFeedbackController.js b/ghost/audience-feedback/lib/AudienceFeedbackController.js new file mode 100644 index 0000000000..9cf71a5e67 --- /dev/null +++ b/ghost/audience-feedback/lib/AudienceFeedbackController.js @@ -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} add + * @prop {(feedback: Feedback) => Promise} edit + * @prop {(postId, memberId) => Promise} get + * @prop {(id: string) => Promise} 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; diff --git a/ghost/audience-feedback/lib/AudienceFeedbackService.js b/ghost/audience-feedback/lib/AudienceFeedbackService.js new file mode 100644 index 0000000000..3836a31d45 --- /dev/null +++ b/ghost/audience-feedback/lib/AudienceFeedbackService.js @@ -0,0 +1,8 @@ +class AudienceFeedbackService { + buildLink() { + // todo + return new URL('https://example.com'); + } +} + +module.exports = AudienceFeedbackService; diff --git a/ghost/audience-feedback/lib/Feedback.js b/ghost/audience-feedback/lib/Feedback.js new file mode 100644 index 0000000000..0521b19e46 --- /dev/null +++ b/ghost/audience-feedback/lib/Feedback.js @@ -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; + } + } +}; diff --git a/ghost/audience-feedback/lib/audience-feedback.js b/ghost/audience-feedback/lib/audience-feedback.js new file mode 100644 index 0000000000..54638b2d02 --- /dev/null +++ b/ghost/audience-feedback/lib/audience-feedback.js @@ -0,0 +1,5 @@ +module.exports = { + AudienceFeedbackService: require('./AudienceFeedbackService'), + AudienceFeedbackController: require('./AudienceFeedbackController'), + Feedback: require('./Feedback') +}; diff --git a/ghost/audience-feedback/package.json b/ghost/audience-feedback/package.json new file mode 100644 index 0000000000..1eeb134933 --- /dev/null +++ b/ghost/audience-feedback/package.json @@ -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" + } +} diff --git a/ghost/audience-feedback/test/.eslintrc.js b/ghost/audience-feedback/test/.eslintrc.js new file mode 100644 index 0000000000..829b601eb0 --- /dev/null +++ b/ghost/audience-feedback/test/.eslintrc.js @@ -0,0 +1,6 @@ +module.exports = { + plugins: ['ghost'], + extends: [ + 'plugin:ghost/test' + ] +}; diff --git a/ghost/audience-feedback/test/hello.test.js b/ghost/audience-feedback/test/hello.test.js new file mode 100644 index 0000000000..85d69d1e08 --- /dev/null +++ b/ghost/audience-feedback/test/hello.test.js @@ -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'); + }); +}); diff --git a/ghost/audience-feedback/test/utils/assertions.js b/ghost/audience-feedback/test/utils/assertions.js new file mode 100644 index 0000000000..7364ee8aa1 --- /dev/null +++ b/ghost/audience-feedback/test/utils/assertions.js @@ -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; +// }); diff --git a/ghost/audience-feedback/test/utils/index.js b/ghost/audience-feedback/test/utils/index.js new file mode 100644 index 0000000000..0d67d86ff8 --- /dev/null +++ b/ghost/audience-feedback/test/utils/index.js @@ -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'); diff --git a/ghost/audience-feedback/test/utils/overrides.js b/ghost/audience-feedback/test/utils/overrides.js new file mode 100644 index 0000000000..90203424ee --- /dev/null +++ b/ghost/audience-feedback/test/utils/overrides.js @@ -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'); diff --git a/ghost/core/core/boot.js b/ghost/core/core/boot.js index 283b58a44e..5bd1ba3545 100644 --- a/ghost/core/core/boot.js +++ b/ghost/core/core/boot.js @@ -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'); @@ -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'); diff --git a/ghost/core/core/server/api/endpoints/feedback-members.js b/ghost/core/core/server/api/endpoints/feedback-members.js new file mode 100644 index 0000000000..ad77ae1e78 --- /dev/null +++ b/ghost/core/core/server/api/endpoints/feedback-members.js @@ -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); + } + } +}; diff --git a/ghost/core/core/server/api/endpoints/index.js b/ghost/core/core/server/api/endpoints/index.js index 01c8bae78e..941bd69969 100644 --- a/ghost/core/core/server/api/endpoints/index.js +++ b/ghost/core/core/server/api/endpoints/index.js @@ -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'); + } }; diff --git a/ghost/core/core/server/services/audience-feedback/FeedbackRepository.js b/ghost/core/core/server/services/audience-feedback/FeedbackRepository.js new file mode 100644 index 0000000000..5aa0d623dc --- /dev/null +++ b/ghost/core/core/server/services/audience-feedback/FeedbackRepository.js @@ -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}); + } +}; diff --git a/ghost/core/core/server/services/audience-feedback/index.js b/ghost/core/core/server/services/audience-feedback/index.js new file mode 100644 index 0000000000..a111db6e5a --- /dev/null +++ b/ghost/core/core/server/services/audience-feedback/index.js @@ -0,0 +1,28 @@ +const FeedbackRepository = require('./FeedbackRepository'); + +class AudienceFeedbackServiceWrapper { + async init() { + if (this.service) { + // Already done + return; + } + + // Wire up all the dependencies + const models = require('../../models'); + + const {AudienceFeedbackService, AudienceFeedbackController, Feedback} = require('@tryghost/audience-feedback'); + + this.repository = new FeedbackRepository({ + Member: models.Member, + MemberFeedback: models.MemberFeedback, + Feedback, + Post: models.Post + }); + + // Expose the service + this.service = new AudienceFeedbackService(); + this.controller = new AudienceFeedbackController({repository: this.repository}); + } +} + +module.exports = new AudienceFeedbackServiceWrapper(); diff --git a/ghost/core/core/server/services/members/middleware.js b/ghost/core/core/server/services/members/middleware.js index 2196de1cb5..85abf6883b 100644 --- a/ghost/core/core/server/services/members/middleware.js +++ b/ghost/core/core/server/services/members/middleware.js @@ -5,6 +5,13 @@ const models = require('../../models'); const urlUtils = require('../../../shared/url-utils'); const spamPrevention = require('../../web/shared/middleware/api/spam-prevention'); const {formattedMemberResponse} = require('./utils'); +const errors = require('@tryghost/errors'); +const tpl = require('@tryghost/tpl'); + +const messages = { + missingUuid: 'Missing uuid.', + invalidUuid: 'Invalid uuid.' +}; // @TODO: This piece of middleware actually belongs to the frontend, not to the member app // Need to figure a way to separate these things (e.g. frontend actually talks to members API) @@ -20,6 +27,38 @@ const loadMemberSession = async function (req, res, next) { } }; +/** + * Require member authentication, and make it possible to authenticate via uuid. + * You can chain this after loadMemberSession to make it possible to authetnicate via both the uuid and the session. + */ +const authMemberByUuid = async function (req, res, next) { + try { + if (res.locals.member && req.member) { + // Already authenticated via session + return next(); + } + + const uuid = req.query.uuid; + if (!uuid) { + throw new errors.UnauthorizedError({ + messsage: tpl(messages.missingUuid) + }); + } + + const member = await membersService.api.memberBREADService.read({uuid}); + if (!member) { + throw new errors.UnauthorizedError({ + message: tpl(messages.invalidUuid) + }); + } + Object.assign(req, {member}); + res.locals.member = req.member; + next(); + } catch (err) { + next(err); + } +}; + const getIdentityToken = async function (req, res) { try { const token = await membersService.ssr.getIdentityTokenForMemberFromSession(req, res); @@ -216,6 +255,7 @@ const createSessionFromMagicLink = async function (req, res, next) { // Set req.member & res.locals.member if a cookie is set module.exports = { loadMemberSession, + authMemberByUuid, createSessionFromMagicLink, getIdentityToken, getMemberNewsletters, diff --git a/ghost/core/core/server/web/members/app.js b/ghost/core/core/server/web/members/app.js index b2e11872e5..996573c23f 100644 --- a/ghost/core/core/server/web/members/app.js +++ b/ghost/core/core/server/web/members/app.js @@ -10,6 +10,8 @@ const shared = require('../shared'); const labs = require('../../../shared/labs'); const errorHandler = require('@tryghost/mw-error-handler'); const config = require('../../../shared/config'); +const {http} = require('@tryghost/api-framework'); +const api = require('../../api').endpoints; const commentRouter = require('../comments'); @@ -65,6 +67,16 @@ module.exports = function setupMembersApp() { // Comments membersApp.use('/api/comments', commentRouter()); + // Feedback + membersApp.post( + '/api/feedback', + labs.enabledMiddleware('audienceFeedback'), + bodyParser.json({limit: '50mb'}), + middleware.loadMemberSession, + middleware.authMemberByUuid, + http(api.feedbackMembers.add) + ); + // API error handling membersApp.use('/api', errorHandler.resourceNotFound); membersApp.use('/api', errorHandler.handleJSONResponse(sentry)); diff --git a/ghost/core/test/e2e-api/members/__snapshots__/feedback.test.js.snap b/ghost/core/test/e2e-api/members/__snapshots__/feedback.test.js.snap new file mode 100644 index 0000000000..d9487d896b --- /dev/null +++ b/ghost/core/test/e2e-api/members/__snapshots__/feedback.test.js.snap @@ -0,0 +1,265 @@ +// Jest Snapshot v1, https://goo.gl/fbAQLP + +exports[`Members Feedback Authentication Allows authentication via session 1: [body] 1`] = ` +Object { + "feedback": Array [ + Object { + "id": StringMatching /\\[a-f0-9\\]\\{24\\}/, + "memberId": StringMatching /\\[a-f0-9\\]\\{24\\}/, + "postId": StringMatching /\\[a-f0-9\\]\\{24\\}/, + "score": 1, + }, + ], +} +`; + +exports[`Members Feedback Authentication Allows authentication via session 2: [headers] 1`] = ` +Object { + "access-control-allow-origin": "*", + "cache-control": "no-cache, private, no-store, must-revalidate, max-stale=0, post-check=0, pre-check=0", + "content-length": "132", + "content-type": "application/json; charset=utf-8", + "etag": StringMatching /\\(\\?:W\\\\/\\)\\?"\\(\\?:\\[ !#-\\\\x7E\\\\x80-\\\\xFF\\]\\*\\|\\\\r\\\\n\\[\\\\t \\]\\|\\\\\\\\\\.\\)\\*"/, + "location": StringMatching /https\\?:\\\\/\\\\/\\.\\*\\?\\\\/feedback\\\\/\\[a-f0-9\\]\\{24\\}\\\\//, + "vary": "Accept-Encoding", + "x-powered-by": "Express", +} +`; + +exports[`Members Feedback Can add feedback 1: [body] 1`] = ` +Object { + "feedback": Array [ + Object { + "id": StringMatching /\\[a-f0-9\\]\\{24\\}/, + "memberId": StringMatching /\\[a-f0-9\\]\\{24\\}/, + "postId": StringMatching /\\[a-f0-9\\]\\{24\\}/, + "type": "positive", + }, + ], +} +`; + +exports[`Members Feedback Can add feedback 2: [headers] 1`] = ` +Object { + "access-control-allow-origin": "*", + "cache-control": "no-cache, private, no-store, must-revalidate, max-stale=0, post-check=0, pre-check=0", + "content-length": "140", + "content-type": "application/json; charset=utf-8", + "etag": StringMatching /\\(\\?:W\\\\/\\)\\?"\\(\\?:\\[ !#-\\\\x7E\\\\x80-\\\\xFF\\]\\*\\|\\\\r\\\\n\\[\\\\t \\]\\|\\\\\\\\\\.\\)\\*"/, + "location": StringMatching /https\\?:\\\\/\\\\/\\.\\*\\?\\\\/feedback\\\\/\\[a-f0-9\\]\\{24\\}\\\\//, + "vary": "Accept-Encoding", + "x-powered-by": "Express", +} +`; + +exports[`Members Feedback Can add positive feedback 1: [body] 1`] = ` +Object { + "feedback": Array [ + Object { + "id": StringMatching /\\[a-f0-9\\]\\{24\\}/, + "memberId": StringMatching /\\[a-f0-9\\]\\{24\\}/, + "postId": StringMatching /\\[a-f0-9\\]\\{24\\}/, + "score": 1, + }, + ], +} +`; + +exports[`Members Feedback Can add positive feedback 2: [headers] 1`] = ` +Object { + "access-control-allow-origin": "*", + "cache-control": "no-cache, private, no-store, must-revalidate, max-stale=0, post-check=0, pre-check=0", + "content-length": "132", + "content-type": "application/json; charset=utf-8", + "etag": StringMatching /\\(\\?:W\\\\/\\)\\?"\\(\\?:\\[ !#-\\\\x7E\\\\x80-\\\\xFF\\]\\*\\|\\\\r\\\\n\\[\\\\t \\]\\|\\\\\\\\\\.\\)\\*"/, + "location": StringMatching /https\\?:\\\\/\\\\/\\.\\*\\?\\\\/feedback\\\\/\\[a-f0-9\\]\\{24\\}\\\\//, + "vary": "Accept-Encoding", + "x-powered-by": "Express", +} +`; + +exports[`Members Feedback Can change existing feedback 1: [body] 1`] = ` +Object { + "feedback": Array [ + Object { + "id": StringMatching /\\[a-f0-9\\]\\{24\\}/, + "memberId": StringMatching /\\[a-f0-9\\]\\{24\\}/, + "postId": StringMatching /\\[a-f0-9\\]\\{24\\}/, + "score": 0, + }, + ], +} +`; + +exports[`Members Feedback Can change existing feedback 2: [headers] 1`] = ` +Object { + "access-control-allow-origin": "*", + "cache-control": "no-cache, private, no-store, must-revalidate, max-stale=0, post-check=0, pre-check=0", + "content-length": "132", + "content-type": "application/json; charset=utf-8", + "etag": StringMatching /\\(\\?:W\\\\/\\)\\?"\\(\\?:\\[ !#-\\\\x7E\\\\x80-\\\\xFF\\]\\*\\|\\\\r\\\\n\\[\\\\t \\]\\|\\\\\\\\\\.\\)\\*"/, + "location": StringMatching /https\\?:\\\\/\\\\/\\.\\*\\?\\\\/feedback\\\\/\\[a-f0-9\\]\\{24\\}\\\\//, + "vary": "Accept-Encoding", + "x-powered-by": "Express", +} +`; + +exports[`Members Feedback Can change existing feedback 3: [body] 1`] = ` +Object { + "feedback": Array [ + Object { + "id": StringMatching /\\[a-f0-9\\]\\{24\\}/, + "memberId": StringMatching /\\[a-f0-9\\]\\{24\\}/, + "postId": StringMatching /\\[a-f0-9\\]\\{24\\}/, + "score": 1, + }, + ], +} +`; + +exports[`Members Feedback Can change existing feedback 4: [headers] 1`] = ` +Object { + "access-control-allow-origin": "*", + "cache-control": "no-cache, private, no-store, must-revalidate, max-stale=0, post-check=0, pre-check=0", + "content-length": "132", + "content-type": "application/json; charset=utf-8", + "etag": StringMatching /\\(\\?:W\\\\/\\)\\?"\\(\\?:\\[ !#-\\\\x7E\\\\x80-\\\\xFF\\]\\*\\|\\\\r\\\\n\\[\\\\t \\]\\|\\\\\\\\\\.\\)\\*"/, + "location": StringMatching /https\\?:\\\\/\\\\/\\.\\*\\?\\\\/feedback\\\\/\\[a-f0-9\\]\\{24\\}\\\\//, + "vary": "Accept-Encoding", + "x-powered-by": "Express", +} +`; + +exports[`Members Feedback Can change existing feedback 5: [body] 1`] = ` +Object { + "feedback": Array [ + Object { + "id": StringMatching /\\[a-f0-9\\]\\{24\\}/, + "memberId": StringMatching /\\[a-f0-9\\]\\{24\\}/, + "postId": StringMatching /\\[a-f0-9\\]\\{24\\}/, + "score": 1, + }, + ], +} +`; + +exports[`Members Feedback Can change existing feedback 6: [headers] 1`] = ` +Object { + "access-control-allow-origin": "*", + "cache-control": "no-cache, private, no-store, must-revalidate, max-stale=0, post-check=0, pre-check=0", + "content-length": "132", + "content-type": "application/json; charset=utf-8", + "etag": StringMatching /\\(\\?:W\\\\/\\)\\?"\\(\\?:\\[ !#-\\\\x7E\\\\x80-\\\\xFF\\]\\*\\|\\\\r\\\\n\\[\\\\t \\]\\|\\\\\\\\\\.\\)\\*"/, + "location": StringMatching /https\\?:\\\\/\\\\/\\.\\*\\?\\\\/feedback\\\\/\\[a-f0-9\\]\\{24\\}\\\\//, + "vary": "Accept-Encoding", + "x-powered-by": "Express", +} +`; + +exports[`Members Feedback Validation Throws for invalid score 1: [body] 1`] = ` +Object { + "errors": Array [ + Object { + "code": null, + "context": "Invalid feedback score. Only 1 or 0 is currently allowed.", + "details": null, + "ghostErrorCode": null, + "help": null, + "id": StringMatching /\\[a-f0-9\\]\\{8\\}-\\[a-f0-9\\]\\{4\\}-\\[a-f0-9\\]\\{4\\}-\\[a-f0-9\\]\\{4\\}-\\[a-f0-9\\]\\{12\\}/, + "message": "Validation error, cannot save feedback.", + "property": null, + "type": "ValidationError", + }, + ], +} +`; + +exports[`Members Feedback Validation Throws for invalid score type 1: [body] 1`] = ` +Object { + "errors": Array [ + Object { + "code": null, + "context": "Invalid feedback score. Only 1 or 0 is currently allowed.", + "details": null, + "ghostErrorCode": null, + "help": null, + "id": StringMatching /\\[a-f0-9\\]\\{8\\}-\\[a-f0-9\\]\\{4\\}-\\[a-f0-9\\]\\{4\\}-\\[a-f0-9\\]\\{4\\}-\\[a-f0-9\\]\\{12\\}/, + "message": "Validation error, cannot save feedback.", + "property": null, + "type": "ValidationError", + }, + ], +} +`; + +exports[`Members Feedback Validation Throws for invalid type 1: [body] 1`] = ` +Object { + "errors": Array [ + Object { + "code": null, + "context": "Invalid feedback type", + "details": null, + "ghostErrorCode": null, + "help": null, + "id": StringMatching /\\[a-f0-9\\]\\{8\\}-\\[a-f0-9\\]\\{4\\}-\\[a-f0-9\\]\\{4\\}-\\[a-f0-9\\]\\{4\\}-\\[a-f0-9\\]\\{12\\}/, + "message": "Validation error, cannot save feedback.", + "property": null, + "type": "ValidationError", + }, + ], +} +`; + +exports[`Members Feedback Validation Throws for invalid uuid 1: [body] 1`] = ` +Object { + "errors": Array [ + Object { + "code": null, + "context": null, + "details": null, + "ghostErrorCode": null, + "help": null, + "id": StringMatching /\\[a-f0-9\\]\\{8\\}-\\[a-f0-9\\]\\{4\\}-\\[a-f0-9\\]\\{4\\}-\\[a-f0-9\\]\\{4\\}-\\[a-f0-9\\]\\{12\\}/, + "message": "Invalid uuid.", + "property": null, + "type": "UnauthorizedError", + }, + ], +} +`; + +exports[`Members Feedback Validation Throws for nonexisting post 1: [body] 1`] = ` +Object { + "errors": Array [ + Object { + "code": null, + "context": "Post not found.", + "details": null, + "ghostErrorCode": null, + "help": null, + "id": StringMatching /\\[a-f0-9\\]\\{8\\}-\\[a-f0-9\\]\\{4\\}-\\[a-f0-9\\]\\{4\\}-\\[a-f0-9\\]\\{4\\}-\\[a-f0-9\\]\\{12\\}/, + "message": "Resource not found error, cannot save feedback.", + "property": null, + "type": "NotFoundError", + }, + ], +} +`; + +exports[`Members Feedback Validation Throws for nonexisting uuid 1: [body] 1`] = ` +Object { + "errors": Array [ + Object { + "code": null, + "context": null, + "details": null, + "ghostErrorCode": null, + "help": null, + "id": StringMatching /\\[a-f0-9\\]\\{8\\}-\\[a-f0-9\\]\\{4\\}-\\[a-f0-9\\]\\{4\\}-\\[a-f0-9\\]\\{4\\}-\\[a-f0-9\\]\\{12\\}/, + "message": "Invalid uuid.", + "property": null, + "type": "UnauthorizedError", + }, + ], +} +`; diff --git a/ghost/core/test/e2e-api/members/feedback.test.js b/ghost/core/test/e2e-api/members/feedback.test.js new file mode 100644 index 0000000000..c97834b65f --- /dev/null +++ b/ghost/core/test/e2e-api/members/feedback.test.js @@ -0,0 +1,282 @@ +const assert = require('assert'); +const {agentProvider, mockManager, fixtureManager, matchers, configUtils} = require('../../utils/e2e-framework'); +const {anyEtag, anyObjectId, anyLocationFor, anyErrorId} = matchers; +const models = require('../../../core/server/models'); +const sinon = require('sinon'); + +describe('Members Feedback', function () { + let membersAgent, membersAgent2, memberUuid; + let clock; + + before(async function () { + membersAgent = await agentProvider.getMembersAPIAgent(); + membersAgent2 = await agentProvider.getMembersAPIAgent(); + + await fixtureManager.init('posts', 'members'); + memberUuid = fixtureManager.get('members', 0).uuid; + }); + + beforeEach(function () { + mockManager.mockMail(); + }); + + afterEach(function () { + clock?.restore(); + clock = undefined; + configUtils.restore(); + mockManager.restore(); + }); + + describe('Authentication', function () { + it('Allows authentication via session', async function () { + const postId = fixtureManager.get('posts', 0).id; + await membersAgent2.loginAs('authenticationtest@email.com'); + + await membersAgent2 + .post('/api/feedback/') + .body({ + feedback: [{ + score: 1, + post_id: postId + }] + }) + .expectStatus(201) + .matchHeaderSnapshot({ + etag: anyEtag, + location: anyLocationFor('feedback') + }) + .matchBodySnapshot({ + feedback: [ + { + id: anyObjectId, + memberId: anyObjectId, + postId: anyObjectId + } + ] + }); + }); + }); + + describe('Validation', function () { + const postId = fixtureManager.get('posts', 0).id; + + it('Throws for invalid score', async function () { + await membersAgent + .post(`/api/feedback/?uuid=${memberUuid}`) + .body({ + feedback: [{ + score: 2, + post_id: postId + }] + }) + .expectStatus(422) + .matchBodySnapshot({ + errors: [ + { + id: anyErrorId + } + ] + }); + }); + + it('Throws for invalid score type', async function () { + await membersAgent + .post(`/api/feedback/?uuid=${memberUuid}`) + .body({ + feedback: [{ + score: 'text', + post_id: postId + }] + }) + .expectStatus(422) + .matchBodySnapshot({ + errors: [ + { + id: anyErrorId + } + ] + }); + }); + + it('Throws for invalid uuid', async function () { + await membersAgent + .post(`/api/feedback/?uuid=1234`) + .body({ + feedback: [{ + score: 1, + post_id: postId + }] + }) + .expectStatus(401) + .matchBodySnapshot({ + errors: [ + { + id: anyErrorId + } + ] + }); + }); + + it('Throws for nonexisting uuid', async function () { + const uuid = '00000000-0000-0000-0000-000000000000'; + await membersAgent + .post(`/api/feedback/?uuid=${uuid}`) + .body({ + feedback: [{ + score: 1, + post_id: postId + }] + }) + .expectStatus(401) + .matchBodySnapshot({ + errors: [ + { + id: anyErrorId + } + ] + }); + }); + + it('Throws for nonexisting post', async function () { + await membersAgent + .post(`/api/feedback/?uuid=${memberUuid}`) + .body({ + feedback: [{ + score: 1, + post_id: '123' + }] + }) + .expectStatus(404) + .matchBodySnapshot({ + errors: [ + { + id: anyErrorId + } + ] + }); + }); + }); + + it('Can add positive feedback', async function () { + const postId = fixtureManager.get('posts', 0).id; + + await membersAgent + .post(`/api/feedback/?uuid=${memberUuid}`) + .body({ + feedback: [{ + score: 1, + post_id: postId + }] + }) + .expectStatus(201) + .matchHeaderSnapshot({ + etag: anyEtag, + location: anyLocationFor('feedback') + }) + .matchBodySnapshot({ + feedback: [ + { + id: anyObjectId, + memberId: anyObjectId, + postId: anyObjectId + } + ] + }); + }); + + it('Can change existing feedback', async function () { + clock = sinon.useFakeTimers(new Date()); + const postId = fixtureManager.get('posts', 1).id; + + const {body} = await membersAgent + .post(`/api/feedback/?uuid=${memberUuid}`) + .body({ + feedback: [{ + score: 0, + post_id: postId + }] + }) + .expectStatus(201) + .matchHeaderSnapshot({ + etag: anyEtag, + location: anyLocationFor('feedback') + }) + .matchBodySnapshot({ + feedback: [ + { + id: anyObjectId, + memberId: anyObjectId, + postId: anyObjectId + } + ] + }); + + assert.equal(body.feedback[0].score, 0); + const feedbackId = body.feedback[0].id; + + // Fetch real model + const model1 = await models.MemberFeedback.findOne({id: feedbackId}, {require: true}); + + clock.tick(10 * 60 * 1000); + + const {body: body2} = await membersAgent + .post(`/api/feedback/?uuid=${memberUuid}`) + .body({ + feedback: [{ + score: 1, + post_id: postId + }] + }) + .expectStatus(201) + .matchHeaderSnapshot({ + etag: anyEtag, + location: anyLocationFor('feedback') + }) + .matchBodySnapshot({ + feedback: [ + { + id: anyObjectId, + memberId: anyObjectId, + postId: anyObjectId + } + ] + }); + + assert.equal(body2.feedback[0].id, feedbackId); + assert.equal(body2.feedback[0].score, 1); + const model2 = await models.MemberFeedback.findOne({id: feedbackId}, {require: true}); + + assert.notEqual(model2.get('updated_at').getTime(), model1.get('updated_at').getTime()); + + clock.tick(10 * 60 * 1000); + + // Do the same change again, and the model shouldn't change + const {body: body3} = await membersAgent + .post(`/api/feedback/?uuid=${memberUuid}`) + .body({ + feedback: [{ + score: 1, + post_id: postId + }] + }) + .expectStatus(201) + .matchHeaderSnapshot({ + etag: anyEtag, + location: anyLocationFor('feedback') + }) + .matchBodySnapshot({ + feedback: [ + { + id: anyObjectId, + memberId: anyObjectId, + postId: anyObjectId + } + ] + }); + + const model3 = await models.MemberFeedback.findOne({id: feedbackId}, {require: true}); + assert.equal(body3.feedback[0].id, feedbackId); + assert.equal(body3.feedback[0].score, 1); + assert.equal(model3.get('updated_at').getTime(), model2.get('updated_at').getTime()); + }); +});