Update code from review

cmd/instances.go

@@ -152,7 +152,7 @@ cozy-stack instances add allows to create an instance on the cozy for a
 given domain.
 
 If the COZY_DISABLE_INSTANCES_ADD_RM env variable is set, creating and
-destroying instances will be desactivated and the content of this variable will
+destroying instances will be disabled and the content of this variable will
 be used as the error message.
 `,
 	Example: "$ cozy-stack instances add --passphrase cozy --apps drive,photos,settings,home,store cozy.localhost:8080",

docs/cli/cozy-stack_instances_add.md

@@ -9,7 +9,7 @@ cozy-stack instances add allows to create an instance on the cozy for a
 given domain.
 
 If the COZY_DISABLE_INSTANCES_ADD_RM env variable is set, creating and
-destroying instances will be desactivated and the content of this variable will
+destroying instances will be disabled and the content of this variable will
 be used as the error message.
 
 

docs/sharing.md

@@ -887,8 +887,8 @@ Content-Type: application/vnd.api+json
 
 This is an internal route for the stack. It is called by the recipient cozy on
 the owner cozy to add recipients and groups to the sharing (`open_sharing:
-true` only). It should send an `email` address, but if the email address is not
-known, an `instance` URL can also be used.
+true` only). Data for direct recipients should contain an email address but if
+it is not known, an instance URL can also be provided.
 
 #### Request
 
@@ -1233,7 +1233,7 @@ This route can be only be called on the cozy instance of the sharer to revoke a
 group of the sharing. The parameter is the index of this recipient in the
 `groups` array of the sharing. The `removed` property for this group will be
 set to `true`, and it will revoke the members of this group unless they are
-still part of the sharing via another group.
+still part of the sharing via another group or as direct recipients.
 
 #### Request
 

docs/workers.md

@@ -352,9 +352,9 @@ The stack have 4 workers to power the sharings (internal usage only):
 
 ### Share-group
 
-When a contact is added or removed to a group, they should be added to the
-sharings of this group. The message is composed of the contact ID, the list
-of groups added and the list of groups removed.
+When a contact is added to or removed from a group, the change should be
+reflected in the group's sharings' recipients. The message is composed of the
+contact ID, the list of groups added and the list of groups removed.
 
 ### Share-track
 

model/contact/contact_test.go

@@ -14,7 +14,7 @@ import (
 	"github.com/stretchr/testify/require"
 )
 
