Security

[cryogenian]

Currently only stubs and specs are ready for this in slamdata/quasar-advanced#50 and I'm not sure if this should be merged before sharing is implemented in quasar-advanced. Although this won't harm any existed code probably.

@garyb Could you please take a look?

[garyb]

Apart from those few comments the code all looks good to me. :+1:

I'd like to have at leas one super simple test case for each of the advanced endpoints running before we merge it though, as we have for the community stuff currently, so I guess we need the new quasar-advanced for that first too.

[cryogenian]

• Changed message in Quasar.Error
• Changed comment for UserId decoding
• API works with records, newtypes used internally for encoding/decoding.

Unfortunately backend isn't ready yet --> I can't implement tests. Could we push this to separate branch? Or maybe just don't close this pr until tests? Or merge and don't release?

[cryogenian]

I've left newtypes for special String cases like TokenHash and PermissionId though

[garyb]

Thanks for the amends!

String newtypes are :ok_hand:, it's more to avoid the need for lenses to make record manipulations convenient.

[garyb]

Oops, posted that ^ before I was done typing.

As for tests, I realise that they're not implementable yet, I was just saying ideally before we merge this we'd have something to test against. If we need this to make progress with SlamData maybe we could reference your branch in the bower.json?

I'm fine with just keeping the PR open until we have tests - I don't think there's anything else we need from purescript-quasar right now, so we shouldn't have any conflicts at least!

[cryogenian]

Ok, thanks for review!

[garyb]

I'll go ahead and merge this since we've been using it a while anyway :wink: (and I need to update this for 0.9)

The build error is just a Debug.Trace reference, so I'll remove that while updating.