From 9f33285b030e8ceeba24ae35fcb10beaee17274f Mon Sep 17 00:00:00 2001 From: Fredrik Strand Oseberg Date: Mon, 7 Jun 2021 11:11:42 +0200 Subject: [PATCH] Feat/pnps feedback (#862) * feat: setup user feedback service * fix: map rows * feat: add tests * wrap service calls in try catch * fix: add test for retrieving feedback on user * feat: add fake user feedback store * fix: check ffor feedback id in controller * feat: add test for bad request --- src/lib/db/index.ts | 4 +- src/lib/db/user-feedback-store.ts | 86 +++++++++++++++++ src/lib/routes/admin-api/index.ts | 5 + .../admin-api/user-feedback-controller.ts | 93 +++++++++++++++++++ src/lib/routes/admin-api/user.ts | 24 +++-- src/lib/services/index.ts | 3 + src/lib/services/user-feedback-service.ts | 35 +++++++ src/lib/types/services.ts | 2 + src/lib/types/stores.ts | 2 + .../20210602115555-create-feedback-table.js | 26 ++++++ src/test/e2e/api/admin/feedback.e2e.test.ts | 90 ++++++++++++++++++ src/test/fixtures/fake-user-feedback-store.ts | 5 + src/test/fixtures/store.js | 2 + 13 files changed, 370 insertions(+), 7 deletions(-) create mode 100644 src/lib/db/user-feedback-store.ts create mode 100644 src/lib/routes/admin-api/user-feedback-controller.ts create mode 100644 src/lib/services/user-feedback-service.ts create mode 100644 src/migrations/20210602115555-create-feedback-table.js create mode 100644 src/test/e2e/api/admin/feedback.e2e.test.ts create mode 100644 src/test/fixtures/fake-user-feedback-store.ts diff --git a/src/lib/db/index.ts b/src/lib/db/index.ts index 472d27e6da..40d02ffc41 100644 --- a/src/lib/db/index.ts +++ b/src/lib/db/index.ts @@ -1,7 +1,7 @@ 'use strict'; // eslint-disable-next-line -import EventEmitter from "events"; +import EventEmitter from 'events'; import { IUnleashConfig } from '../types/option'; import { IUnleashStores } from '../types/stores'; @@ -25,6 +25,7 @@ import { ApiTokenStore } from './api-token-store'; import SessionStore from './session-store'; import { AccessStore } from './access-store'; import { ResetTokenStore } from './reset-token-store'; +import UserFeedbackStore from './user-feedback-store'; export const createStores = ( config: IUnleashConfig, @@ -59,6 +60,7 @@ export const createStores = ( apiTokenStore: new ApiTokenStore(db, eventBus, getLogger), resetTokenStore: new ResetTokenStore(db, eventBus, getLogger), sessionStore: new SessionStore(db, eventBus, getLogger), + userFeedbackStore: new UserFeedbackStore(db, eventBus, getLogger), }; }; diff --git a/src/lib/db/user-feedback-store.ts b/src/lib/db/user-feedback-store.ts new file mode 100644 index 0000000000..e190530cbe --- /dev/null +++ b/src/lib/db/user-feedback-store.ts @@ -0,0 +1,86 @@ +'use strict'; + +import { Knex } from 'knex'; +import { EventEmitter } from 'events'; +import { LogProvider, Logger } from '../logger'; + +const COLUMNS = ['given', 'user_id', 'feedback_id', 'nevershow']; +const TABLE = 'user_feedback'; + +interface IUserFeedbackTable { + nevershow?: boolean; + feedback_id: string; + given?: Date; + user_id: number; +} + +export interface IUserFeedback { + neverShow: boolean; + feedbackId: string; + given?: Date; + userId: number; +} + +const fieldToRow = (fields: IUserFeedback): IUserFeedbackTable => { + return { + nevershow: fields.neverShow, + feedback_id: fields.feedbackId, + given: fields.given, + user_id: fields.userId, + }; +}; + +const rowToField = (row: IUserFeedbackTable): IUserFeedback => { + return { + neverShow: row.nevershow, + feedbackId: row.feedback_id, + given: row.given, + userId: row.user_id, + }; +}; + +export default class UserFeedbackStore { + private db: Knex; + + private logger: Logger; + + constructor(db: Knex, eventBus: EventEmitter, getLogger: LogProvider) { + this.db = db; + this.logger = getLogger('user-feedback-store.js'); + } + + async getAllUserFeedback(userId: number): Promise { + const userFeedback = await this.db + .table(TABLE) + .select() + .where({ user_id: userId }); + + return userFeedback.map(rowToField); + } + + async getFeedback( + userId: number, + feedbackId: string, + ): Promise { + const userFeedback = await this.db + .table(TABLE) + .select() + .where({ user_id: userId, feedback_id: feedbackId }) + .first(); + + return rowToField(userFeedback); + } + + async updateFeedback(feedback: IUserFeedback): Promise { + const insertedFeedback = await this.db + .table(TABLE) + .insert(fieldToRow(feedback)) + .onConflict(['user_id', 'feedback_id']) + .merge() + .returning(COLUMNS); + + return rowToField(insertedFeedback[0]); + } +} + +module.exports = UserFeedbackStore; diff --git a/src/lib/routes/admin-api/index.ts b/src/lib/routes/admin-api/index.ts index e54c23f2c2..f7da4de32c 100644 --- a/src/lib/routes/admin-api/index.ts +++ b/src/lib/routes/admin-api/index.ts @@ -19,6 +19,7 @@ import AddonController from './addon'; import ApiTokenController from './api-token-controller'; import UserAdminController from './user-admin'; import EmailController from './email'; +import UserFeedbackController from './user-feedback-controller'; class AdminApi extends Controller { constructor(config: IUnleashConfig, services: IUnleashServices) { @@ -75,6 +76,10 @@ class AdminApi extends Controller { '/user-admin', new UserAdminController(config, services).router, ); + this.app.use( + '/feedback', + new UserFeedbackController(config, services).router, + ); } index(req, res) { diff --git a/src/lib/routes/admin-api/user-feedback-controller.ts b/src/lib/routes/admin-api/user-feedback-controller.ts new file mode 100644 index 0000000000..322c4227c1 --- /dev/null +++ b/src/lib/routes/admin-api/user-feedback-controller.ts @@ -0,0 +1,93 @@ +import { Response } from 'express'; + +import Controller from '../controller'; +import { Logger } from '../../logger'; +import { IUserRequest } from './user'; +import { IUnleashConfig } from '../../types/option'; +import { IUnleashServices } from '../../types/services'; +import UserFeedbackService from '../../services/user-feedback-service'; +import { handleErrors } from './util'; + +interface IFeedbackBody { + neverShow?: boolean; + feedbackId: string; + given?: Date; +} + +class UserFeedbackController extends Controller { + private logger: Logger; + + private userFeedbackService: UserFeedbackService; + + constructor( + config: IUnleashConfig, + { userFeedbackService }: Pick, + ) { + super(config); + this.logger = config.getLogger('feedback-controller.ts'); + this.userFeedbackService = userFeedbackService; + + this.post('/', this.recordFeedback); + this.put('/:id', this.updateFeedbackSettings); + } + + private async recordFeedback( + req: IUserRequest, + res: Response, + ): Promise { + const BAD_REQUEST = 400; + const { user } = req; + + const { feedbackId } = req.body; + + if (!feedbackId) { + res.status(BAD_REQUEST).json({ + error: 'feedbackId must be present.', + }); + return; + } + + const feedback = { + ...req.body, + userId: user.id, + given: new Date(), + neverShow: req.body.neverShow || false, + }; + + try { + const updated = await this.userFeedbackService.updateFeedback( + feedback, + ); + res.json(updated); + } catch (e) { + handleErrors(res, this.logger, e); + } + } + + private async updateFeedbackSettings( + req: IUserRequest, + res: Response, + ): Promise { + const { user } = req; + const { id } = req.params; + + const feedback = { + ...req.body, + feedbackId: id, + userId: user.id, + neverShow: req.body.neverShow || false, + }; + + try { + const updated = await this.userFeedbackService.updateFeedback( + feedback, + ); + res.json(updated); + } catch (e) { + handleErrors(res, this.logger, e); + } + } +} + +module.exports = UserFeedbackController; +export default UserFeedbackController; diff --git a/src/lib/routes/admin-api/user.ts b/src/lib/routes/admin-api/user.ts index 4715b75f14..5c8642044d 100644 --- a/src/lib/routes/admin-api/user.ts +++ b/src/lib/routes/admin-api/user.ts @@ -11,13 +11,14 @@ import User from '../../types/user'; import { Logger } from '../../logger'; import { handleErrors } from './util'; import SessionService from '../../services/session-service'; +import UserFeedbackService from '../../services/user-feedback-service'; interface IChangeUserRequest { password: string; confirmPassword: string; } -interface UserRequest +export interface IUserRequest extends Request { user: User; } @@ -27,6 +28,8 @@ class UserController extends Controller { private userService: UserService; + private userFeedbackService: UserFeedbackService; + private sessionService: SessionService; private logger: Logger; @@ -37,15 +40,20 @@ class UserController extends Controller { accessService, userService, sessionService, + userFeedbackService, }: Pick< - IUnleashServices, - 'accessService' | 'userService' | 'sessionService' + IUnleashServices, + | 'accessService' + | 'userService' + | 'sessionService' + | 'userFeedbackService' >, ) { super(config); this.accessService = accessService; this.userService = userService; this.sessionService = sessionService; + this.userFeedbackService = userFeedbackService; this.logger = config.getLogger('lib/routes/admin-api/user.ts'); this.get('/', this.getUser); @@ -60,17 +68,21 @@ class UserController extends Controller { const permissions = await this.accessService.getPermissionsForUser( user, ); + const feedback = await this.userFeedbackService.getAllUserFeedback( + user.id, + ); + delete user.permissions; // TODO: remove return res .status(200) - .json({ user, permissions }) + .json({ user, permissions, feedback }) .end(); } return res.status(404).end(); } async updateUserPass( - req: UserRequest, + req: IUserRequest, res: Response, ): Promise { const { user } = req; @@ -93,7 +105,7 @@ class UserController extends Controller { } async mySessions( - req: UserRequest, + req: IUserRequest, res: Response, ): Promise { const { user } = req; diff --git a/src/lib/services/index.ts b/src/lib/services/index.ts index 0dd3f46792..66fdb2f0e5 100644 --- a/src/lib/services/index.ts +++ b/src/lib/services/index.ts @@ -22,6 +22,7 @@ import UserService from './user-service'; import ResetTokenService from './reset-token-service'; import SettingService from './setting-service'; import SessionService from './session-service'; +import UserFeedbackService from './user-feedback-service'; export const createServices = ( stores: IUnleashStores, @@ -52,6 +53,7 @@ export const createServices = ( const versionService = new VersionService(stores, config); const healthService = new HealthService(stores, config); const settingService = new SettingService(stores, config); + const userFeedbackService = new UserFeedbackService(stores, config); return { accessService, @@ -74,6 +76,7 @@ export const createServices = ( eventService, settingService, sessionService, + userFeedbackService, }; }; diff --git a/src/lib/services/user-feedback-service.ts b/src/lib/services/user-feedback-service.ts new file mode 100644 index 0000000000..4e5d69e6ab --- /dev/null +++ b/src/lib/services/user-feedback-service.ts @@ -0,0 +1,35 @@ +import { Logger } from '../logger'; +import UserFeedbackStore, { IUserFeedback } from '../db/user-feedback-store'; +import { IUnleashStores } from '../types/stores'; +import { IUnleashConfig } from '../types/option'; + +export default class UserFeedbackService { + private userFeedbackStore: UserFeedbackStore; + + private logger: Logger; + + constructor( + { userFeedbackStore }: Pick, + { getLogger }: Pick, + ) { + this.userFeedbackStore = userFeedbackStore; + this.logger = getLogger('services/user-feedback-service.js'); + } + + async getAllUserFeedback(user_id: number): Promise { + return this.userFeedbackStore.getAllUserFeedback(user_id); + } + + async getFeedback( + user_id: number, + feedback_id: string, + ): Promise { + return this.userFeedbackStore.getFeedback(user_id, feedback_id); + } + + async updateFeedback(feedback: IUserFeedback): Promise { + return this.userFeedbackStore.updateFeedback(feedback); + } +} + +module.exports = UserFeedbackService; diff --git a/src/lib/types/services.ts b/src/lib/types/services.ts index 2bb0dc8584..1195034759 100644 --- a/src/lib/types/services.ts +++ b/src/lib/types/services.ts @@ -18,6 +18,7 @@ import EventService from '../services/event-service'; import HealthService from '../services/health-service'; import SettingService from '../services/setting-service'; import SessionService from '../services/session-service'; +import UserFeedbackService from '../services/user-feedback-service'; export interface IUnleashServices { accessService: AccessService; @@ -40,4 +41,5 @@ export interface IUnleashServices { tagService: TagService; userService: UserService; versionService: VersionService; + userFeedbackService: UserFeedbackService; } diff --git a/src/lib/types/stores.ts b/src/lib/types/stores.ts index e593a59305..7e7ab010e7 100644 --- a/src/lib/types/stores.ts +++ b/src/lib/types/stores.ts @@ -13,6 +13,7 @@ import UserStore from '../db/user-store'; import TagStore from '../db/tag-store'; import TagTypeStore from '../db/tag-type-store'; import AddonStore from '../db/addon-store'; +import UserFeedbackStore from '../db/user-feedback-store'; import { AccessStore } from '../db/access-store'; import { ApiTokenStore } from '../db/api-token-store'; import { ResetTokenStore } from '../db/reset-token-store'; @@ -37,5 +38,6 @@ export interface IUnleashStores { accessStore: AccessStore; apiTokenStore: ApiTokenStore; resetTokenStore: ResetTokenStore; + userFeedbackStore: UserFeedbackStore; db: Knex; } diff --git a/src/migrations/20210602115555-create-feedback-table.js b/src/migrations/20210602115555-create-feedback-table.js new file mode 100644 index 0000000000..c7152bacb7 --- /dev/null +++ b/src/migrations/20210602115555-create-feedback-table.js @@ -0,0 +1,26 @@ +'use strict'; + +exports.up = function(db, cb) { + db.runSql( + ` + CREATE TABLE IF NOT EXISTS user_feedback + (user_id INTEGER NOT NULL references users (id), + feedback_id TEXT, + given TIMESTAMP WITH TIME ZONE, + neverShow BOOLEAN NOT NULL DEFAULT false, + PRIMARY KEY (user_id, feedback_id)); + CREATE INDEX user_feedback_user_id_idx ON user_feedback (user_id); + `, + cb, + ); +}; + +exports.down = function(db, cb) { + db.runSql( + ` + DROP INDEX user_feedback_user_id_idx; + DROP TABLE user_feedback; + `, + cb, + ); +}; diff --git a/src/test/e2e/api/admin/feedback.e2e.test.ts b/src/test/e2e/api/admin/feedback.e2e.test.ts new file mode 100644 index 0000000000..79af385d42 --- /dev/null +++ b/src/test/e2e/api/admin/feedback.e2e.test.ts @@ -0,0 +1,90 @@ +import { setupAppWithCustomAuth } from '../../helpers/test-helper'; +import dbInit from '../../helpers/database-init'; +import getLogger from '../../../fixtures/no-logger'; +import { IUnleashConfig } from '../../../../lib/types/option'; +import { IUnleashServices } from '../../../../lib/types/services'; + +let stores; +let db; +let app; + +beforeAll(async () => { + db = await dbInit('feedback_api_serial', getLogger); + stores = db.stores; + + const email = 'custom-user@mail.com'; + + const preHook = ( + app: any, + config: IUnleashConfig, + { userService }: IUnleashServices, + ) => { + app.use('/api/admin/', async (req, res, next) => { + req.user = await userService.loginUserWithoutPassword(email, true); + next(); + }); + }; + + app = await setupAppWithCustomAuth(stores, preHook); +}); + +afterAll(async () => { + await app.destroy(); + await db.destroy(); +}); + +test('it creates feedback for user', async () => { + expect.assertions(1); + + return app.request + .post('/api/admin/feedback') + .send({ feedbackId: 'pnps' }) + .set('Content-Type', 'application/json') + .expect('Content-Type', /json/) + .expect(200) + .expect(res => { + expect(res.body.feedbackId).toBe('pnps'); + }); +}); + +test('it gives 400 when feedback is not present', async () => { + expect.assertions(1); + + return app.request + .post('/api/admin/feedback') + .send({}) + .set('Content-Type', 'application/json') + .expect('Content-Type', /json/) + .expect(400) + .expect(res => { + expect(res.body.error).toBeTruthy(); + }); +}); + +test('it updates feedback for user', async () => { + expect.assertions(1); + + return app.request + .put('/api/admin/feedback/pnps') + .send({ neverShow: true }) + .set('Content-Type', 'application/json') + .expect('Content-Type', /json/) + .expect(200) + .expect(res => { + expect(res.body.neverShow).toBe(true); + }); +}); + +test('it retrieves feedback for user', async () => { + expect.assertions(2); + + return app.request + .get('/api/admin/user') + .set('Content-Type', 'application/json') + .expect('Content-Type', /json/) + .expect(200) + .expect(res => { + expect(res.body.feedback.length).toBe(1); + expect(res.body.feedback[0].feedbackId).toBe('pnps'); + }); +}); diff --git a/src/test/fixtures/fake-user-feedback-store.ts b/src/test/fixtures/fake-user-feedback-store.ts new file mode 100644 index 0000000000..f86cf03e46 --- /dev/null +++ b/src/test/fixtures/fake-user-feedback-store.ts @@ -0,0 +1,5 @@ +module.exports = () => ({ + getAllUserFeedback: () => Promise.resolve([]), + getFeedback: () => Promise.resolve({}), + updateFeedback: () => Promise.resolve({}), +}); diff --git a/src/test/fixtures/store.js b/src/test/fixtures/store.js index fe0762bd4f..b0d4b64e00 100644 --- a/src/test/fixtures/store.js +++ b/src/test/fixtures/store.js @@ -14,6 +14,7 @@ const addonStore = require('./fake-addon-store'); const projectStore = require('./fake-project-store'); const UserStore = require('./fake-user-store'); const AccessStore = require('./fake-access-store'); +const userFeedbackStore = require('./fake-user-feedback-store'); module.exports = { createStores: (databaseIsUp = true) => { @@ -39,6 +40,7 @@ module.exports = { projectStore: projectStore(databaseIsUp), userStore: new UserStore(), accessStore: new AccessStore(), + userFeedbackStore: userFeedbackStore(databaseIsUp), }; }, };