feat(frontend): add operator-contextual dropdowns for tenant selection #3289
Conversation
github-actions
bot
added
type: tests
njlie
requested review from
koekiebox,
BlairCurrey,
mkurapov,
sanducb and
oana-lolea
| <Dropdown | ||
| options={tenants.map((tenant) => ({ | ||
| label: tenant.node.id, | ||
| value: `${tenant.node.id} ${tenant.node.publicName ? `(${tenant.node.publicName})` : ''}` |
There was a problem hiding this comment.
Good call.... came here to suggest using the name for the select after just seeing the id in the UI, but I understand the default operator just doesnt have one.
| required | ||
| /> | ||
| {tenants ? ( | ||
| <Select |
There was a problem hiding this comment.
Why chose Select here and in wallet address create, but Dropdown in asset create? Just wondering if we can standardize them or if they need to be different?
I see the asset defaults to the session tenant (ie the operator) and the others dont.
| export const createAsset = async ( | ||
| request: Request, | ||
| args: CreateAssetInput, | ||
| tenantId?: string | ||
| ) => { | ||
| const apolloClient = await getApolloClient(request, tenantId) |
There was a problem hiding this comment.
I think something is missing here.
The tenantId is never passed in createAsset (just called once) nor getApolloClient (only passed in here but its undefined). And there is no tenantId on the CreateAssetInput. The backend mutation uses the header, so it's just always going to be the operator.
You can validate this by changing the defaultValue on the tenant dropdown to any bogus value and submitting and see that it works (should fail because its not a real tenant id).
I think the intention was probably to pass the tenantId in here and override the header but I think previously we agreed we didn't want to do that to ensure the header always identifies the requestor.
I think we can remove the header overriding, put the tenantId on the gql input, and use the ctx.forTenantId provided by the setForTenantIdGraphQLMutationMiddleware (looks like mutation context type for assets needs to be updated to ForTenantIdContext).
| } | ||
| } | ||
| ) | ||
| ctx.logger.info({ tenantId }, 'tenantId for create asset') |
There was a problem hiding this comment.
| ctx.logger.info({ tenantId }, 'tenantId for create asset') | |
| @@ -669,6 +671,7 @@ type Asset implements Model { | |||
| ): FeesConnection | |||
| "The date and time when the asset was created." | |||
| createdAt: String! | |||
| tenantId: String | |||
There was a problem hiding this comment.
| tenantId: String | |
| tenantId: ID! |
| @@ -0,0 +1,84 @@ | |||
| import { gql } from '@apollo/client' | |||
There was a problem hiding this comment.
If you update 2893/multi-tenancy-v1 locally, you should see changes for this file from #3256
…n during asset, wallet address, and peer creation
njlie
force-pushed
the
nl/3273/admin-ui-tenant-selection
branch
from
a3ba1dd to
ff4054f
Compare
Changes proposed in this pull request
frontendpages:Context
Fixes #3273.
Checklist
fixes #numberuser-docslabel (if necessary)