graph: Add 'terminationDate' property to educationSchool

Schools can now have a terminationDate set. Schools can only be deleted if the
terminationDate is in the past. Schools without a terminationDate cannot be deleted.

services/graph/pkg/identity/ldap_education_school.go

@@ -4,6 +4,7 @@ import (
 	"context"
 	"errors"
 	"fmt"
+	"time"
 
 	"github.com/go-ldap/ldap/v3"
 	"github.com/gofrs/uuid"
@@ -31,9 +32,10 @@ type educationConfig struct {
 }
 
 type schoolAttributeMap struct {
-	displayName  string
-	schoolNumber string
-	id           string
+	displayName     string
+	schoolNumber    string
+	id              string
+	terminationDate string
 }
 
 type schoolUpdateOperation uint8
@@ -41,10 +43,14 @@ type schoolUpdateOperation uint8
 const (
 	tooManyValues schoolUpdateOperation = iota
 	schoolUnchanged
-	displayNameUpdated
-	schoolNumberUpdated
+	schoolRenamed
+	schoolPropertiesUpdated
 )
 
+const ldapDateFormat = "20060102150405Z0700"
+
+var errNotSet = errors.New("Attribute not set")
+
 func defaultEducationConfig() educationConfig {
 	return educationConfig{
 		schoolObjectClass:       "ocEducationSchool",
@@ -95,9 +101,10 @@ func newEducationConfig(config config.LDAP) (educationConfig, error) {
 
 func newSchoolAttributeMap() schoolAttributeMap {
 	return schoolAttributeMap{
-		displayName:  "ou",
-		schoolNumber: "ocEducationSchoolNumber",
-		id:           "owncloudUUID",
+		displayName:     "ou",
+		schoolNumber:    "ocEducationSchoolNumber",
+		id:              "owncloudUUID",
+		terminationDate: "ocEducationSchoolTerminationTimestamp",
 	}
 }
 
@@ -153,19 +160,38 @@ func (i *LDAP) UpdateEducationSchoolOperation(
 	schoolUpdate libregraph.EducationSchool,
 	currentSchool libregraph.EducationSchool,
 ) schoolUpdateOperation {
-	providedDisplayName := schoolUpdate.GetDisplayName()
-	schoolNumber := schoolUpdate.GetSchoolNumber()
 
-	if providedDisplayName != "" && schoolNumber != "" {
+	providedDisplayName, displayNameIsSet := schoolUpdate.GetDisplayNameOk()
+	if displayNameIsSet {
+		if *providedDisplayName == "" || *providedDisplayName == currentSchool.GetDisplayName() {
+			// The school name hasn't changed
+			displayNameIsSet = false
+		}
+	}
+
+	var propertiesUpdated bool
+
+	switch {
+	case schoolUpdate.HasSchoolNumber():
+		if schoolUpdate.GetSchoolNumber() != "" && schoolUpdate.GetSchoolNumber() != currentSchool.GetSchoolNumber() {
+			propertiesUpdated = true
+		}
+	case schoolUpdate.HasTerminationDate():
+		if schoolUpdate.GetTerminationDate() != currentSchool.GetTerminationDate() {
+			propertiesUpdated = true
+		}
+	}
+
+	if propertiesUpdated && displayNameIsSet {
 		return tooManyValues
 	}
 
-	if providedDisplayName != "" && providedDisplayName != currentSchool.GetDisplayName() {
-		return displayNameUpdated
+	if displayNameIsSet {
+		return schoolRenamed
 	}
 
-	if schoolNumber != "" && schoolNumber != currentSchool.GetSchoolNumber() {
-		return schoolNumberUpdated
+	if propertiesUpdated {
+		return schoolPropertiesUpdated
 	}
 
 	return schoolUnchanged
@@ -199,18 +225,34 @@ func (i *LDAP) updateDisplayName(ctx context.Context, dn string, providedDisplay
 	return nil
 }
 
-// updateSchoolNumber checks if a school number is already taken, and if not updates the school number
-func (i *LDAP) updateSchoolNumber(ctx context.Context, dn string, schoolNumber string) error {
+// updateSchoolProperties updates the properties (other that displayName) of a school.
+// It checks if a school number is already taken, before updating the school number
+func (i *LDAP) updateSchoolProperties(ctx context.Context, dn string, currentSchool, updatedSchool libregraph.EducationSchool) error {
 	logger := i.logger.SubloggerWithRequestID(ctx)
-	_, err := i.getSchoolByNumberOrID(schoolNumber)
-	if err == nil {
-		errmsg := fmt.Sprintf("school number '%s' already exists", schoolNumber)
-		err = fmt.Errorf(errmsg)
-		return err
-	}
 
 	mr := ldap.NewModifyRequest(dn, nil)
-	mr.Replace(i.educationConfig.schoolAttributeMap.schoolNumber, []string{schoolNumber})
+	if updatedSchoolNumber, ok := updatedSchool.GetSchoolNumberOk(); ok {
+		if *updatedSchoolNumber != "" && currentSchool.GetSchoolNumber() != *updatedSchoolNumber {
+			_, err := i.getSchoolByNumberOrID(*updatedSchoolNumber)
+			if err == nil {
+				errmsg := fmt.Sprintf("school number '%s' already exists", *updatedSchoolNumber)
+				err = fmt.Errorf(errmsg)
+				return err
+			}
+			mr.Replace(i.educationConfig.schoolAttributeMap.schoolNumber, []string{*updatedSchoolNumber})
+		}
+	}
+
+	if updatedTerminationDate, ok := updatedSchool.GetTerminationDateOk(); ok {
+		if updatedTerminationDate == nil && currentSchool.HasTerminationDate() {
+			// Delete the termination date
+			mr.Delete(i.educationConfig.schoolAttributeMap.terminationDate, []string{})
+		}
+		if updatedTerminationDate != nil && *updatedTerminationDate != currentSchool.GetTerminationDate() {
+			ldapDateTime := updatedTerminationDate.UTC().Format(ldapDateFormat)
+			mr.Replace(i.educationConfig.schoolAttributeMap.terminationDate, []string{ldapDateTime})
+		}
+	}
 
 	if err := i.conn.Modify(mr); err != nil {
 		var lerr *ldap.Error
@@ -234,13 +276,6 @@ func (i *LDAP) UpdateEducationSchool(ctx context.Context, numberOrID string, sch
 		return nil, ErrReadOnly
 	}
 
-	providedDisplayName := school.GetDisplayName()
-	schoolNumber := school.GetSchoolNumber()
-
-	if providedDisplayName != "" && schoolNumber != "" {
-		return nil, fmt.Errorf("school name and school number cannot be updated in the same request")
-	}
-
 	e, err := i.getSchoolByNumberOrID(numberOrID)
 	if err != nil {
 		return nil, err
@@ -252,13 +287,13 @@ func (i *LDAP) UpdateEducationSchool(ctx context.Context, numberOrID string, sch
 		return nil, fmt.Errorf("school name and school number cannot be updated in the same request")
 	case schoolUnchanged:
 		logger.Debug().Str("backend", "ldap").Msg("UpdateEducationSchool: Nothing changed")
-		return i.createSchoolModelFromLDAP(e), nil
-	case displayNameUpdated:
-		if err := i.updateDisplayName(ctx, e.DN, providedDisplayName); err != nil {
+		return currentSchool, nil
+	case schoolRenamed:
+		if err := i.updateDisplayName(ctx, e.DN, school.GetDisplayName()); err != nil {
 			return nil, err
 		}
-	case schoolNumberUpdated:
-		if err := i.updateSchoolNumber(ctx, e.DN, schoolNumber); err != nil {
+	case schoolPropertiesUpdated:
+		if err := i.updateSchoolProperties(ctx, e.DN, *currentSchool, school); err != nil {
 			return nil, err
 		}
 	}
@@ -318,11 +353,7 @@ func (i *LDAP) GetEducationSchools(ctx context.Context) ([]*libregraph.Education
 		i.educationConfig.schoolScope,
 		ldap.NeverDerefAliases, 0, 0, false,
 		filter,
-		[]string{
-			i.educationConfig.schoolAttributeMap.displayName,
-			i.educationConfig.schoolAttributeMap.id,
-			i.educationConfig.schoolAttributeMap.schoolNumber,
-		},
+		i.getEducationSchoolAttrTypes(),
 		nil,
 	)
 	i.logger.Debug().Str("backend", "ldap").
@@ -640,11 +671,7 @@ func (i *LDAP) getSchoolByFilter(filter string) (*ldap.Entry, error) {
 		i.educationConfig.schoolScope,
 		ldap.NeverDerefAliases, 1, 0, false,
 		filter,
-		[]string{
-			i.educationConfig.schoolAttributeMap.displayName,
-			i.educationConfig.schoolAttributeMap.id,
-			i.educationConfig.schoolAttributeMap.schoolNumber,
-		},
+		i.getEducationSchoolAttrTypes(),
 		nil,
 	)
 	i.logger.Debug().Str("backend", "ldap").
@@ -682,11 +709,18 @@ func (i *LDAP) createSchoolModelFromLDAP(e *ldap.Entry) *libregraph.EducationSch
 	id := i.getID(e)
 	schoolNumber := i.getSchoolNumber(e)
 
+	t, err := i.getTerminationDate(e)
+	if err != nil && !errors.Is(err, errNotSet) {
+		i.logger.Error().Err(err).Str("dn", e.DN).Msg("Error reading termination date for LDAP entry")
+	}
 	if id != "" && displayName != "" && schoolNumber != "" {
 		school := libregraph.NewEducationSchool()
 		school.SetDisplayName(displayName)
 		school.SetSchoolNumber(schoolNumber)
 		school.SetId(id)
+		if t != nil {
+			school.SetTerminationDate(*t)
+		}
 		return school
 	}
 	i.logger.Warn().Str("dn", e.DN).Str("id", id).Str("displayName", displayName).Str("schoolNumber", schoolNumber).Msg("Invalid School. Missing required attribute")
@@ -707,3 +741,25 @@ func (i *LDAP) getDisplayName(e *ldap.Entry) string {
 	displayName := e.GetEqualFoldAttributeValue(i.educationConfig.schoolAttributeMap.displayName)
 	return displayName
 }
+
+func (i *LDAP) getTerminationDate(e *ldap.Entry) (*time.Time, error) {
+	dateString := e.GetEqualFoldAttributeValue(i.educationConfig.schoolAttributeMap.terminationDate)
+	if dateString == "" {
+		return nil, errNotSet
+	}
+	t, err := time.Parse(ldapDateFormat, dateString)
+	if err != nil {
+		err = fmt.Errorf("Error parsing LDAP date: '%s': %w", dateString, err)
+		return nil, err
+	}
+	return &t, nil
+}
+
+func (i *LDAP) getEducationSchoolAttrTypes() []string {
+	return []string{
+		i.educationConfig.schoolAttributeMap.displayName,
+		i.educationConfig.schoolAttributeMap.id,
+		i.educationConfig.schoolAttributeMap.schoolNumber,
+		i.educationConfig.schoolAttributeMap.terminationDate,
+	}
+}

services/graph/pkg/identity/ldap_education_school_test.go

@@ -3,6 +3,7 @@ package identity
 import (
 	"context"
 	"testing"
+	"time"
 
 	"github.com/go-ldap/ldap/v3"
 	libregraph "github.com/owncloud/libre-graph-api-go"
@@ -49,6 +50,13 @@ var schoolEntry1 = ldap.NewEntry("ou=Test School1",
 		"ocEducationSchoolNumber": {"0042"},
 		"owncloudUUID":            {"hijk-defg"},
 	})
+var schoolEntryWithTermination = ldap.NewEntry("ou=Test School",
+	map[string][]string{
+		"ou":                                    {"Test School"},
+		"ocEducationSchoolNumber":               {"0123"},
+		"owncloudUUID":                          {"abcd-defg"},
+		"ocEducationSchoolTerminationTimestamp": {"20420131120000Z"},
+	})
 
 var (
 	filterSchoolSearchByIdExisting        = "(&(objectClass=ocEducationSchool)(|(owncloudUUID=abcd-defg)(ocEducationSchoolNumber=abcd-defg)))"
@@ -59,8 +67,20 @@ var (
 
 func TestCreateEducationSchool(t *testing.T) {
 	lm := &mocks.Client{}
-	lm.On("Add", mock.Anything).
-		Return(nil)
+
+	ldapSchoolAddRequestMatcher := func(ar *ldap.AddRequest) bool {
+		if ar.DN != "ou=Test School," {
+			return false
+		}
+		for _, attr := range ar.Attributes {
+			if attr.Type == "ocEducationSchoolTerminationTimestamp" {
+				return false
+			}
+		}
+		return true
+	}
+
+	lm.On("Add", mock.MatchedBy(ldapSchoolAddRequestMatcher)).Return(nil)
 
 	lm.On("Search", mock.Anything).
 		Return(
@@ -84,6 +104,56 @@ func TestCreateEducationSchool(t *testing.T) {
 	assert.Equal(t, res_school.GetDisplayName(), school.GetDisplayName())
 	assert.Equal(t, res_school.GetId(), school.GetId())
 	assert.Equal(t, res_school.GetSchoolNumber(), school.GetSchoolNumber())
+	assert.False(t, res_school.HasTerminationDate())
+}
+
+func TestUpdateEducationSchoolTerminationDate(t *testing.T) {
+	lm := &mocks.Client{}
+
+	ldapSchoolTerminationRequestMatcher := func(mr *ldap.ModifyRequest) bool {
+		if mr.DN != "ou=Test School" {
+			return false
+		}
+		for _, mod := range mr.Changes {
+			if mod.Operation == ldap.ReplaceAttribute &&
+				mod.Modification.Type == "ocEducationSchoolTerminationTimestamp" &&
+				mod.Modification.Vals[0] == "20420131120000Z" {
+				return true
+			}
+		}
+		return false
+	}
+	lm.On("Modify", mock.MatchedBy(ldapSchoolTerminationRequestMatcher)).Return(nil)
+	lm.On("Search", mock.Anything).
+		Return(
+			&ldap.SearchResult{
+				Entries: []*ldap.Entry{schoolEntry},
+			},
+			nil).
+		Once()
+	lm.On("Search", mock.Anything).
+		Return(
+			&ldap.SearchResult{
+				Entries: []*ldap.Entry{schoolEntryWithTermination},
+			},
+			nil).
+		Once()
+
+	b, err := getMockedBackend(lm, eduConfig, &logger)
+	assert.Nil(t, err)
+	assert.NotEqual(t, "", b.educationConfig.schoolObjectClass)
+	school := libregraph.NewEducationSchool()
+	terminationTime := time.Date(2042, time.January, 31, 12, 0, 0, 0, time.UTC)
+	school.SetTerminationDate(terminationTime)
+	res_school, err := b.UpdateEducationSchool(context.Background(), "abcd-defg", *school)
+	lm.AssertNumberOfCalls(t, "Search", 2)
+	assert.Nil(t, err)
+	assert.NotNil(t, res_school)
+	assert.Equal(t, "Test School", res_school.GetDisplayName())
+	assert.Equal(t, "abcd-defg", res_school.GetId())
+	assert.Equal(t, "0123", res_school.GetSchoolNumber())
+	assert.True(t, res_school.HasTerminationDate())
+	assert.True(t, terminationTime.Equal(res_school.GetTerminationDate()))
 }
 
 func TestUpdateEducationSchoolOperation(t *testing.T) {
@@ -96,9 +166,15 @@ func TestUpdateEducationSchoolOperation(t *testing.T) {
 		expectedOperation schoolUpdateOperation
 	}{
 		{
-			name:              "Test using school with both number and name",
+			name:              "Test using school with both number and name, unchanged",
 			displayName:       testSchoolName,
 			schoolNumber:      testSchoolNumber,
+			expectedOperation: schoolUnchanged,
+		},
+		{
+			name:              "Test using school with both number and name, unchanged",
+			displayName:       "A new name",
+			schoolNumber:      "9876",
 			expectedOperation: tooManyValues,
 		},
 		{
@@ -114,12 +190,12 @@ func TestUpdateEducationSchoolOperation(t *testing.T) {
 		{
 			name:              "Test new name",
 			displayName:       "Something new",
-			expectedOperation: displayNameUpdated,
+			expectedOperation: schoolRenamed,
 		},
 		{
 			name:              "Test new number",
 			schoolNumber:      "9876",
-			expectedOperation: schoolNumberUpdated,
+			expectedOperation: schoolPropertiesUpdated,
 		},
 	}
 
@@ -186,7 +262,7 @@ func TestDeleteEducationSchool(t *testing.T) {
 			Scope:      2,
 			SizeLimit:  1,
 			Filter:     tt.filter,
-			Attributes: []string{"ou", "owncloudUUID", "ocEducationSchoolNumber"},
+			Attributes: []string{"ou", "owncloudUUID", "ocEducationSchoolNumber", "ocEducationSchoolTerminationTimestamp"},
 			Controls:   []ldap.Control(nil),
 		}
 		if tt.expectedItemNotFound {
@@ -255,7 +331,7 @@ func TestGetEducationSchool(t *testing.T) {
 			Scope:      2,
 			SizeLimit:  1,
 			Filter:     tt.filter,
-			Attributes: []string{"ou", "owncloudUUID", "ocEducationSchoolNumber"},
+			Attributes: []string{"ou", "owncloudUUID", "ocEducationSchoolNumber", "ocEducationSchoolTerminationTimestamp"},
 			Controls:   []ldap.Control(nil),
 		}
 		if tt.expectedItemNotFound {
@@ -289,7 +365,7 @@ func TestGetEducationSchools(t *testing.T) {
 		Scope:      2,
 		SizeLimit:  0,
 		Filter:     "(objectClass=ocEducationSchool)",
-		Attributes: []string{"ou", "owncloudUUID", "ocEducationSchoolNumber"},
+		Attributes: []string{"ou", "owncloudUUID", "ocEducationSchoolNumber", "ocEducationSchoolTerminationTimestamp"},
 		Controls:   []ldap.Control(nil),
 	}
 	lm.On("Search", sr1).Return(&ldap.SearchResult{Entries: []*ldap.Entry{schoolEntry, schoolEntry1}}, nil)
@@ -306,7 +382,7 @@ var schoolByIDSearch1 *ldap.SearchRequest = &ldap.SearchRequest{
 	Scope:      2,
 	SizeLimit:  1,
 	Filter:     filterSchoolSearchByIdExisting,
-	Attributes: []string{"ou", "owncloudUUID", "ocEducationSchoolNumber"},
+	Attributes: []string{"ou", "owncloudUUID", "ocEducationSchoolNumber", "ocEducationSchoolTerminationTimestamp"},
 	Controls:   []ldap.Control(nil),
 }
 
@@ -315,7 +391,7 @@ var schoolByNumberSearch *ldap.SearchRequest = &ldap.SearchRequest{
 	Scope:      2,
 	SizeLimit:  1,
 	Filter:     filterSchoolSearchByNumberExisting,
-	Attributes: []string{"ou", "owncloudUUID", "ocEducationSchoolNumber"},
+	Attributes: []string{"ou", "owncloudUUID", "ocEducationSchoolNumber", "ocEducationSchoolTerminationTimestamp"},
 	Controls:   []ldap.Control(nil),
 }
 

services/graph/pkg/service/v0/educationschools.go

@@ -7,6 +7,7 @@ import (
 	"net/url"
 	"sort"
 	"strings"
+	"time"
 
 	"github.com/CiscoM31/godata"
 	libregraph "github.com/owncloud/libre-graph-api-go"
@@ -193,6 +194,26 @@ func (g Graph) DeleteEducationSchool(w http.ResponseWriter, r *http.Request) {
 		return
 	}
 
+	// Read school and check if termination date is set
+	school, err := g.identityEducationBackend.GetEducationSchool(r.Context(), schoolID)
+	if err != nil {
+		logger.Debug().Err(err).Msg("could not get school: backend error")
+		errorcode.RenderError(w, r, err)
+		return
+	}
+	termination, ok := school.GetTerminationDateOk()
+	if !ok {
+		logger.Debug().Msg("cannot delete school: not termination date set")
+		errorcode.NotAllowed.Render(w, r, http.StatusMethodNotAllowed, "no termination date set")
+		return
+	}
+
+	if time.Now().Before(*termination) {
+		logger.Debug().Time("terminationDate", *termination).Msg("cannot delete school: termination date not reached")
+		errorcode.NotAllowed.Render(w, r, http.StatusMethodNotAllowed, "can't delete school before termination date")
+		return
+	}
+
 	logger.Debug().Str("schoolID", schoolID).Msg("Getting users of school")
 	users, err := g.identityEducationBackend.GetEducationSchoolUsers(r.Context(), schoolID)
 	if err != nil {

services/graph/pkg/service/v0/educationschools_test.go

@@ -8,6 +8,7 @@ import (
 	"io"
 	"net/http"
 	"net/http/httptest"
+	"time"
 
 	gateway "github.com/cs3org/go-cs3apis/cs3/gateway/v1beta1"
 	userv1beta1 "github.com/cs3org/go-cs3apis/cs3/identity/user/v1beta1"
@@ -342,45 +343,64 @@ var _ = Describe("Schools", func() {
 	})
 
 	Describe("DeleteEducationSchool", func() {
+		schoolWithFutureTermination := libregraph.NewEducationSchool()
+		schoolWithFutureTermination.SetId("schoolWithFutureTermination")
+		schoolWithFutureTermination.SetTerminationDate(time.Now().Add(time.Hour * 24))
+
+		schoolWithPastTermination := libregraph.NewEducationSchool()
+		schoolWithPastTermination.SetId("schoolWithPastTermination")
+		schoolWithPastTermination.SetTerminationDate(time.Now().Add(-time.Hour * 24))
+
 		Context("with an existing school", func() {
 			BeforeEach(func() {
-				identityEducationBackend.On("GetEducationSchool", mock.Anything, mock.Anything, mock.Anything).Return(newSchool, nil)
+				identityEducationBackend.On("GetEducationSchool", mock.Anything, "school1").Return(newSchool, nil)
+				identityEducationBackend.On("GetEducationSchool", mock.Anything, "schoolWithFutureTermination", mock.Anything).Return(schoolWithFutureTermination, nil)
+				identityEducationBackend.On("GetEducationSchool", mock.Anything, "schoolWithPastTermination", mock.Anything).Return(schoolWithPastTermination, nil)
 			})
-		})
 
-		It("deletes the school", func() {
-			identityEducationBackend.On("DeleteEducationSchool", mock.Anything, mock.Anything, mock.Anything).Return(nil)
-			identityEducationBackend.On("GetEducationSchoolUsers", mock.Anything, mock.Anything, mock.Anything).Return([]*libregraph.EducationUser{}, nil)
-			r := httptest.NewRequest(http.MethodPatch, "/graph/v1.0/education/schools", nil)
-			rctx := chi.NewRouteContext()
-			rctx.URLParams.Add("schoolID", *newSchool.Id)
-			r = r.WithContext(context.WithValue(ctxpkg.ContextSetUser(ctx, currentUser), chi.RouteCtxKey, rctx))
-			svc.DeleteEducationSchool(rr, r)
+			DescribeTable("checks terminnation date",
+				func(schoolId string, statusCode int) {
+					identityEducationBackend.On("DeleteEducationSchool", mock.Anything, mock.Anything, mock.Anything).Return(nil)
+					identityEducationBackend.On("GetEducationSchoolUsers", mock.Anything, mock.Anything, mock.Anything).Return([]*libregraph.EducationUser{}, nil)
+					r := httptest.NewRequest(http.MethodDelete, "/graph/v1.0/education/schools", nil)
+					rctx := chi.NewRouteContext()
+					rctx.URLParams.Add("schoolID", schoolId)
+					r = r.WithContext(context.WithValue(ctxpkg.ContextSetUser(ctx, currentUser), chi.RouteCtxKey, rctx))
+					svc.DeleteEducationSchool(rr, r)
 
-			Expect(rr.Code).To(Equal(http.StatusNoContent))
-			identityEducationBackend.AssertNumberOfCalls(GinkgoT(), "DeleteEducationSchool", 1)
-		})
+					Expect(rr.Code).To(Equal(statusCode))
+					if rr.Code == http.StatusNoContent {
+						identityEducationBackend.AssertNumberOfCalls(GinkgoT(), "DeleteEducationSchool", 1)
+					} else {
+						identityEducationBackend.AssertNumberOfCalls(GinkgoT(), "DeleteEducationSchool", 0)
+					}
+				},
+				Entry("fails when school has no termination date", "school1", http.StatusMethodNotAllowed),
+				Entry("fails when school has a termination date in the future", "schoolWithFutureTermination", http.StatusMethodNotAllowed),
+				Entry("succeeds when school has a termination date in the past", "schoolWithPastTermination", http.StatusNoContent),
+			)
 
-		It("removes the users from the school", func() {
-			user1 := libregraph.NewEducationUser()
-			user1.SetId("user1")
-			user2 := libregraph.NewEducationUser()
-			user2.SetId("user2")
-			identityEducationBackend.On("GetEducationSchoolUsers", mock.Anything, mock.Anything, mock.Anything).Return([]*libregraph.EducationUser{user1, user2}, nil)
-			identityEducationBackend.On("DeleteEducationSchool", mock.Anything, mock.Anything, mock.Anything).Return(nil)
-			identityEducationBackend.On("RemoveUserFromEducationSchool", mock.Anything, mock.Anything, *user1.Id).Return(nil)
-			identityEducationBackend.On("RemoveUserFromEducationSchool", mock.Anything, mock.Anything, *user2.Id).Return(nil)
+			It("removes the users from the school", func() {
+				user1 := libregraph.NewEducationUser()
+				user1.SetId("user1")
+				user2 := libregraph.NewEducationUser()
+				user2.SetId("user2")
+				identityEducationBackend.On("GetEducationSchoolUsers", mock.Anything, mock.Anything, mock.Anything).Return([]*libregraph.EducationUser{user1, user2}, nil)
+				identityEducationBackend.On("DeleteEducationSchool", mock.Anything, mock.Anything, mock.Anything).Return(nil)
+				identityEducationBackend.On("RemoveUserFromEducationSchool", mock.Anything, mock.Anything, *user1.Id).Return(nil)
+				identityEducationBackend.On("RemoveUserFromEducationSchool", mock.Anything, mock.Anything, *user2.Id).Return(nil)
 
-			r := httptest.NewRequest(http.MethodPatch, "/graph/v1.0/education/schools", nil)
-			rctx := chi.NewRouteContext()
-			rctx.URLParams.Add("schoolID", *newSchool.Id)
-			r = r.WithContext(context.WithValue(ctxpkg.ContextSetUser(ctx, currentUser), chi.RouteCtxKey, rctx))
-			svc.DeleteEducationSchool(rr, r)
+				r := httptest.NewRequest(http.MethodDelete, "/graph/v1.0/education/schools", nil)
+				rctx := chi.NewRouteContext()
+				rctx.URLParams.Add("schoolID", "schoolWithPastTermination")
+				r = r.WithContext(context.WithValue(ctxpkg.ContextSetUser(ctx, currentUser), chi.RouteCtxKey, rctx))
+				svc.DeleteEducationSchool(rr, r)
 
-			Expect(rr.Code).To(Equal(http.StatusNoContent))
-			identityEducationBackend.AssertNumberOfCalls(GinkgoT(), "DeleteEducationSchool", 1)
-			identityEducationBackend.AssertNumberOfCalls(GinkgoT(), "RemoveUserFromEducationSchool", 2)
-			identityEducationBackend.AssertNumberOfCalls(GinkgoT(), "GetEducationSchoolUsers", 1)
+				Expect(rr.Code).To(Equal(http.StatusNoContent))
+				identityEducationBackend.AssertNumberOfCalls(GinkgoT(), "DeleteEducationSchool", 1)
+				identityEducationBackend.AssertNumberOfCalls(GinkgoT(), "RemoveUserFromEducationSchool", 2)
+				identityEducationBackend.AssertNumberOfCalls(GinkgoT(), "GetEducationSchoolUsers", 1)
+			})
 		})
 	})