Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Add customgroup sharing for contacts app #33712

Merged
merged 1 commit into from
Dec 10, 2018
Merged

Add customgroup sharing for contacts app #33712

merged 1 commit into from
Dec 10, 2018

Conversation

Julian1998
Copy link
Contributor

@Julian1998 Julian1998 commented Nov 30, 2018 •
edited by DeepDiver1975
Loading

Description

Added sharing scope in cardDav backend to enable addressbook sharing with customgroups

How Has This Been Tested?

  • User A created a userdefined customgroup
  • Add User B to this group
  • Create or use an existing addressbook in contacts app
  • share that addressbook with the created customgroup
  • Have a look into User B's contacts app if the contact(s) and addressbook appeared

Types of changes

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to change)
  • Technical debt
  • Tests

Checklist:

  • Code changes
  • Unit tests added
  • Acceptance tests added
  • Documentation ticket raised:

Open tasks:

  • Backport

@Julian1998 Julian1998 added this to the development milestone Nov 30, 2018
@Julian1998 Julian1998 self-assigned this Nov 30, 2018
@Julian1998
Copy link
Contributor Author

@PVince81 Is this the right way of using the 'sharing' scope for customgroup sharing?

@codecov
Copy link

codecov bot commented Nov 30, 2018 •
edited
Loading

Codecov Report

Merging #33712 into master will not change coverage.
The diff coverage is 100%.

Impacted file tree graph

@@            Coverage Diff            @@
##             master   #33712   +/-   ##
=========================================
  Coverage     64.35%   64.35%           
  Complexity    18309    18309           
=========================================
  Files          1195     1195           
  Lines         69255    69255           
  Branches       1276     1276           
=========================================
  Hits          44566    44566           
  Misses        24317    24317           
  Partials        372      372
Flag Coverage Δ Complexity Δ
#javascript 52.98% <ø> (ø) 0 <ø> (ø) ⬇️
#phpunit 65.66% <100%> (ø) 18309 <0> (ø) ⬇️
Impacted Files Coverage Δ Complexity Δ
apps/dav/lib/Connector/Sabre/Principal.php 96.92% <100%> (ø) 27 <0> (ø) ⬇️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update ad60d7c...cbb4bd9. Read the comment docs.

DeepDiver1975
DeepDiver1975 reviewed Dec 3, 2018
View reviewed changes
apps/dav/lib/Connector/Sabre/Principal.php Outdated
@@ -145,7 +146,7 @@ public function getGroupMembership($principal, $needGroups = false) {
}

if ($this->hasGroups || $needGroups) {
$groups = $this->groupManager->getUserGroups($user);
$groups = $this->groupManager->getUserGroups($user, $scope);
Copy link
Member

Choose a reason for hiding this comment

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

please query the code base within apps/dav where getGroupMembership is used.

From my poc we should use scope 'sharing' everywhere ....

@micbar
Copy link
Contributor

micbar commented Dec 10, 2018

@Julian1998 Who started this? Is this a PR started in engineering?

@Julian1998
Copy link
Contributor Author

@micbar It's a customer request: https://talk.owncloud.com/channel/calendar-and-contacts?msg=879RrbKC9n8mmTT8p

@Julian1998 Julian1998 force-pushed the add-customgroups-for-contacts-sharing branch from 09a1f87 to cbb4bd9 Compare December 10, 2018 18:56
@Julian1998
Copy link
Contributor Author

@DeepDiver1975 please review again!

@DeepDiver1975 DeepDiver1975 merged commit 588da73 into master Dec 10, 2018
@delete-merged-branch delete-merged-branch bot deleted the add-customgroups-for-contacts-sharing branch December 10, 2018 21:43
@DeepDiver1975 DeepDiver1975 mentioned this pull request Dec 10, 2018
Merged
@DeepDiver1975
Copy link
Member

#33849

@davitol davitol mentioned this pull request Jan 10, 2019
Closed
39 tasks
@lock lock bot locked as resolved and limited conversation to collaborators Dec 10, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@DeepDiver1975 DeepDiver1975 DeepDiver1975 approved these changes

@PVince81 PVince81 Awaiting requested review from PVince81

Assignees

@Julian1998 Julian1998

Labels
3 - To Review discussion
Projects
None yet
Milestone
development
Development

Successfully merging this pull request may close these issues.
3 participants
@Julian1998 @micbar @DeepDiver1975