From 665cf85b545542ad247aecaa89a0ba743aeab12f Mon Sep 17 00:00:00 2001
From: Jure Triglav <juretriglav@gmail.com>
Date: Thu, 24 Jan 2019 14:07:08 +1300
Subject: [PATCH] feat(model-team): simplify objectId and objectType storage
BREAKING CHANGE: Previously objectId and objectType were stored in a JSONB column on the teams
table. This has changed (and the migration takes care of table and data migration) in favor of
storing objectId and objectType as flat columns on the teams table. For reasons of querying,
indexing and ease of use, this is a better option. The read side of the API still returns a nested
TeamObject, but the GraphQL mutations have changed (see `team_graphql_test.js` for some examples).
---
packages/components/model-team/package.json | 1 +
packages/components/model-team/src/graphql.js | 36 ++---------
.../migrations/1548205276-simplify-object.js | 27 ++++++++
packages/components/model-team/src/team.js | 3 +-
.../test/1548205276-simplify-object_test.js | 41 ++++++++++++
.../model-team/test/team_graphql_test.js | 64 ++++++++++---------
6 files changed, 112 insertions(+), 60 deletions(-)
create mode 100644 packages/components/model-team/src/migrations/1548205276-simplify-object.js
create mode 100644 packages/components/model-team/test/1548205276-simplify-object_test.js
diff --git a/packages/components/model-team/package.json b/packages/components/model-team/package.json
index 857bc14b1..74153d821 100644
--- a/packages/components/model-team/package.json
+++ b/packages/components/model-team/package.json
@@ -19,6 +19,7 @@
},
"peerDependencies": {
"@pubsweet/model-user": "^2.0.0",
+ "@pubsweet/models": "^0.1.3",
"config": "^3.0.1",
"lodash": "^4.17.11",
"pubsweet-server": "^11.0.0",
diff --git a/packages/components/model-team/src/graphql.js b/packages/components/model-team/src/graphql.js
index 2f4034d32..483f62367 100644
--- a/packages/components/model-team/src/graphql.js
+++ b/packages/components/model-team/src/graphql.js
@@ -4,30 +4,6 @@ const resolvers = {
return ctx.connectors.Team.fetchOne(id, ctx)
},
teams(_, { where }, ctx) {
- // A little bit of API sugar to provide querying with e.g.
- // where: {
- // role: 'test',
- // object: {
- // objectId: fragment.id,
- // objectType: 'fragment',
- // },
- // members: [UUID]
- // }
- // which is then translated into appropriate JSON/relation queries.
- if (
- where &&
- where.object &&
- where.object.objectId &&
- where.object.objectType
- ) {
- const { object } = where
- delete where.object
- where._json = [
- { ref: 'teams.object:objectId', value: object.objectId },
- { ref: 'teams.object:objectType', value: object.objectType },
- ]
- }
-
if (where.members) {
const { members } = where
delete where.members
@@ -87,6 +63,10 @@ const resolvers = {
members(team, vars, ctx) {
return ctx.connectors.User.fetchSome(team.members, ctx)
},
+ object(team, vars, ctx) {
+ const { objectId, objectType } = team
+ return objectId && objectType ? { objectId, objectType } : null
+ },
},
}
@@ -123,16 +103,12 @@ const typeDefs = `
input TeamInput {
role: String
name: String
- object: TeamObjectInput
+ objectId: ID
+ objectType: String
members: [ID!]
global: Boolean
}
-
- input TeamObjectInput {
- objectId: ID!
- objectType: String!
- }
`
module.exports = { resolvers, typeDefs }
diff --git a/packages/components/model-team/src/migrations/1548205276-simplify-object.js b/packages/components/model-team/src/migrations/1548205276-simplify-object.js
new file mode 100644
index 000000000..f04443c6a
--- /dev/null
+++ b/packages/components/model-team/src/migrations/1548205276-simplify-object.js
@@ -0,0 +1,27 @@
+exports.up = async knex => {
+ const { Team } = require('@pubsweet/models')
+ const teams = await Team.query()
+
+ const saves = []
+
+ await knex.schema.table('teams', table => {
+ table.uuid('object_id')
+ table.string('object_type')
+ table.index(['object_id', 'object_type'])
+ })
+
+ teams.forEach(team => {
+ if (team.object && team.object.objectId && team.object.objectType) {
+ team.objectId = team.object.objectId
+ team.objectType = team.object.objectType
+ delete team.object
+ saves.push(team.save())
+ }
+ })
+
+ await Promise.all(saves)
+
+ return knex.schema.table('teams', table => {
+ table.dropColumn('object')
+ })
+}
diff --git a/packages/components/model-team/src/team.js b/packages/components/model-team/src/team.js
index 28e0cca60..287239daa 100644
--- a/packages/components/model-team/src/team.js
+++ b/packages/components/model-team/src/team.js
@@ -34,7 +34,8 @@ class Team extends BaseModel {
static get schema() {
return {
properties: {
- object: { type: ['object', 'null'] },
+ objectId: { type: ['string', 'null'], format: 'uuid' },
+ objectType: { type: ['string', 'null'] },
name: { type: 'string' },
role: { type: ['string'] },
owners: {
diff --git a/packages/components/model-team/test/1548205276-simplify-object_test.js b/packages/components/model-team/test/1548205276-simplify-object_test.js
new file mode 100644
index 000000000..22ccbd75b
--- /dev/null
+++ b/packages/components/model-team/test/1548205276-simplify-object_test.js
@@ -0,0 +1,41 @@
+process.env.NODE_CONFIG = `{"pubsweet":{
+ "components":[
+ "@pubsweet/model-user",
+ "@pubsweet/model-team",
+ "@pubsweet/model-team-member",
+ "@pubsweet/model-fragment"
+ ]
+}}`
+
+const { model: Team } = require('../src')
+const { dbCleaner } = require('pubsweet-server/test')
+const migrate = require('@pubsweet/db-manager/src/commands/migrate')
+
+describe('Migration to simplify object storage', () => {
+ it('successfully migrates from JSONB to separate columns', async () => {
+ // Clean database and run up until the migration we're testing
+ await dbCleaner({ to: '1548205275-move-members.js' })
+
+ // Get a team with the previous members array structure
+ let team = await new Team({
+ name: 'Test',
+ role: 'test',
+ }).save()
+
+ // Using id and type 'team' here just for testing
+ await Team.raw('UPDATE teams SET object = ?::jsonb WHERE id = ?', [
+ JSON.stringify({ objectId: team.id, objectType: 'team' }),
+ team.id,
+ ])
+
+ // Do the migration
+ await migrate({ to: '1548205276-simplify-object.js' })
+
+ // Check that members have migrated to the relationship
+ team = await Team.query().findById(team.id)
+
+ expect(team.objectId).toEqual(team.id)
+ expect(team.objectType).toEqual('team')
+ expect(team.object).toBeUndefined()
+ })
+})
diff --git a/packages/components/model-team/test/team_graphql_test.js b/packages/components/model-team/test/team_graphql_test.js
index 9d899d4f1..7b65377b5 100644
--- a/packages/components/model-team/test/team_graphql_test.js
+++ b/packages/components/model-team/test/team_graphql_test.js
@@ -12,7 +12,7 @@ const { model: Fragment } = require('@pubsweet/model-fragment')
const { dbCleaner, api } = require('pubsweet-server/test')
-const fixtures = require('pubsweet-server/test/fixtures/fixtures')
+const { fixtures } = require('@pubsweet/model-user/test')
const authentication = require('pubsweet-server/src/authentication')
const Team = require('../src/team')
@@ -26,6 +26,10 @@ describe('Team queries', () => {
`query($where: TeamInput) {
teams(where: $where) {
name
+ object {
+ objectId
+ objectType
+ }
}
}`,
{
@@ -43,6 +47,8 @@ describe('Team queries', () => {
})
it('creates a team', async () => {
+ const fragment = await new Fragment({ fragmentType: 'post' }).save()
+
const { body } = await api.graphql.query(
`mutation($input: TeamInput) {
createTeam(input: $input) {
@@ -50,6 +56,10 @@ describe('Team queries', () => {
members {
id
}
+ object {
+ objectId
+ objectType
+ }
}
}`,
{
@@ -57,6 +67,8 @@ describe('Team queries', () => {
name: 'My team',
role: 'test',
members: [user.id],
+ objectId: fragment.id,
+ objectType: 'fragment',
},
},
token,
@@ -67,6 +79,10 @@ describe('Team queries', () => {
createTeam: {
name: 'My team',
members: [{ id: user.id }],
+ object: {
+ objectId: fragment.id,
+ objectType: 'fragment',
+ },
},
},
})
@@ -137,10 +153,8 @@ describe('Team queries', () => {
{
role: 'test',
name: 'Test',
- object: {
- objectId: fragment.id,
- objectType: 'fragment',
- },
+ objectId: fragment.id,
+ objectType: 'fragment',
members: [
{
'#dbRef': user.id,
@@ -152,10 +166,8 @@ describe('Team queries', () => {
const body = await whereQuery({
role: 'test',
- object: {
- objectId: fragment.id,
- objectType: 'fragment',
- },
+ objectId: fragment.id,
+ objectType: 'fragment',
})
expect(body.data.teams).toHaveLength(1)
@@ -176,10 +188,8 @@ describe('Team queries', () => {
{
role: 'test',
name: 'Test',
- object: {
- objectId: fragment.id,
- objectType: 'fragment',
- },
+ objectId: fragment.id,
+ objectType: 'fragment',
members: [
{
'#dbRef': user.id,
@@ -196,23 +206,23 @@ describe('Team queries', () => {
it('finds a team for 1 member', async () => {
const body = await whereQuery({
role: 'test',
- object: {
- objectId: fragment.id,
- objectType: 'fragment',
- },
+ objectId: fragment.id,
+ objectType: 'fragment',
members: [user.id],
})
expect(body.data.teams).toHaveLength(1)
+ expect(body.data.teams[0].object).toEqual({
+ objectId: fragment.id,
+ objectType: 'fragment',
+ })
})
it('finds a team for both members', async () => {
const body = await whereQuery({
role: 'test',
- object: {
- objectId: fragment.id,
- objectType: 'fragment',
- },
+ objectId: fragment.id,
+ objectType: 'fragment',
members: [user.id, user2.id],
})
@@ -222,10 +232,8 @@ describe('Team queries', () => {
it('does not find a team for non-existent member', async () => {
const body = await whereQuery({
role: 'test',
- object: {
- objectId: fragment.id,
- objectType: 'fragment',
- },
+ objectId: fragment.id,
+ objectType: 'fragment',
members: ['54513de6-b473-4b39-8f95-bcbb3ae58a2a'],
})
@@ -235,10 +243,8 @@ describe('Team queries', () => {
it('does not find a team if missing one of the members', async () => {
const body = await whereQuery({
role: 'test',
- object: {
- objectId: fragment.id,
- objectType: 'fragment',
- },
+ objectId: fragment.id,
+ objectType: 'fragment',
members: [user.id, user2.id, '54513de6-b473-4b39-8f95-bcbb3ae58a2a'],
})
--
GitLab