add pdfs mutation + query #69
Conversation
also, use db users instead of env vars and simplify auth
| }); | ||
|
|
||
| const twoOrgs = await client.request(` | ||
| query twoOrgs { |
There was a problem hiding this comment.
Dumb question, is the left align necessary here? makes it a little tough to skim but NBD
There was a problem hiding this comment.
i'm very open to changing the indentation for these blocks!
There was a problem hiding this comment.
Yeah! I wondered if it was necessary for parsing or a convention for graphql but I'm not familiar. If it's not having it indented I think would help.
|
|
||
| expect(res.pdfs.length).toBe(2); | ||
| expect(res.pdfs[0].uuid).toEqual(thisUserPdf.uuid); | ||
| expect(res.pdfs[1].uuid).toEqual(anotherUserPdf.uuid); |
There was a problem hiding this comment.
Wait, does it not randomize the test order?
In general I would expect each test to be self-contained in terms of setup and teardown.
There was a problem hiding this comment.
jest executes the tests within a suite sequentially - jestjs/jest#4386 - the files themselves will be run in a random order though
There was a problem hiding this comment.
Ah I'm so used to rails tests, which randomize order to enforce test isolation and catch random order-of-operations bugs.
| }); | ||
|
|
||
| const twoOrgs = await client.request(` | ||
| query twoOrgs { |
There was a problem hiding this comment.
Yeah! I wondered if it was necessary for parsing or a convention for graphql but I'm not familiar. If it's not having it indented I think would help.
|
|
||
| expect(res.pdfs.length).toBe(2); | ||
| expect(res.pdfs[0].uuid).toEqual(thisUserPdf.uuid); | ||
| expect(res.pdfs[1].uuid).toEqual(anotherUserPdf.uuid); |
There was a problem hiding this comment.
Ah I'm so used to rails tests, which randomize order to enforce test isolation and catch random order-of-operations bugs.
src/auth.ts
Outdated
| @@ -49,13 +47,16 @@ export const authenticatedOnly = (handler, client: OAuth2Client) => ( | |||
|
|
|||
| export const getTokenFromReq = req => req.headers['x-auth-token']; | |||
|
|
|||
| export const getUserFromToken = async (tok: string, client: OAuth2Client) => { | |||
| export const getUserFromToken = async ( | |||
| tok: string, | |||
|
|
||
| import { getUserFromToken } from '../../auth'; | ||
|
|
||
| import { ledgerListArgs, MAX_LIMIT } from '../../helpers'; |
There was a problem hiding this comment.
Maybe not for this PR, but is there a way we can escape relative imports? I've done it in the past with CRA, but haven't with regular node -- looks like we may be able to by making the app a "type": "module" in the package.json? [This could be an issue instead of a comment]
|
|
||
| const ADMINS = [ | ||
| { name: 'Ben', email: 'anonyben@gmail.com' }, | ||
| { name: 'Matt', email: 'matthew.hampel@gmail.com' }, |
There was a problem hiding this comment.
could I be matt@matth.org?
Oh wait if it needs to be gmail, then mahatm@gmail.com
src/server.ts
Outdated
|
|
||
| if (!organization) throw new Error('missing organization'); | ||
|
|
||
| const user = input.user |
There was a problem hiding this comment.
QQ, so we can specify a user in the form/input, and then we fall back to the authenticated user?
There was a problem hiding this comment.
ooh yeah i think this should change -- we should be able to add a pdf without assigning a user right away.
also, use db users instead of env vars
and simplify auth