-func TestFindContacts(t *testing.T) {
+func TestGetAllContacts(t *testing.T) {
 	config.UseTestFile(t)
 	instPrefix := prefixer.NewPrefixer(0, "cozy-test", "cozy-test")
 	t.Cleanup(func() { _ = couchdb.DeleteDB(instPrefix, consts.Contacts) })
@@ -76,7 +76,7 @@ func TestFindContacts(t *testing.T) {
 	require.NoError(t, json.Unmarshal([]byte(gaby), &doc.M))
 	require.NoError(t, couchdb.CreateDoc(instPrefix, &doc))
 
-	contacts, err := g.FindContacts(instPrefix)
+	contacts, err := g.GetAllContacts(instPrefix)
 	require.NoError(t, err)
 	require.Len(t, contacts, 1)
 	assert.Equal(t, contacts[0].PrimaryName(), "Gaby")

model/contact/contacts.go

@@ -84,9 +84,9 @@ func (c *Contact) PrimaryName() string {
 	return primary
 }
 
-// ByFamilyNameGivenNameEmailCozyURL returns a string that can be used for
-// sorting the contacts like in the contacts app.
-func (c *Contact) ByFamilyNameGivenNameEmailCozyURL() string {
+// SortingKey returns a string that can be used for sorting the contacts like
+// in the contacts app.
+func (c *Contact) SortingKey() string {
 	indexes, ok := c.Get("indexes").(map[string]interface{})
 	if !ok {
 		return c.PrimaryName()

model/contact/group.go

@@ -39,8 +39,8 @@ func FindGroup(db prefixer.Prefixer, groupID string) (*Group, error) {
 	return doc, err
 }
 
-// FindContacts returns the list of contacts inside this group.
-func (g *Group) FindContacts(db prefixer.Prefixer) ([]*Contact, error) {
+// GetAllContacts returns the list of contacts inside this group.
+func (g *Group) GetAllContacts(db prefixer.Prefixer) ([]*Contact, error) {
 	var docs []*Contact
 	req := &couchdb.FindRequest{
 		UseIndex: "by-groups",
@@ -65,8 +65,8 @@ func (g *Group) FindContacts(db prefixer.Prefixer) ([]*Contact, error) {
 
 	// XXX I didn't find a way to make a mango request with the correct sort
 	less := func(i, j int) bool {
-		a := docs[i].ByFamilyNameGivenNameEmailCozyURL()
-		b := docs[j].ByFamilyNameGivenNameEmailCozyURL()
+		a := docs[i].SortingKey()
+		b := docs[j].SortingKey()
 		return a < b
 	}
 	sort.Slice(docs, less)

model/sharing/group.go

@@ -25,7 +25,7 @@ import (
 type Group struct {
 	ID       string `json:"id,omitempty"`
 	Name     string `json:"name"`
-	AddedBy  int    `json:"addedBy"` // The index of the member who have added the group
+	AddedBy  int    `json:"addedBy"` // The index of the member who added the group
 	ReadOnly bool   `json:"read_only"`
 	Removed  bool   `json:"removed,omitempty"`
 }
@@ -37,7 +37,7 @@ func (s *Sharing) AddGroup(inst *instance.Instance, groupID string, readOnly boo
 	if err != nil {
 		return err
 	}
-	contacts, err := group.FindContacts(inst)
+	contacts, err := group.GetAllContacts(inst)
 	if err != nil {
 		return err
 	}
@@ -50,22 +50,26 @@ func (s *Sharing) AddGroup(inst *instance.Instance, groupID string, readOnly boo
 		if err != nil {
 			return err
 		}
-		s.Members[idx].Groups = append(s.Members[idx].Groups, groupIndex)
-		sort.Ints(s.Members[idx].Groups)
+		pos := sort.SearchInts(s.Members[idx].Groups, groupIndex)
+		if pos == len(s.Members[idx].Groups) || s.Members[idx].Groups[pos] != groupIndex {
+			s.Members[idx].Groups = append(s.Members[idx].Groups, groupIndex)
+			sort.Ints(s.Members[idx].Groups)
+		}
 	}
 
 	g := Group{ID: groupID, Name: group.Name(), AddedBy: 0, ReadOnly: readOnly}
 	s.Groups = append(s.Groups, g)
 	return nil
 }
 
-// RevokeGroup revoke a group of members on the sharer Cozy. After that, the
-// sharing is desactivated if there are no longer any active recipient.
+// RevokeGroup revokes a group of members on the sharer Cozy. After that, the
+// sharing is disabled if there are no longer any active recipient.
 func (s *Sharing) RevokeGroup(inst *instance.Instance, index int) error {
 	if !s.Owner {
 		return ErrInvalidSharing
 	}
 
+	var errm error
 	for i, m := range s.Members {
 		inGroup := false
 		for _, idx := range m.Groups {
@@ -89,13 +93,16 @@ func (s *Sharing) RevokeGroup(inst *instance.Instance, index int) error {
 		}
 		if m.OnlyInGroups && len(s.Members[i].Groups) == 0 {
 			if err := s.RevokeRecipient(inst, i); err != nil {
-				return err
+				errm = multierror.Append(errm, err)
 			}
 		}
 	}
 
 	s.Groups[index].Removed = true
-	return couchdb.UpdateDoc(inst, s)
+	if err := couchdb.UpdateDoc(inst, s); err != nil {
+		errm = multierror.Append(errm, err)
+	}
+	return errm
 }
 
 // UpdateGroups is called when a contact is added or removed to a group. It

model/sharing/member.go

@@ -66,8 +66,8 @@ type Member struct {
 	Email        string `json:"email,omitempty"`
 	Instance     string `json:"instance,omitempty"`
 	ReadOnly     bool   `json:"read_only,omitempty"`
-	OnlyInGroups bool   `json:"only_in_groups,omitempty"` // False if the member as been added as an io.cozy.contacts
-	Groups       []int  `json:"groups,omitempty"`         // The indexes of the groups
+	OnlyInGroups bool   `json:"only_in_groups,omitempty"` // False if the member has been added as an io.cozy.contacts
+	Groups       []int  `json:"groups,omitempty"`         // The indexes of the groups a member is part of
 }
 
 // PrimaryName returns the main name of this member
@@ -316,7 +316,7 @@ func (s *Sharing) DelegateAddContactsAndGroups(inst *instance.Instance, groupIDs
 		g := Group{ID: groupID, Name: group.Name(), ReadOnly: readOnly}
 		api.groups = append(api.groups, g)
 
-		contacts, err := group.FindContacts(inst)
+		contacts, err := group.GetAllContacts(inst)
 		if err != nil {
 			return err
 		}

model/sharing/sharing.go

@@ -485,7 +485,7 @@ func (s *Sharing) RevokePreviewPermissions(inst *instance.Instance) error {
 
 // RevokeRecipient revoke only one recipient on the sharer. After that, if the
 // sharing has still at least one active member, we keep it as is. Else, we
-// desactivate the sharing.
+// disable the sharing.
 func (s *Sharing) RevokeRecipient(inst *instance.Instance, index int) error {
 	if !s.Owner {
 		return ErrInvalidSharing