bug: capabilities.assignRole() should update existing role

[gmaclennan]

Description

capabilities.assignRole() currently just creates a role record, it does not update an existing role. We currently only use it for the initial assignment of a role when inviting a device, but we should:

1. Wait until sync and indexing has finished, to ensure that any existing role for a device has synced.
2. Update the existing role, rather than create a new one (using the dataType.update() function)

This is probably best done as part of a PR to add member.assignRole() to expose this method to the public API, so that we can test it in e2e tests.

[achou11]

Wait until sync and indexing has finished, to ensure that any existing role for a device has synced.

Do you have a clear idea of where this happens? I figured it would be in the Capabilities class based on what you wrote. Was hoping to use SyncApi.waitForSync() there instead of re-implementing that logic, but that doesn't seem like it will work since the SyncApi needs a capabilities instance first (e.g. chicken vs egg problem).

Seems like to avoid re-implementing sync code, I would have to do the sync-waiting outside of the capabilities instance i.e. in the relevant MemberApi method. Something like:

// in member-api.js
async assignRole(deviceId, roleId) {
  await this.#waitForSync()
  return this.#capabilities.assignRole(deviceId, roleId)
}

where this.#waitForSync is based on a contructor opt for the MemberApi class. For example, in the MapeoProject implementation, it would be done like:

// in mapeo-project.js
this.#memberApi = new MemberApi({
  waitForSync: async () => {
    await this.#syncApi.waitForSync('initial')
  },
  // all of the other stuff...
})

This is all based on my early implementation explorations - there may be something I'm missing that's not clear to me at the moment

[achou11]

Another question: should we also wait for sync and indexing to finish when fetching the capabilities to ensure that we have up-to-date information? again, not sure where this lives e.g. does it live in Capabilities (e.g. getCapabilities() + getAll()) or outside of it (e.g. MemberApi.getById() + MemberApi.getMany())?

[gmaclennan]

I'm re-thinking this, and think I might have been overthinking before. The datastore/type implementation already waits for indexing, so I think that's enough. I don't think there is really a way to "wait for sync" in a meaningful way when assigning roles.

[achou11]

I'm re-thinking this, and think I might have been overthinking before. The datastore/type implementation already waits for indexing, so I think that's enough. I don't think there is really a way to "wait for sync" in a meaningful way when assigning roles.

yeah i figured the indexing question was less uncertain - just the syncing part had me wondering. if we don't need to add something specific for that, hopefully changing this is straightforward then!