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.

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

Fix: Ensure Proper Uniqueness Checks and Handle Constraint Violations Gracefully #391

Open
wants to merge 5 commits into
base: dev
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
20 changes: 18 additions & 2 deletions internal/models/newsletter.go
Original file line number Diff line number Diff line change
Expand Up @@ -55,12 +55,19 @@ func (n *NewsLetter) GetDeletedNewsLetterById(db database.DatabaseManager, ID st
}

func (n *NewsLetter) CreateNewsLetter(db database.DatabaseManager) error {
err := db.CreateOneRecord(&n)
unique, uniqueErr := utility.CheckForUniqueness(db, &NewsLetter{}, "email", n.Email)

if uniqueErr != nil {
return uniqueErr
}
if !unique {
return fmt.Errorf("email already subscribed")
}

err := db.CreateOneRecord(&n)
if err != nil {
return err
}

return nil
}

Expand All @@ -76,6 +83,15 @@ func (n *NewsLetter) DeleteNewsLetter(db database.DatabaseManager) error {
}

func (n *NewsLetter) UpdateNewsLetter(db database.DatabaseManager) error {
unique, uniqueErr := utility.CheckForUniqueness(db, &NewsLetter{}, "email", n.Email)

if uniqueErr != nil {
return uniqueErr
}
if !unique {
return fmt.Errorf("email already subscribed")
}

_, err := db.SaveAllFields(&n)
return err
}
Expand Down
11 changes: 11 additions & 0 deletions internal/models/organisation.go
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ package models

