Skip to content

Commit

Permalink
Don't fail user creation if org exists (woodpecker-ci#4687)
Browse files Browse the repository at this point in the history
  • Loading branch information
pat-s authored Jan 10, 2025
1 parent d5233d8 commit 628c0e8
Show file tree
Hide file tree
Showing 3 changed files with 66 additions and 4 deletions.
10 changes: 8 additions & 2 deletions server/store/datastore/org.go
Original file line number Diff line number Diff line change
Expand Up @@ -77,9 +77,15 @@ func (s storage) orgDelete(sess *xorm.Session, id int64) error {
func (s storage) OrgFindByName(name string) (*model.Org, error) {
// sanitize
name = strings.ToLower(name)
// find
org := new(model.Org)
return org, wrapGet(s.engine.Where("name = ?", name).Get(org))
has, err := s.engine.Where("name = ?", name).Get(org)
if err != nil {
return nil, fmt.Errorf("failed to check if org exists: %w", err)
}
if !has {
return nil, nil
}
return org, nil
}

func (s storage) OrgRepoList(org *model.Org, p *model.ListOptions) ([]*model.Repo, error) {
Expand Down
21 changes: 19 additions & 2 deletions server/store/datastore/user.go
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,8 @@
package datastore

import (
"fmt"

"xorm.io/xorm"

"go.woodpecker-ci.org/woodpecker/v3/server/model"
Expand Down Expand Up @@ -59,9 +61,24 @@ func (s storage) CreateUser(user *model.User) error {
Name: user.Login,
IsUser: true,
}
err := s.orgCreate(org, sess)

existingOrg, err := s.OrgFindByName(org.Name)
if err != nil {
return err
return fmt.Errorf("failed to check if org exists: %w", err)
}

if existingOrg != nil {
if existingOrg.Name == user.Login {
err = s.OrgUpdate(org)
if err != nil {
return fmt.Errorf("failed to update existing org: %w", err)
}
}
} else {
err = s.orgCreate(org, sess)
if err != nil {
return fmt.Errorf("failed to create new org: %w", err)
}
}
user.OrgID = org.ID
// only Insert set auto created ID back to object
Expand Down
39 changes: 39 additions & 0 deletions server/store/datastore/users_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -94,3 +94,42 @@ func TestUsers(t *testing.T) {
_, err3 := store.GetUser(getUser.ID)
assert.Error(t, err3)
}

func TestCreateUserWithExistingOrg(t *testing.T) {
store, closer := newTestStore(t, new(model.User), new(model.Org), new(model.Perm))
defer closer()

existingOrg := &model.Org{
ForgeID: 1,
IsUser: true,
Name: "existingorg",
Private: false,
}

err := store.OrgCreate(existingOrg)
assert.NoError(t, err)
assert.EqualValues(t, "existingorg", existingOrg.Name)

// Create a new user with the same name as the existing organization
newUser := &model.User{
Login: "existingOrg",
Hash: "A",
}
err = store.CreateUser(newUser)
assert.NoError(t, err)

updatedOrg, err := store.OrgGet(existingOrg.ID)
assert.NoError(t, err)
assert.Equal(t, "existingorg", updatedOrg.Name)

newUser2 := &model.User{
Login: "new-user",
Hash: "B",
}
err = store.CreateUser(newUser2)
assert.NoError(t, err)

newOrg, err := store.OrgFindByName("new-user")
assert.NoError(t, err)
assert.Equal(t, "new-user", newOrg.Name)
}

0 comments on commit 628c0e8

Please sign in to comment.