fix: block duplicate resident IDs when editing a resident (EN-116)

backend/src/csv.go

@@ -180,10 +180,12 @@ func ValidateUserRow(row []string, rowNum int, headerMap *HeaderMapping, existin
 	}
 
 	if residentID != "" {
-		if existingRowNum, exists := existingResidentIDs[residentID]; exists {
+		// The intra-batch check compares IDs the way the database check does.
+		residentKey := models.NormalizeResidentID(residentID)
+		if existingRowNum, exists := existingResidentIDs[residentKey]; exists {
 			errors = append(errors, fmt.Sprintf("Duplicate Resident ID - also found in row %d", existingRowNum))
 		} else {
-			existingResidentIDs[residentID] = rowNum
+			existingResidentIDs[residentKey] = rowNum
 		}
 	}
 

backend/src/database/users.go

@@ -281,17 +281,30 @@ func (db *DB) UserIdentityExists(username string, doc string) (bool, bool) {
 		log.Error("Error checking username: ", err)
 	}
 	if doc != "" {
-		if err := db.Model(&models.User{}).
-			Select("1").
-			Where("doc_id = ?", doc).
-			Limit(1).
-			Find(&docIDExists).Error; err != nil {
+		exists, err := db.ResidentIDExists(doc)
+		if err != nil {
 			log.Error("Error checking doc_id: ", err)
 		}
+		docIDExists = exists
 	}
 	return usernameExists, docIDExists
 }
 
+// ResidentIDExists reports whether any user already has docID as their
+// resident ID, compared as models.NormalizeResidentID compares them.
+func (db *DB) ResidentIDExists(docID string) (bool, error) {
+	var exists bool
+	if err := db.Model(&models.User{}).
+		Select("1").
+		// The stored side of models.NormalizeResidentID; change both together.
+		Where("LOWER(TRIM(doc_id)) = ?", models.NormalizeResidentID(docID)).
+		Limit(1).
+		Find(&exists).Error; err != nil {
+		return false, newGetRecordsDBError(err, "users")
+	}
+	return exists, nil
+}
+
 func (db *DB) UpdateUser(user *models.User) error {
 	if user.ID == 0 {
 		return newUpdateDBError(errors.New("invalid user ID"), "users")

backend/src/handlers/user_handler.go

@@ -26,6 +26,12 @@ import (
 // Postgres and fails as a 22001 (500) instead of a 400.
 const maxDocIDLen = 32
 
+// A taken resident ID is a 409, so the frontend can show this message on the
+// Resident ID field.
+func newResidentIDInUseError() serviceError {
+	return NewServiceError(errors.New("resident id in use"), http.StatusConflict, "This resident ID is already in use")
+}
+
 func (srv *Server) registerUserRoutes() []routeDef {
 	resolver := UserRoleResolver("id")
 	return []routeDef{
@@ -238,7 +244,7 @@ func (srv *Server) handleCreateUser(w http.ResponseWriter, r *http.Request, log
 		return newBadRequestServiceError(err, "Username already exists")
 	}
 	if docExists {
-		return newBadRequestServiceError(err, "Resident ID already exists")
+		return newResidentIDInUseError()
 	}
 	reqForm.User.Username = stripNonAlphaChars(reqForm.User.Username, isUsernameChar)
 	err = srv.WithUserContext(r).CreateUser(&reqForm.User)
@@ -359,6 +365,18 @@ func (srv *Server) handleUpdateUser(w http.ResponseWriter, r *http.Request, log
 	if invalidDocID := validateResidentID(toUpdate.Role, models.FormatNullableString(user.DocID)); invalidDocID != "" {
 		return newBadRequestServiceError(errors.New("invalid resident id"), invalidDocID)
 	}
+	// Only a changed ID is checked, so saving a resident with their own ID
+	// always works, even one another resident already shares.
+	newDocID := models.NormalizeResidentID(models.FormatNullableString(user.DocID))
+	if newDocID != "" && newDocID != models.NormalizeResidentID(models.FormatNullableString(toUpdate.DocID)) {
+		exists, err := srv.Db.ResidentIDExists(newDocID)
+		if err != nil {
+			return newDatabaseServiceError(err)
+		}
+		if exists {
+			return newResidentIDInUseError()
+		}
+	}
 	invalidUser := validateUser(&user)
 	if invalidUser != "" {
 		return newBadRequestServiceError(errors.New("invalid user"), invalidUser)

backend/src/models/users.go

@@ -4,6 +4,7 @@ import (
 	"crypto/rand"
 	"math/big"
 	"slices"
+	"strings"
 	"time"
 
 	"github.com/UnlockedLabs/unlocked-contracts/users"
@@ -77,6 +78,12 @@ func (User) TableName() string {
 	return "users"
 }
 
+// NormalizeResidentID is the form two resident IDs are compared in: IDs that
+// differ only in case or surrounding spaces are the same ID.
+func NormalizeResidentID(id string) string {
+	return strings.ToLower(strings.TrimSpace(id))
+}
+
 func (usr *User) BeforeCreate(tx *gorm.DB) error {
 	if err := usr.DatabaseFields.BeforeCreate(tx); err != nil {
 		return err

backend/tests/integration/users_test.go

@@ -136,6 +136,47 @@ func TestCreateUserHandler(t *testing.T) {
 		_, err = env.CreateTestUser("importedresident", models.Student, facility.ID, "")
 		require.NoError(t, err)
 	})
+
+	t.Run("resident ID already in use", func(t *testing.T) {
+		facility, err := env.CreateTestFacility("Duplicate ID Facility")
+		require.NoError(t, err)
+		_, err = env.CreateTestUser("crtowner", models.Student, facility.ID, "RID-4000")
+		require.NoError(t, err)
+
+		tests := []struct {
+			name     string
+			username string
+			docID    string
+		}{
+			{"same ID", "crtsame", "RID-4000"},
+			{"different case and spaces", "crtcase", "  rid-4000 "},
+		}
+		for _, tt := range tests {
+			t.Run(tt.name, func(t *testing.T) {
+				var form struct {
+					User      models.User `json:"user"`
+					Providers []int       `json:"provider_platforms"`
+				}
+				form.User = models.User{
+					Username:   tt.username,
+					NameFirst:  "Test",
+					NameLast:   "User",
+					Role:       models.Student,
+					Email:      tt.username + "@example.com",
+					DocID:      models.StringPtr(tt.docID),
+					FacilityID: facility.ID,
+				}
+
+				NewRequest[struct {
+					User models.User `json:"user"`
+				}](env.Client, t, http.MethodPost, "/api/users", form).
+					WithTestClaims(&handlers.Claims{Role: models.SystemAdmin}).
+					Do().
+					ExpectStatus(http.StatusConflict).
+					ExpectMessage("This resident ID is already in use")
+			})
+		}
+	})
 }
 
 func TestUpdateUserHandler(t *testing.T) {
@@ -191,4 +232,58 @@ func TestUpdateUserHandler(t *testing.T) {
 
 		require.Equal(t, "Renamed", got.NameFirst)
 	})
+
+	// A resident ID belongs to one user across every facility, ignoring case
+	// and surrounding spaces.
+	t.Run("resident ID already in use", func(t *testing.T) {
+		otherFacility, err := env.CreateTestFacility("Other Facility")
+		require.NoError(t, err)
+		_, err = env.CreateTestUser("updowner", models.Student, otherFacility.ID, "RID-3000")
+		require.NoError(t, err)
+
+		tests := []struct {
+			name       string
+			username   string
+			ownID      string
+			newID      string
+			wantStatus int
+		}{
+			{"another resident's ID", "updtaken", "RID-3001", "RID-3000", http.StatusConflict},
+			{"another resident's ID in different case", "updtakencase", "RID-3002", "rid-3000", http.StatusConflict},
+			{"another resident's ID with spaces", "updtakenspace", "RID-3003", "  RID-3000 ", http.StatusConflict},
+			{"own ID unchanged", "updown", "RID-3004", "RID-3004", http.StatusOK},
+			{"own ID in different case", "updowncase", "RID-3005", "rid-3005", http.StatusOK},
+			{"an unused ID", "updunused", "RID-3006", "RID-3999", http.StatusOK},
+		}
+		for _, tt := range tests {
+			t.Run(tt.name, func(t *testing.T) {
+				user, err := env.CreateTestUser(tt.username, models.Student, facility.ID, tt.ownID)
+				require.NoError(t, err)
+
+				resp := patch(t, user.ID, map[string]any{"name_first": "Renamed", "doc_id": tt.newID}).
+					ExpectStatus(tt.wantStatus)
+				if tt.wantStatus == http.StatusConflict {
+					resp.ExpectMessage("This resident ID is already in use")
+					stored, err := env.DB.GetUserByID(user.ID)
+					require.NoError(t, err)
+					require.Equal(t, tt.ownID, models.FormatNullableString(stored.DocID))
+				}
+			})
+		}
+	})
+
+	// Sites may already have two residents sharing an ID. Editing one without
+	// changing the ID must still work.
+	t.Run("resident already sharing an ID can be renamed", func(t *testing.T) {
+		_, err := env.CreateTestUser("updshareda", models.Student, facility.ID, "RID-3100")
+		require.NoError(t, err)
+		user, err := env.CreateTestUser("updsharedb", models.Student, facility.ID, "RID-3100")
+		require.NoError(t, err)
+
+		got := patch(t, user.ID, map[string]any{"name_first": "Renamed", "doc_id": "RID-3100"}).
+			ExpectStatus(http.StatusOK).
+			GetData()
+
+		require.Equal(t, "Renamed", got.NameFirst)
+	})
 }

frontend/src/components/residents/AddResidentDialog.tsx

@@ -105,6 +105,8 @@ export function AddResidentDialog({
                 setKratosRegistered(response.data.kratos_registered);
                 setShowPasswordModal(true);
                 onSuccess(response.data.user);
+            } else if (response.status === 409) {
+                form.setError('doc_id', { message: response.message });
             } else {
                 toast.error(response.message ?? 'Failed to create resident');
             }

frontend/src/components/residents/ResidentDialogs.tsx

@@ -94,6 +94,8 @@ export function EditResidentDialog({
             );
             onOpenChange(false);
             onSuccess();
+        } else if (response.status === 409) {
+            form.setError('doc_id', { message: response.message });
         } else {
             toast.error(response.message ?? 'Failed to update resident');
         }