import (
"errors"
"fmt"
"math"
"time"

Expand All @@ -10,6 +11,7 @@ import (
"github.com/gin-gonic/gin"
"github.com/hngprojects/hng_boilerplate_golang_web/pkg/repository/storage/database"
"github.com/hngprojects/hng_boilerplate_golang_web/pkg/repository/storage/postgresql"
"github.com/hngprojects/hng_boilerplate_golang_web/utility"
)

type Organisation struct {
Expand Down Expand Up @@ -64,6 +66,15 @@ type AddUserToOrgRequestModel struct {
}

func (c *Organisation) CreateOrganisation(db database.DatabaseManager) error {
unique, uniqueErr := utility.CheckForUniqueness(db, &Organisation{}, "email", c.Email)

if uniqueErr != nil {
return uniqueErr
}
if !unique {
return fmt.Errorf("the email %s already exists in this organisation", c.Email)
}

err := db.CreateOneRecord(&c)
if err != nil {
return err
Expand Down
17 changes: 13 additions & 4 deletions internal/models/role.go
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ import (
"gorm.io/gorm"

"github.com/hngprojects/hng_boilerplate_golang_web/pkg/repository/storage/database"
"github.com/hngprojects/hng_boilerplate_golang_web/utility"
)

type RoleName string
Expand Down Expand Up @@ -74,13 +75,21 @@ func (p PermissionList) Value() (driver.Value, error) {
}

func (r *OrgRole) CreateOrgRole(db database.DatabaseManager) error {
err := db.CreateOneRecord(&r)
isUniqueName, errName := utility.CheckForUniqueness(db, &OrgRole{}, "name", r.Name)
isUniqueId, errId := utility.CheckForUniqueness(db, &OrgRole{}, "organisation_id", r.OrganisationID)

if err != nil {
return err
if errName != nil {
return errName
}
if errId != nil {
return errId
}
if !isUniqueName || !isUniqueId {
return fmt.Errorf("Role with the name %s already exists in this organisation", r.Name)
}

return nil
createError := db.CreateOneRecord(&r)
return createError
}

func (r *OrgRole) DeleteOrgRole(db database.DatabaseManager) error {
Expand Down
64 changes: 52 additions & 12 deletions internal/models/seed/seed.go
Original file line number Diff line number Diff line change
Expand Up @@ -79,9 +79,9 @@ func SeedDatabase(db database.DatabaseManager) {

// Create organisations and categories
organisations := []models.Organisation{
{ID: utility.GenerateUUID(), Name: "Org1", Email: fmt.Sprintf(utility.RandomString(4) + "@email.com"), Description: "Description1", OwnerID: Userid1},
{ID: utility.GenerateUUID(), Name: "Org2", Email: fmt.Sprintf(utility.RandomString(4) + "@email.com"), Description: "Description2", OwnerID: Userid1},
{ID: utility.GenerateUUID(), Name: "Org3", Email: fmt.Sprintf(utility.RandomString(4) + "@email.com"), Description: "Description3", OwnerID: Userid2},
{ID: utility.GenerateUUID(), Name: "Org1", Email: fmt.Sprintln(utility.RandomString(4) + "@email.com"), Description: "Description1", OwnerID: Userid1},
{ID: utility.GenerateUUID(), Name: "Org2", Email: fmt.Sprintln(utility.RandomString(4) + "@email.com"), Description: "Description2", OwnerID: Userid1},
{ID: utility.GenerateUUID(), Name: "Org3", Email: fmt.Sprintln(utility.RandomString(4) + "@email.com"), Description: "Description3", OwnerID: Userid2},
}

var existingUser models.User
Expand Down Expand Up @@ -166,7 +166,6 @@ func SeedDatabase(db database.DatabaseManager) {
}

func SeedTestDatabase(db database.DatabaseManager) {

roles := []models.Role{
{ID: int(models.RoleIdentity.User), Name: "user", Description: "user related functions"},
{ID: int(models.RoleIdentity.SuperAdmin), Name: "super admin", Description: "super admin related functions"},
Expand All @@ -175,20 +174,16 @@ func SeedTestDatabase(db database.DatabaseManager) {
var existingRole models.Role
if err := db.DB().Where("id = ?", roles[0].ID).First(&existingRole).Error; err != nil {
if err == gorm.ErrRecordNotFound {

db.CreateMultipleRecords(&roles, len(roles))
} else {
fmt.Println("An error occurred: ", err)
}

} else {
fmt.Println("Roles already exist, skipping seeding.")
}

}

func SeedOrgRolesAndPermissions(db database.DatabaseManager) {

var organizations []models.Organisation
if err := db.DB().Find(&organizations).Error; err != nil {
fmt.Printf("Error fetching organizations: %v\n", err)
Expand All @@ -202,19 +197,64 @@ func SeedOrgRolesAndPermissions(db database.DatabaseManager) {
}

for _, role := range roles {
if err := db.CreateOneRecord(&role); err != nil {
fmt.Printf("Error creating role: %v\n", err)
isUniqueName, errName := utility.CheckForUniqueness(db, &models.OrgRole{}, "name", role.Name)
isUniqueId, errId := utility.CheckForUniqueness(db, &models.OrgRole{}, "organisation_id", role.OrganisationID)

if errName != nil {
fmt.Printf("Error checking role name uniqueness: %v", errName)
continue
}
if errId != nil {
fmt.Printf("Error checking role id uniqueness: %v", errId)
continue
}
if isUniqueName && isUniqueId {
if err := db.CreateOneRecord(&role); err != nil {
fmt.Printf("Error creating role: %v\n", err)
continue
}
} else {
fmt.Printf("Role %s already exists in organisation %s\n", role.Name, org.Name)
var existingRole models.OrgRole
result := db.DB().Where("name = ? AND organisation_id = ?", role.Name, role.OrganisationID).First(&existingRole)
if result.Error != nil {
fmt.Printf("Error fetching existing role: %v\n", result.Error)
continue
}
role.ID = existingRole.ID
fmt.Printf("Using existing role: ID %s", role.ID)
}

permissions := []models.Permission{
{ID: utility.GenerateUUID(), RoleID: role.ID, Category: "Transactions", PermissionList: map[string]bool{"can_view_transactions": true, "can_edit_transactions": true}},
{ID: utility.GenerateUUID(), RoleID: role.ID, Category: "Refunds", PermissionList: map[string]bool{"can_view_refunds": true}},
}

for _, permission := range permissions {
if err := db.CreateOneRecord(&permission); err != nil {
fmt.Printf("Error creating permission: %v\n", err)
isUniqueId, errId := utility.CheckForUniqueness(db, &models.Permission{}, "role_id", permission.RoleID)
isUniqueCategory, errCategory := utility.CheckForUniqueness(db, &models.Permission{}, "category", permission.Category)
if errId != nil {
fmt.Printf("Error checking permissions id uniqueness: %v", errId)
continue
}
if errCategory != nil {
fmt.Printf("Error checking permissions category uniqueness: %v", errCategory)
continue
}
if isUniqueId && isUniqueCategory {
if err := db.CreateOneRecord(&permission); err != nil {
fmt.Printf("Error creating permission: %v\n", err)
}
} else {
fmt.Printf("Category %s already exists for role ID %s in permissions\n", permission.Category, permission.RoleID)
var existingPermission models.Permission
result := db.DB().Where("role_id = ? AND category = ?", permission.RoleID, permission.Category).First(&existingPermission)
if result.Error != nil {
fmt.Printf("Error fetching existing permission: %v\n", result.Error)
continue
}
permission.ID = existingPermission.ID
fmt.Printf("Using existing permission: ID %s", permission.ID)
}
}
}
Expand Down
53 changes: 51 additions & 2 deletions internal/models/superadmin.go
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ package models

import (
"errors"
"fmt"
"time"

"github.com/hngprojects/hng_boilerplate_golang_web/pkg/repository/storage/database"
Expand Down Expand Up @@ -95,6 +96,18 @@ func (u *UserRegionTimezoneLanguage) CreateUserRegion(db database.DatabaseManage
}

func (l *Language) CreateLanguage(db database.DatabaseManager) error {
isUniqueName, errName := utility.CheckForUniqueness(db, &Language{}, "name", l.Name)
isUniqueCode, errCode := utility.CheckForUniqueness(db, &Language{}, "code", l.Code)

if errName != nil {
return errName
}
if errCode != nil {
return errCode
}
if !isUniqueName || !isUniqueCode {
return fmt.Errorf("name already exists")
}
err := db.CreateOneRecord(&l)

if err != nil {
Expand All @@ -105,8 +118,20 @@ func (l *Language) CreateLanguage(db database.DatabaseManager) error {
}

func (t *Timezone) CreateTimeZone(db database.DatabaseManager) error {
err := db.CreateOneRecord(&t)
uniqueTime, uniqueErrTime := utility.CheckForUniqueness(db, &Timezone{}, "timezone", t.Timezone)
uniqueGmt, uniqueErrGmt := utility.CheckForUniqueness(db, &Timezone{}, "gmt_offset", t.GmtOffset)

if uniqueErrTime != nil {
return uniqueErrTime
}
if uniqueErrGmt != nil {
return uniqueErrGmt
}
if !uniqueTime || !uniqueGmt {
return fmt.Errorf("timezone already exists")
}

err := db.CreateOneRecord(&t)
if err != nil {
return err
}
Expand All @@ -115,8 +140,20 @@ func (t *Timezone) CreateTimeZone(db database.DatabaseManager) error {
}

func (r *Region) CreateRegion(db database.DatabaseManager) error {
err := db.CreateOneRecord(&r)
isUniqueName, errName := utility.CheckForUniqueness(db, &Region{}, "name", r.Name)
isUniqueCode, errCode := utility.CheckForUniqueness(db, &Region{}, "code", r.Code)

if errName != nil {
return errName
}
if errCode != nil {
return errCode
}
if !isUniqueName || !isUniqueCode {
return fmt.Errorf("name already exists")
}

err := db.CreateOneRecord(&r)
if err != nil {
return err
}
Expand Down Expand Up @@ -181,6 +218,18 @@ func (t *Timezone) GetTimezoneByID(db database.DatabaseManager, ID string) (Time
}

func (t *Timezone) UpdateTimeZone(db database.DatabaseManager) error {
uniqueTime, uniqueErrTime := utility.CheckForUniqueness(db, &Timezone{}, "timezone", t.Timezone)
uniqueGmt, uniqueErrGmt := utility.CheckForUniqueness(db, &Timezone{}, "gmt_offset", t.GmtOffset)

if uniqueErrTime != nil {
return uniqueErrTime
}
if uniqueErrGmt != nil {
return uniqueErrGmt
}
if !uniqueTime || !uniqueGmt {
return fmt.Errorf("timezone already exists")
}
_, err := db.SaveAllFields(&t)
return err
}
2 changes: 1 addition & 1 deletion pkg/controller/newsletter/newsletter.go
Original file line number Diff line number Diff line change
Expand Up @@ -75,7 +75,7 @@ func (base *Controller) SubscribeNewsLetter(c *gin.Context) {
err = service.NewsLetterSubscribe(&req, base.Db.Postgresql.DB())
if err != nil {
if err == models.ErrEmailAlreadySubscribed {
rd := utility.BuildErrorResponse(http.StatusConflict, "error", "Email already subscribed", nil, nil)
rd := utility.BuildErrorResponse(http.StatusConflict, "error", "email already subscribed", nil, nil)
c.JSON(http.StatusConflict, rd)
} else {
rd := utility.BuildErrorResponse(http.StatusBadRequest, "error", "Failed to subscribe", err, nil)
Expand Down
2 changes: 1 addition & 1 deletion tests/test_newsletter/create_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -82,7 +82,7 @@ func TestPostNewsletter_CheckDuplicateEmail(t *testing.T) {

response := tst.ParseResponse(resp)
tst.AssertStatusCode(t, resp.Code, http.StatusConflict)
tst.AssertResponseMessage(t, response["message"].(string), "Email already subscribed")
tst.AssertResponseMessage(t, response["message"].(string), "email already subscribed")
}

func TestPostNewsletter_SaveData(t *testing.T) {
Expand Down
27 changes: 27 additions & 0 deletions tests/test_organisation/create_roles_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -163,4 +163,31 @@ func TestCreateOrgRole(t *testing.T) {
response := tests.ParseResponse(resp)
tests.AssertResponseMessage(t, response["message"].(string), "Validation failed")
})

t.Run("Duplicate Create Org Role", func(t *testing.T) {
router, orgController := setup()

loginData := models.LoginRequestModel{
Email: adminUser.Email,
Password: "password",
}
token := tests.GetLoginToken(t, router, *orgController, loginData)

role := models.OrgRole{
Name: fmt.Sprintf("Admin Role-%v", utility.RandomString(5)),
Description: "New role description",
}
roleJSON, _ := json.Marshal(role)

duplicateReq, _ := http.NewRequest(http.MethodPost, fmt.Sprintf("/api/v1/organisations/%s/roles", orgID), bytes.NewBuffer(roleJSON))
duplicateReq.Header.Set("Content-Type", "application/json")
duplicateReq.Header.Set("Authorization", fmt.Sprintf("Bearer %s", token))

duplicateResp := httptest.NewRecorder()
router.ServeHTTP(duplicateResp, duplicateReq)

tests.AssertStatusCode(t, duplicateResp.Code, http.StatusBadRequest)
duplicateResponse := tests.ParseResponse(duplicateResp)
tests.AssertResponseMessage(t, duplicateResponse["message"].(string), fmt.Sprintf("Role with the name %s already exists in this organisation", role.Name))
})
}
1 change: 1 addition & 0 deletions tests/test_organisation/update_roles_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -172,6 +172,7 @@ func TestUpdateOrgRole(t *testing.T) {
response := tests.ParseResponse(resp)
tests.AssertResponseMessage(t, response["message"].(string), "Validation failed")
})

}

func TestUpdateOrgPermissions(t *testing.T) {
Expand Down
2 changes: 2 additions & 0 deletions tests/test_superadmin/language_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -127,6 +127,8 @@ func TestAddToLanguage(t *testing.T) {
router.ServeHTTP(resp, req)

tests.AssertStatusCode(t, resp.Code, http.StatusBadRequest)
response := tests.ParseResponse(resp)
tests.AssertResponseMessage(t, response["message"].(string), "name already exists")
})

t.Run("Unauthorized Access", func(t *testing.T) {
Expand Down
2 changes: 1 addition & 1 deletion tests/test_superadmin/region_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -212,7 +212,7 @@ func TestGetRegions(t *testing.T) {
tests.AssertResponseMessage(t, response["message"].(string), "Regions retrieved successfully")
})

t.Run("Successful Get Regions for user", func(t *testing.T) {
t.Run("Duplicate Get Regions for user", func(t *testing.T) {
router, authController := setup()
loginData := models.LoginRequestModel{
Email: regularUser.Email,
Expand Down
Loading
Loading