Multiple organization support #46
Conversation
| @@ -54,10 +55,11 @@ def initialize(options = | |||
| @auth_client = Fabricio::Authorization::AuthorizationClient.new | |||
| session = obtain_session | |||
| network_client = Fabricio::Networking::NetworkClient.new(@auth_client, @session_storage) | |||
| organization_id_provider = Fabricio::Networking::OrganizationIdProvider.new(lambda { return @app_service.all }) | |||
There was a problem hiding this comment.
I'm not 100% happy with this approach, but it's the easiest way I found to break the circular reference between OrganizationIdProvider and AppService without giving up constructor injection (the provider needs the service to lazily request the list of applications, and the service needs the provider to build some URLs via its model factory).
There was a problem hiding this comment.
Thanks for contributing! I haven't encountered this problem before. @CognitiveDisson will review changes too and after that we'll merge the PR and update gem.
| @@ -54,10 +55,11 @@ def initialize(options = | |||
| @auth_client = Fabricio::Authorization::AuthorizationClient.new | |||
| session = obtain_session | |||
| network_client = Fabricio::Networking::NetworkClient.new(@auth_client, @session_storage) | |||
| organization_id_provider = Fabricio::Networking::OrganizationIdProvider.new(lambda { return @app_service.all }) | |||
|
We just discussed the implementation logic with @CognitiveDisson. It seems that hiding details of multiple organisations from the user isn't a good idea. |
|
Sure, I can do that, it should be easy! That being said, your approach will break existing clients since we will start asking for an extra argument in pretty much all the relevant |
|
Hi, Any ETA on this feature? Regarding the impact on previous versions, why don’t the gem use the first organisation found if not provided? Regards |
|
@eMxPi @xavierjurado Implement multiple organization support in #49 . If there are any problems please inform |
|
Thanks for the update @CognitiveDisson. As far as I can see some requests in |
Description
Add support to accounts with more than one organization.
Rationale
An
organization_idis needed by most API requests because it's the only way to properly identify an application. We were assuming that an account was member of a single organization, so we made a request to/organizationsright after we got a valid session and stored the identifier of the first object returned (as seen here: https://github.com/strongself/fabricio/blob/develop/lib/fabricio/authorization/authorization_client.rb#L118).This is sadly not a valid approach for accounts linked to two or more organizations and can lead to issues like #45.
With this PR I've tried to solve this issue by building (and caching) an
app_idtoorganization_idmap (seeOrganizationIdProviderclass). I've done so lazily since this involves performing a new request in order to obtain the full list of applications for the account. Hopefully no performance loss should come from this because I've been able to delete the previous request to/organizations.I'm new to this project and I'm not used to write Ruby so any feedback is more than welcomed!