Revert "backports changes back to feature-rebac (#1003)" #1030
Conversation
kian99
requested review from
mina1460,
alesstimec,
ale8k and
babakks
and removed request for
mina1460 and
alesstimec
|
I thought we will just change the call from the old method to the new method that we already had. |
@mina1460 So I tried just changing the call, but the OpenFGA method for retrieving the list of models a user has access to only returns a list of UUID so I thought it would be better to just revert the entire commit. But thinking about it, maybe I should just grab all the models and then do things similar to the way it was before. |
kian99
force-pushed
the
CSS-5254-revert-cross-model-query-using-postgres
branch
from
edc6952 to
fb64886
Compare
|
@mina1460 I've switched it as I realised it's probably better to get all the models first, ptal |
|
@ale8k sorry the original description is misleading, if you read the follow-up comments you'll see I discussed with Mina and changed it not to be a simple revert. |
|
code looks good, but there are some failing tests |
| models[i] = m.Model_ | ||
| // UUIDs are returned in the form "model:uuid" | ||
| for i, uuid := range modelUUIDs { | ||
| modelUUIDs[i] = strings.Split(uuid, ":")[1] |
There was a problem hiding this comment.
strings.Split bit assumes we will never change the format.. which might be ok.. also assumes there will be a : in the string..
There was a problem hiding this comment.
perhaps we need a function that will split the tag type from the value in jaas tags...
There was a problem hiding this comment.
I'm going to do this in a follow up PR, I believe we should be returning ofga entities from ofga related queries, not strings. Then we won't have this parsing problem.
Description
Reverts #1003 the backport introduced a regression where we query Postgres for user access to models instead of OpenFGA. I think the best way forward is simply to revert the commit, which is all this PR does.
Fixes CSS-5254
Engineering checklist
Check only items that apply