diff --git a/Makefile b/Makefile index 650fece301..b017c90556 100644 --- a/Makefile +++ b/Makefile @@ -55,6 +55,7 @@ clean: .PHONY: test test: go test -cover -coverprofile cover.out $(PACKAGES) + go tool cover -html=cover.out -o cover.html required-gofmt-version: @go version | grep -q '\(1.7\|1.8\|1.9\|1.10\)' || { echo "We require go version 1.7, 1.8, 1.9 or 1.10 to format code" >&2 && exit 1; } diff --git a/models/error.go b/models/error.go index 2981182257..2035cf7b6e 100644 --- a/models/error.go +++ b/models/error.go @@ -479,3 +479,19 @@ func IsErrUserIsMemberOfTeam(err error) bool { func (err ErrUserIsMemberOfTeam) Error() string { return fmt.Sprintf("This user is already a member of that team. [Team ID: %d, User ID: %d]", err.TeamID, err.UserID) } + +// ErrCannotDeleteLastTeamMember represents an error where a user wants to delete the last member of a team (probably himself) +type ErrCannotDeleteLastTeamMember struct { + TeamID int64 + UserID int64 +} + +// IsErrCannotDeleteLastTeamMember checks if an error is ErrCannotDeleteLastTeamMember. +func IsErrCannotDeleteLastTeamMember(err error) bool { + _, ok := err.(ErrCannotDeleteLastTeamMember) + return ok +} + +func (err ErrCannotDeleteLastTeamMember) Error() string { + return fmt.Sprintf("This user is already a member of that team. [Team ID: %d, User ID: %d]", err.TeamID, err.UserID) +} diff --git a/models/fixtures/team_members.yml b/models/fixtures/team_members.yml new file mode 100644 index 0000000000..09b24a20da --- /dev/null +++ b/models/fixtures/team_members.yml @@ -0,0 +1,7 @@ +- + team_id: 1 + user_id: 1 + admin: true +- + team_id: 1 + user_id: 2 diff --git a/models/fixtures/users.yml b/models/fixtures/users.yml index 8c40e8ce1a..e278cd49fc 100644 --- a/models/fixtures/users.yml +++ b/models/fixtures/users.yml @@ -2,4 +2,14 @@ id: 1 username: 'user1' password: '1234' - email: 'johndoe@example.com' \ No newline at end of file + email: 'johndoe@example.com' +- + id: 2 + username: 'user2' + password: '1234' + email: 'johndoe@example.com' +- + id: 3 + username: 'user3' + password: '1234' + email: 'johndoe@example.com' diff --git a/models/team_members_delete.go b/models/team_members_delete.go index 9dd740641f..1dda4b8e64 100644 --- a/models/team_members_delete.go +++ b/models/team_members_delete.go @@ -2,6 +2,15 @@ package models // Delete deletes a user from a team func (tm *TeamMember) Delete() (err error) { + + total, err := x.Where("team_id = ?", tm.TeamID).Count(&TeamMember{}) + if err != nil { + return + } + if total == 1 { + return ErrCannotDeleteLastTeamMember{tm.TeamID, tm.UserID} + } + _, err = x.Where("team_id = ? AND user_id = ?", tm.TeamID, tm.UserID).Delete(&TeamMember{}) return } diff --git a/models/team_members_test.go b/models/team_members_test.go new file mode 100644 index 0000000000..2e8000220b --- /dev/null +++ b/models/team_members_test.go @@ -0,0 +1,63 @@ +package models + +import ( + "testing" + "github.com/stretchr/testify/assert" +) + +func TestTeamMember_Create(t *testing.T) { + + // Dummy team member + dummyteammember := TeamMember{ + TeamID: 1, + UserID: 3, + } + + // Doer + doer, _, err := GetUserByID(1) + assert.NoError(t, err) + + // Insert a new team member + assert.True(t, dummyteammember.CanCreate(&doer)) + err = dummyteammember.Create(&doer) + assert.NoError(t, err) + + // Check he's in there + team := Team{ID:1} + err = team.ReadOne() + assert.NoError(t, err) + assert.Equal(t, 3, len(team.Members)) + + // Try inserting a user twice + err = dummyteammember.Create(&doer) + assert.Error(t, err) + assert.True(t, IsErrUserIsMemberOfTeam(err)) + + // Delete it + assert.True(t, dummyteammember.CanDelete(&doer)) + err = dummyteammember.Delete() + assert.NoError(t, err) + + // Delete the other one + tm := TeamMember{TeamID:1, UserID:2} + err = tm.Delete() + assert.NoError(t, err) + + // Try deleting the last one + tm = TeamMember{TeamID:1, UserID:1} + err = tm.Delete() + assert.Error(t, err) + assert.True(t, IsErrCannotDeleteLastTeamMember(err)) + + // Try inserting a user which does not exist + dummyteammember.UserID = 9484 + err = dummyteammember.Create(&doer) + assert.Error(t, err) + assert.True(t, IsErrUserDoesNotExist(err)) + + // Try adding a user to a team which does not exist + tm = TeamMember{TeamID:94824, UserID:1} + err = tm.Create(&doer) + assert.Error(t, err) + assert.True(t, IsErrTeamDoesNotExist(err)) +} diff --git a/models/teams_test.go b/models/teams_test.go index c57d6c898f..696a39ca13 100644 --- a/models/teams_test.go +++ b/models/teams_test.go @@ -36,7 +36,7 @@ func TestTeam_Create(t *testing.T) { assert.NoError(t, err) assert.Equal(t, reflect.TypeOf(ts).Kind(), reflect.Slice) s := reflect.ValueOf(ts) - assert.Equal(t, 1, s.Len()) + assert.Equal(t, 2, s.Len()) // Check inserting it with an empty name dummyteam.Name = "" diff --git a/routes/crud/delete.go b/routes/crud/delete.go index b81b7b6924..c4b966f71e 100644 --- a/routes/crud/delete.go +++ b/routes/crud/delete.go @@ -37,6 +37,10 @@ func (c *WebHandler) DeleteWeb(ctx echo.Context) error { return echo.NewHTTPError(http.StatusNotFound, "This team does not exist.") } + if models.IsErrCannotDeleteLastTeamMember(err) { + return echo.NewHTTPError(http.StatusBadRequest, "You cannot delete the last member of a team.") + } + return echo.NewHTTPError(http.StatusInternalServerError) }