eluv-io / elv-live-js

Eluvio Live JavaScript SDK
MIT License
4 stars 3 forks source link

Issues #398: tenant_show_new, add/remove content admin #124

Closed elv-Noppapon closed 1 year ago

elv-Noppapon commented 1 year ago

Organizing OG Tenant

TenantShowNew: The original TenantShow- link is expected to be renamed to something along the line of TenantShowNft and replaced by this new function. The function returns the tenant's admins group and content's admins by calling the GroupsMapping field on the tenant's contract.

AddContentAdmin and RemoveContentAdmin: Given 1.) tenantId and 2.) contentAdminsAddress, the function make corresponding addition/removal to the GroupsMapping["content_admin"] field on the contract.

CLI/Function Exposure Notes: All of the above functions can be called directly from the CLI. However, I noticed that similar functions such as TenantAddNft and TenantRemoveNft are not exposed to the CLI directly, but are part of other command lines/functions. We might want something similar here?

Testing:

Screen Shot 2023-06-15 at 10 51 07 AM Screen Shot 2023-06-15 at 10 52 24 AM

TenantShowNew behaves expectedly, but AddContentAdmin and RemoveContentAdmin (called from the command line) do not successfully update the contract. I believe this might be because the onlyAdmin tag on the addGroup/removeGroup functions. I would appreciate any input here!

June 19 Updates

TenantShowNew now returns a warning message if tenant doesn't have content admin's group in GroupsMapping. tenant_fix running tenant_fix as owner will create a new content admin's group where the owner is the group's manager. TenantCreate/Deploy creates/adds content admin's group (initially only tenant admin's group).

Testing: space_tenant_create -> tenant_show_new -> tenant_remove_content_admin -> tenant_show_new -> tenant_add_content_admin -> tenant_show_new behaves as expected.

elv-Noppapon commented 1 year ago

Looking at it briefly, it all seems good to me. Some small nits around print statements.

Re: CLI exposure, I think it makes sense to have this functionality exposed on a CLI.

Definitely get the add/remove functions to pass somehow before merging though. I don't see anything wrong with the key, so most likely the problem is the PRIVATE_KEY environment variable not having admin permissions to the tenant or something. I'm not too familiar with it.

You're right, the problem was with not having admin permissions. Resolved, thanks!

elv-Noppapon commented 1 year ago

When creating a tenant:

  • we create the Tenant Admins and Content Admins group
  • also add _ELV_TENANT_ID to both these contracts (such that we can later retrieve the tenant ID given the group address

In tenant_show, verify that both groups have the _ELV_TENANT_ID and that it is set correctly

In elv-client-js:

Add a function DisambiguateTenantId(string) that takes either the tenant admins group (in iten format) or the actual tenant contract Id (in iten format).

  • find out if this is a group or a tenant contract (potentially by checking the presence of a method - I don't know if there are better ways to check)
  • if it is a group, then read the _ELV_TENANT_ID of the group to retrieve the tenant contract Id
  • return an object
  'tenantAdminsGroupId': '',
  'tenantId': ''

The idea is that given an Id that is either the tenant contract or the tenant admins group ID, figure out the tenant info.

Thank you! Seems like there is already a way to DisambiguateTenantId(string) by calling this.client.authClient.AccessType(id). It returns either 'tenant' or 'group'.