Review Performance

[andreievg]

Related: https://github.com/msupply-foundation/conforma-server/issues/1103

I ran demo server with large data set without row level policies, and it was very snappy, for everything, including review (apart from some delay when review was finalised, i think due to some action/triggers, 5 or so seconds, which should be acceptable).

There are a couple of things we can do in short term to improve Review Performance:

The first step is to improve row lever policy speed by re-instating changes in this PR: https://github.com/msupply-foundation/conforma-server/pull/1079, that were later remove here: https://github.com/msupply-foundation/conforma-server/commit/8a0d9e52c89cc39c8cf8c95c1aa73d19b61e4737. Due to org permissions not being matched correctly. (1 days of work)

Next step is to relocate getReviewResponses query to a rest endpoint. This endpoint will use user JWT to do a query to specified reviewAssignment (to make sure user has required permissions), and then use admin access to do the actual query, ignoring row level policies. (2 days of work)

Ideally we would do most of the re-mapping that front end is doing, in back end (consumer focused API), but that would require quite a bit more work.

[CarlosNZ]

I've been playing around with the Apollo cache a bit and I think I understand it better now. I've been able to (finally) get the cache-and-network policy working nicely with the list (which shows a cached version immediately, but runs a network request in the background and updates the UI if changed)

In the cases where we have multiple getReviewResponses in quick succession, we should definitely be able to get a big improvement by getting the cache settings right. Might not need the additional endpoint even, not sure.

[CarlosNZ]

I would also be keen to have a look at the front-end logic and see if we can reduce the number of calls to getResponses.

[CarlosNZ]

Okay, so suggested structure/workflow is as follows:

1. Add policy restrictions to "reviewResponses" table to be completely restricted to all but "Admin" token
2. Create POST endpoint, like /gql/review/:reviewId
   • Generic gql sub-route as a starting point for future development
   • Pass existing graphQL query (for reviewResponses in this case) as body JSON
3. Endpoint logic as follows:
   • Query reviewId review using request (user's) JWT auth. If no result, then user doesn't have permission so return error
   • If result, then do some inspection of the Gql to ensure that the query/mutation is not trying to access any entities not connected to the reviewId (this is the somewhat tricky bit, need to look into the best way to do this. But I think we'll need to do something along these lines, as there's too many different queries and mutations that involve review responses to hard-code logic for all of them) This logic updated as per IRL discussion
   • If all okay, execute GraqphQL using admin token to avoid additional permission checks, and return result.
4. Update front-end to use this endpoint. Will attempt to create hook to replicate the Apollo behaviour as closely as possible, so as to minimise the changes required in the consuming components.
   • Also, update review response mutations to do multiple in a batch mutation rather than several individual ones

Regarding the additional checks we do on the GraphQL request, I think we can use something like graphql-tag to turn the query into an AST which we could write a recursive validity check function for. I'm open to a simpler way of handling this, but we need some way to handle all the different shaped queries and mutations for review responses without having to write separate logic for each one.

[CarlosNZ]

@andreievg once we agree on the exact solution, you okay to finish the policy improvements and I'll continue with the endpoint? I'm happy to do the bulk of this, but I'll check in with you for clarification if I encounter hurdles we haven't discussed.