From 06ccd496f16ed95d81b5f4409eaa84905fd84101 Mon Sep 17 00:00:00 2001 From: Mihail Dunaev <mihail.dunaev91@gmail.com> Date: Mon, 23 Apr 2018 21:26:58 +0100 Subject: [PATCH] refactor(submission): make the submission backend component cleaner did some tests as well and it seems to work (more tests required) re #52 --- .../submission/WithCurrentSubmission.js | 5 + app/routes.js | 1 - server/db-helpers/index.js | 140 +++++++++++------- server/submission/definitions.js | 104 ++++--------- server/submission/package.json | 1 + 5 files changed, 119 insertions(+), 132 deletions(-) diff --git a/app/components/submission/WithCurrentSubmission.js b/app/components/submission/WithCurrentSubmission.js index 39e03c2..5e54f81 100644 --- a/app/components/submission/WithCurrentSubmission.js +++ b/app/components/submission/WithCurrentSubmission.js @@ -38,6 +38,11 @@ const CREATE_SUBMISSION = gql` } ` +/** + * TODO update submission + * "id": "9e7920ba-ea8b-4ccc-9668-45c80da51947" + */ + class CreateSubmissionWrapper extends React.Component { componentDidMount() { this.props.createSubmission() diff --git a/app/routes.js b/app/routes.js index 1d997d9..68ab5f0 100644 --- a/app/routes.js +++ b/app/routes.js @@ -17,7 +17,6 @@ const Routes = () => ( <Route component={Dashboard} /> </Switch> </Layout> - </AuthenticatedComponent> </Switch> ) diff --git a/server/db-helpers/index.js b/server/db-helpers/index.js index 25ec409..3e00eb4 100644 --- a/server/db-helpers/index.js +++ b/server/db-helpers/index.js @@ -1,32 +1,73 @@ const uuid = require('uuid') const Model = require('pubsweet-server/src/models/Model') -// const NotFoundError = require('pubsweet-server/src/errors/NotFoundError') const db = require('pubsweet-server/src/db') -/* function manuscriptToDb(graphqlObj, ctx) { */ -/* /\** */ -/* * TODO */ -/* * graphql schema is not identical to the db schema */ -/* * this needs to be documented */ -/* * */ -/* * function that converts a manuscript from graphql format to db format */ -/* * this function assumes graphqlObj has all the fields */ -/* *\/ */ -/* const dbObj = { ...graphqlObj } */ -/* dbObj.submissionMeta.createdBy = ctx.user */ -/* dbObj.type = 'manuscript' */ -/* delete dbObj.id */ -/* return dbObj */ -/* } */ +/** + * TODO + * functions here should have sanity checks in the future + */ -/* function manuscriptToGraphql(dbObj, id) { */ -/* /\* TODO *\/ */ -/* const manuscript = { ...dbObj } */ -/* delete manuscript.createdBy */ -/* delete manuscript.type */ -/* manuscript.id = id */ -/* return manuscript */ -/* } */ +function manuscriptToDb(manuscript, owner) { + /** + * converts manuscript from graphql schema to db schema + * for now this is the only place where the db schema for + * a manuscript is documented + */ + const manuscriptDb = { ...manuscript } + delete manuscriptDb.id + manuscriptDb.submissionMeta.createdBy = owner + manuscriptDb.type = 'manuscript' + return manuscriptDb +} + +function manuscriptToGraphql(manuscriptDb, id) { + /** + * converts manuscript from db schema to graphql schema + */ + const manuscript = { ...manuscriptDb } + manuscript.id = id + delete manuscript.submissionMeta.createdBy + delete manuscript.type + return manuscript +} + +const save = async obj => { + /** + * saves generic object to db + */ + const id = uuid.v4() + await db.query('INSERT INTO entities (id, data) VALUES ($1, $2)', [id, obj]) + return id +} + +const update = async (obj, id) => { + /** + * updates generic object in db + * this should be moved to save in the future + */ + await db.query('UPDATE entities SET data = $2 WHERE id = $1', [id, obj]) +} + +const checkPermission = async (id, user) => { + /** + * checks if manuscript is owned by user and returns old data + * in db format (id, data) + * + * manuscript owner should be stored at top level in the future + * or be in sync with other models + */ + const { rows } = await db.query( + `SELECT id, data FROM entities WHERE id = $1`, + [id], + ) + if (!rows.length) { + throw new Error('Manuscript not found') + } + if (user !== rows[0].data.submissionMeta.createdBy) { + throw new Error('Manuscript not owned by user') + } + return rows[0] +} const select = async selector => { const where = Model.selectorToSql(selector) @@ -37,38 +78,31 @@ const select = async selector => { ) return rows - // return manuscriptToGraphql(rows[0].data, rows[0].id) } -const save = async manuscript => { +const getOrcidData = async user => /** - * TODO jsdoc args + returns - * this returns the id of the stored element + * TODO + * get orcid data for user (orcid user id) + * + * @returns {object} - orcid data + * {submissionMeta: {author: {firstName: "", lastName: "", + * email: "", institution: ""}}} */ - const id = uuid.v4() - // const manuscriptDb = manuscriptToDb(manuscript, ctx) - // const manuscriptDb = manuscript - await db.query('INSERT INTO entities (id, data) VALUES ($1, $2)', [ - id, - manuscript, - ]) - return id -} -const update = async (id, manuscript) => { - /** - * TODO jsdoc args + returns - * this should be moved to save() - * sanity checks for args - * error thrown on query fail? - * return something here? - */ - // const manuscriptDb = manuscriptToDb(manuscript, ctx) - // const manuscriptDb = manuscript - await db.query('UPDATE entities SET data = $2 WHERE id = $1', [ - id, - manuscript, - ]) -} + /* const { rows } = await dbx.query( */ + /* `SELECT id, data FROM entities WHERE id = $1`, */ + /* [ctx.user], */ + /* ) */ + + ({}) -module.exports = { select, save, update } +module.exports = { + manuscriptToDb, + manuscriptToGraphql, + select, + save, + update, + checkPermission, + getOrcidData, +} diff --git a/server/submission/definitions.js b/server/submission/definitions.js index 412ef21..c39c675 100644 --- a/server/submission/definitions.js +++ b/server/submission/definitions.js @@ -1,12 +1,9 @@ +const lodash = require('lodash') + const db = require('../db-helpers/') -const dbx = require('pubsweet-server/src/db') /** - * TODO - * better way to have Manuscript as input type - * instead of duplicating each type - * - * do we need ID in Manuscript? + * types & input types should be kept in sync */ const typeDefs = ` extend type Query { @@ -63,13 +60,14 @@ const typeDefs = ` } ` -/* TODO keep this in sync with the schema */ +/** + * this should be kept in sync with the schema + */ const emptyManuscript = { + id: '', title: '', source: '', - type: 'manuscript', submissionMeta: { - createdBy: '', coverLetter: '', author: { firstName: '', @@ -87,41 +85,6 @@ const emptyManuscript = { }, } -/** - * TODO any built-in way to do this? - * would using an external package like deepmerge - * https://github.com/KyleAMathews/deepmerge - * be better here? - * e.g. we make the app more complex, we add more packages/use more memory - * vs - * we might miss to catch some errors - * it's something easy to do anyway - * - * replace with lodash.merge - */ -function applyObj(obj1, obj2) { - /** - * applies values from obj2 (can be nested) - * to "default values"/schema obj1 - * obj1 will have all keys - */ - const obj3 = { ...obj1 } - if (typeof obj2 !== 'object') { - return obj3 - } - Object.keys(obj3).forEach(key => { - if (key in obj2) { - if (typeof obj2[key] === 'object') { - obj3[key] = applyObj(obj3[key], obj2[key]) - } else { - obj3[key] = obj2[key] - } - } - }) - - return obj3 -} - const resolvers = { Query: { async currentSubmission(_, vars, ctx) { @@ -142,12 +105,8 @@ const resolvers = { }, Mutation: { async createSubmission(_, vars, ctx) { - /* const { rows } = await dbx.query( */ - /* `SELECT id, data FROM entities WHERE id = $1`, */ - /* [ctx.user], */ - /* ) */ // TODO get actual data from orcid - // console.log(rows[0]); + // const orcidData = db.getOrcidData(ctx.user); const orcidData = { submissionMeta: { author: { @@ -158,40 +117,29 @@ const resolvers = { }, }, } - const manuscript = applyObj(emptyManuscript, orcidData) - manuscript.submissionMeta.createdBy = ctx.user - const id = await db.save(manuscript) + const manuscript = lodash.merge(emptyManuscript, orcidData) + const manuscriptDb = db.manuscriptToDb(manuscript, ctx.user) + const id = await db.save(manuscriptDb) manuscript.id = id return manuscript }, async updateSubmission(_, vars, ctx) { - /** - * TODO - * check that manuscript with vars.data.id is actually - * owned by ctx.user - * - * we can get manuscript after id, then check with createdBy - * or just get current manuscript for ctx.user and save there - * - * for now this works - get current submission for ctx.user - * if none - create new one for the current user - * else just update - */ - const { rows } = await dbx.query( - `SELECT id, data FROM entities WHERE id = $1`, - [vars.data.id], + // check permission + const { data: oldManDb } = await db.checkPermission( + vars.data.id, + ctx.user, ) - if (!rows.length) { - throw new Error('Manuscript not found') - } - if (ctx.user !== rows[0].data.submissionMeta.createdBy) { - throw new Error('Manuscript not owned by user') - } - // TODO convert from db to manuscript - const manuscript = applyObj(rows[0].data, vars.data) - await db.update(rows[0].id, manuscript) - manuscript.id = rows[0].id - return manuscript + + // compute new manuscript by applying vars.data to old manuscript + const oldMan = db.manuscriptToGraphql(oldManDb, vars.data.id) + const newMan = lodash.merge(oldMan, vars.data) + + // update old one + const newManDb = db.manuscriptToDb(newMan, ctx.user) + await db.update(newManDb, vars.data.id) + + // return updated manuscript to user + return newMan }, }, } diff --git a/server/submission/package.json b/server/submission/package.json index 3016f8a..4d91eed 100644 --- a/server/submission/package.json +++ b/server/submission/package.json @@ -12,6 +12,7 @@ "apollo-boost": "^0.1.4", "config": "^1.30.0", "graphql-tag": "^2.8.0", + "lodash": "^4.17.5", "uuid": "^3.2.1" }, "peerDependencies": { -- GitLab