Skip to content

Use all available group services#2972

Merged
ChristophWurst merged 1 commit into
masterfrom
fix/contacts-integration-services
Apr 27, 2020
Merged

Use all available group services#2972
ChristophWurst merged 1 commit into
masterfrom
fix/contacts-integration-services

Conversation

@ChristophWurst

Copy link
Copy Markdown
Member

When we just had one implementation of a group service we could just
inject the implementation when the interface is used. However, as
another implementation was added, it is necessary to explicitly require
the implementations to be able to use all of them.

Before the patch the contacts group integration overwrote the nextcloud group integration.

Fixes #810

@nextcloud/mail

When we just had one implementation of a group service we could just
inject the implementation when the interface is used. However, as
another implementation was added, it is necessary to explicitly require
the implementations to be able to use all of them.

Signed-off-by: Christoph Wurst <christoph@winzerhof-wurst.at>
@ChristophWurst ChristophWurst force-pushed the fix/contacts-integration-services branch from e6f668f to 858da61 Compare April 23, 2020 07:13
public function __construct(IGroupService $groupService) {
$this->groupServices[] = $groupService;
public function __construct(ContactsGroupService $contactsGroupService, NextcloudGroupService $nextcloudGroupService) {
$this->groupServices = [

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So we decide on ordering of the services here?

If any service fails nothing is returned in getMatchingGroups, but that is outside the scope of this issue.

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code looks good.

However, I wanted to do an integration test on the GUI, but I'm not able to install the mail app since composer install fails on requiring kwi/urllinker which seems to be a private repo?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

fails on requiring kwi/urllinker which seems to be a private repo?

Are you sure it fails? I think that is just a warning. You can perhaps ignore that.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If any service fails nothing is returned in getMatchingGroups, but that is outside the scope of this issue.

Yes and yes :)

@ChristophWurst

Copy link
Copy Markdown
Member Author

/backport to stable1.3

@ChristophWurst ChristophWurst merged commit cf69ebc into master Apr 27, 2020
@ChristophWurst ChristophWurst deleted the fix/contacts-integration-services branch April 27, 2020 07:05
@backportbot-nextcloud

Copy link
Copy Markdown

backport to stable1.3 in #3005

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants