Repository: owncloud/ocis (go)

Issue:
libregraph: PATCH/GET/DELETE with school `id` or `schoolNumber`
We want to treat `schoolNumber` as the unique property, similar to the `userPrincipalName`. Then all request types should allow using the `schoolNumber` or the school `id`: `/education/schools/{id | schoolNumber}`

Pull request description:
Adding so education PATCH updates school name or number.
## Description

This adds the missing PATCH functionality for updating a school using a school name or number.

As we can't have an atomic operation when updating the OU and another property, this only allows mutually exclusive updates of school number or name.

Note: The ldap update functions themselves do not have tests, but rather the "glue" function to select which operation to perform in UpdateEducationUser.

## Related Issue
- Fixes #5245 

## Motivation and Context
This adds the missing update implementation.

## How Has This Been Tested?
I tested this locally with curl and added testcases for the different input data scenarios.

## Types of changes
<!--- What types of changes does your code introduce? Put an `x` in all the boxes that apply: -->
- [ ] Bug fix (non-breaking change which fixes an issue)
- [x] New feature (non-breaking change which adds functionality)
- [ ] Breaking change (fix or feature that would cause existing functionality to change)
- [ ] Technical debt
- [ ] Tests only (no source changes)

## Checklist:
<!-- Tick the checkboxes when done. -->
<!-- Raise documentation ticket in owncloud.github.io/ -->
- [x] Code changes
- [x] Unit tests added
- [ ] Acceptance tests added
- [ ] Documentation ticket raised: <link> 


Patch:
diff --git a/CHANGELOG.md b/CHANGELOG.md
index ff55427aa0a..413de620928 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -13,6 +13,7 @@ The following sections list the changes for unreleased.
 * Enhancement - Async Postprocessing: [#5207](https://github.com/owncloud/ocis/pull/5207)
 * Enhancement - Bump libre-graph-api-go: [#5309](https://github.com/owncloud/ocis/pull/5309)
 * Enhancement - Bump reva version: [#5243](https://github.com/owncloud/ocis/pull/5243)
+* Enhancement - Collect global envvars: [#5367](https://github.com/owncloud/ocis/pull/5367)
 * Enhancement - Drive group permissions: [#5312](https://github.com/owncloud/ocis/pull/5312)
 * Enhancement - Make the group members addition limit configurable: [#5357](https://github.com/owncloud/ocis/pull/5357)
 * Enhancement - Graph Drives IdentitySet displayName: [#5347](https://github.com/owncloud/ocis/pull/5347)
diff --git a/changelog/unreleased/collect-global-envvars.md b/changelog/unreleased/collect-global-envvars.md
new file mode 100644
index 00000000000..7dd08f6889a
--- /dev/null
+++ b/changelog/unreleased/collect-global-envvars.md
@@ -0,0 +1,5 @@
+Enhancement: Collect global envvars
+
+Compose a list of all envvars living in more than 1 service
+
+https://github.com/owncloud/ocis/pull/5367
diff --git a/docs/helpers/adoc-generator.go.tmpl b/docs/helpers/adoc-generator.go.tmpl
index 0ed23f86e53..977363ad152 100644
--- a/docs/helpers/adoc-generator.go.tmpl
+++ b/docs/helpers/adoc-generator.go.tmpl
@@ -9,11 +9,14 @@ import (
 	"regexp"
 	"strings"
 	"text/template"
+	"sort"
 
     {{- range $key, $value := .}}
     pkg{{$key}} "{{$value}}"
-    {{- end}})
+    {{- end}}
+)
 
+// ConfigField is the representation of one configuration field
 type ConfigField struct {
 	EnvVars         []string
 	DefaultValue    string
@@ -23,6 +26,7 @@ type ConfigField struct {
 	DeprecationLink string
 }
 
+// DeprecationField holds information about deprecation
 type DeprecationField struct {
 	DeprecationVersion     string
 	DeprecationInfo        string
@@ -30,6 +34,15 @@ type DeprecationField struct {
 	RemovalVersion         string
 }
 
+// EnvVar holds information about one envvar
+type EnvVar struct {
+    Name 	 string
+    DefaultValue string
+    Type 	 string
+    Description  string
+    Services 	 []string
+}
+
 type templateData struct {
 	ExtensionName   string
 	Fields          []ConfigField
@@ -38,44 +51,66 @@ type templateData struct {
 }
 
 func main() {
-fmt.Println("Generating adoc documentation for environment variables:")
-content, err := os.ReadFile("../../docs/templates/ADOC.tmpl")
-if err != nil {
-    log.Fatal(err)
-}
-replacer := strings.NewReplacer(
-		"github.com/owncloud/ocis/v2/services/", "",
-		"/pkg/config/defaults", "",
-	)
-var fields []ConfigField
-var deprecations []DeprecationField
-var targetFile *os.File
-tpl := template.Must(template.New("").Parse(string(content)))
-
-m := map[string]interface{}{
-{{- range $key, $value := .}}
-	"{{$value}}": *pkg{{$key}}.FullDefaultConfig(),
-{{- end }}
-}
+	fmt.Println("Generating adoc documentation for environment variables:")
+	content, err := os.ReadFile("../../docs/templates/ADOC.tmpl")
+	if err != nil {
+	    log.Fatal(err)
+	}
+	replacer := strings.NewReplacer(
+			"github.com/owncloud/ocis/v2/services/", "",
+			"/pkg/config/defaults", "",
+		)
+	var fields []ConfigField
+	var deprecations []DeprecationField
+	var targetFile *os.File
+	tpl := template.Must(template.New("").Parse(string(content)))
 
-    targetFolder := "../../docs/services/_includes/adoc/"
-    for pkg, conf := range m {
-	fields, deprecations = GetAnnotatedVariables(conf)
-	var hasDeprecations bool
-	if len(deprecations) > 0 {
-		hasDeprecations = true
+	m := map[string]interface{}{
+	{{- range $key, $value := .}}
+		"{{$value}}": *pkg{{$key}}.FullDefaultConfig(),
+	{{- end }}
 	}
 
-	if len(fields) > 0 || len(deprecations) > 0 {
-	fmt.Printf("... %s\n", pkg)
-		targetFile, err = os.Create(filepath.Join(targetFolder, replacer.Replace(pkg) + "_configvars.adoc"))
+	targetFolder := "../../docs/services/_includes/adoc/"
+	all := make(map[string]EnvVar)
+	for pkg, conf := range m {
+	    service := replacer.Replace(pkg)
+	    fields, deprecations = GetAnnotatedVariables(conf)
+	    var hasDeprecations bool
+	    if len(deprecations) > 0 {
+		    hasDeprecations = true
+	    }
+
+	    for _, f := range fields {
+		for _, e := range f.EnvVars {
+		    if env, ok := all[e]; ok {
+			env.Services = append(env.Services, service)
+			sort.Slice(env.Services, func(i, j int) bool {
+			    return env.Services[i] < env.Services[j]
+			})
+			all[e] = env
+		    } else {
+			all[e] = EnvVar{
+			    Name:         e,
+			    Description:  f.Description,
+			    Type: 	  f.Type,
+			    DefaultValue: f.DefaultValue,
+			    Services:     []string{service},
+			}
+		    }
+		}
+	    }
+
+	    if len(fields) > 0 || len(deprecations) > 0 {
+		fmt.Printf("... %s\n", pkg)
+		targetFile, err = os.Create(filepath.Join(targetFolder, service + "_configvars.adoc"))
 		if err != nil {
 			log.Fatalf("Failed to create target file: %s", err)
 		}
 		defer targetFile.Close()
 
 		td := templateData{
-			ExtensionName: replacer.Replace(pkg),
+			ExtensionName: service,
 			Fields:        fields,
 			Deprecations: deprecations,
 			HasDeprecations: hasDeprecations ,
@@ -83,8 +118,43 @@ m := map[string]interface{}{
 		if err := tpl.Execute(targetFile, td); err != nil {
 			log.Fatalf("Failed to execute template: %s", err)
 		}
+	    }
 	}
-    }
+
+	// render global env vars
+	tmplValues := make([]map[string]interface{}, 0)
+	for _, env := range all {
+	    if len(env.Services) > 1 {
+		tmplValues = append(tmplValues, map[string]interface{}{
+			"Name":         env.Name,
+			"Services":     env.Services,
+			"Description":  env.Description,
+			"DefaultValue": env.DefaultValue,
+			"Type":         env.Type,
+		})	    
+	    }
+	}
+
+	// sort
+	sort.Slice(tmplValues, func(i, j int) bool {
+		return tmplValues[i]["Name"].(string) < tmplValues[j]["Name"].(string)
+	})
+
+	glc, err := os.ReadFile("../../docs/templates/ADOC_global.tmpl")
+	if err != nil {
+	    log.Fatal(err)
+	}
+
+	gltpl := template.Must(template.New("").Parse(string(glc)))
+	glfile, err := os.Create(filepath.Join(targetFolder, "global_configvars.adoc"))
+	if err != nil {
+		log.Fatalf("Failed to create target file: %s", err)
+	}
+
+	if err := gltpl.Execute(glfile, tmplValues); err != nil {
+	    log.Printf("Failed to execute template: %s", err)
+	}
+
 	fmt.Println("done")
 }
 
diff --git a/docs/templates/ADOC_global.tmpl b/docs/templates/ADOC_global.tmpl
new file mode 100644
index 00000000000..cbc43b850cd
--- /dev/null
+++ b/docs/templates/ADOC_global.tmpl
@@ -0,0 +1,33 @@
+// collected through docs/helpers/adoc-generator.go.tmpl
+
+[.landscape]
+[caption=]
+.Environment variables with global scope available in multiple services
+[width="100%",cols="30%,25%,~,~,~",options="header"]
+|===
+| Name
+| Services
+| Type
+| Default Value
+| Description
+
+{{ range . }}
+
+a| `{{ .Name }}`
+
+a| [subs=attributes+]
+{{- range .Services}}
+* xref:{s-path}/{{ . }}.adoc[{{ . }}] +
+{{- end }}
+
+a| [subs=-attributes]
+++{{ .Type }} ++
+
+a| [subs=-attributes]
+++{{ .DefaultValue }} ++
+
+a| [subs=-attributes]
+{{ .Description }}
+
+{{- end }}
+|===
diff --git a/services/graph/pkg/identity/backend.go b/services/graph/pkg/identity/backend.go
index cefa4cb2fbe..b521e32d45d 100644
--- a/services/graph/pkg/identity/backend.go
+++ b/services/graph/pkg/identity/backend.go
@@ -51,6 +51,8 @@ type EducationBackend interface {
 	GetEducationSchool(ctx context.Context, nameOrID string, queryParam url.Values) (*libregraph.EducationSchool, error)
 	// GetEducationSchools lists all schools
 	GetEducationSchools(ctx context.Context, queryParam url.Values) ([]*libregraph.EducationSchool, error)
+	// UpdateEducationSchool updates attributes of a school
+	UpdateEducationSchool(ctx context.Context, numberOrID string, school libregraph.EducationSchool) (*libregraph.EducationSchool, error)
 	// GetEducationSchoolUsers lists all members of a school
 	GetEducationSchoolUsers(ctx context.Context, id string) ([]*libregraph.EducationUser, error)
 	// AddUsersToEducationSchool adds new members (reference by a slice of IDs) to supplied school in the identity backend.
diff --git a/services/graph/pkg/identity/err_school.go b/services/graph/pkg/identity/err_school.go
index cbe42c70da6..6460fa46dfa 100644
--- a/services/graph/pkg/identity/err_school.go
+++ b/services/graph/pkg/identity/err_school.go
@@ -30,6 +30,11 @@ func (i *ErrEducationBackend) GetEducationSchools(ctx context.Context, queryPara
 	return nil, errNotImplemented
 }
 
+// UpdateEducationSchool implements the EducationBackend interface for the ErrEducationBackend backend.
+func (i *ErrEducationBackend) UpdateEducationSchool(ctx context.Context, numberOrID string, school libregraph.EducationSchool) (*libregraph.EducationSchool, error) {
+	return nil, errNotImplemented
+}
+
 // GetEducationSchoolUsers implements the EducationBackend interface for the ErrEducationBackend backend.
 func (i *ErrEducationBackend) GetEducationSchoolUsers(ctx context.Context, id string) ([]*libregraph.EducationUser, error) {
 	return nil, errNotImplemented
diff --git a/services/graph/pkg/identity/ldap_school.go b/services/graph/pkg/identity/ldap_school.go
index d73939a5c8a..a50f543fb67 100644
--- a/services/graph/pkg/identity/ldap_school.go
+++ b/services/graph/pkg/identity/ldap_school.go
@@ -34,6 +34,15 @@ type schoolAttributeMap struct {
 	id           string
 }
 
+type schoolUpdateOperation uint8
+
+const (
+	tooManyValues schoolUpdateOperation = iota
+	schoolUnchanged
+	displayNameUpdated
+	schoolNumberUpdated
+)
+
 func defaultEducationConfig() educationConfig {
 	return educationConfig{
 		schoolObjectClass:       "ocEducationSchool",
@@ -95,6 +104,8 @@ func (i *LDAP) CreateEducationSchool(ctx context.Context, school libregraph.Educ
 		return nil, ErrReadOnly
 	}
 
+	// Here we should verify that the school number is not already used
+
 	dn := fmt.Sprintf("%s=%s,%s",
 		i.educationConfig.schoolAttributeMap.displayName,
 		oldap.EscapeDNAttributeValue(school.GetDisplayName()),
@@ -128,6 +139,130 @@ func (i *LDAP) CreateEducationSchool(ctx context.Context, school libregraph.Educ
 	return i.createSchoolModelFromLDAP(e), nil
 }
 
+// UpdateEducationSchoolOperation contains the logic for which update operation to apply to a school
+func (i *LDAP) UpdateEducationSchoolOperation(
+	schoolUpdate libregraph.EducationSchool,
+	currentSchool libregraph.EducationSchool,
+) schoolUpdateOperation {
+	providedDisplayName := schoolUpdate.GetDisplayName()
+	schoolNumber := schoolUpdate.GetSchoolNumber()
+
+	if providedDisplayName != "" && schoolNumber != "" {
+		return tooManyValues
+	}
+
+	if providedDisplayName != "" && providedDisplayName != currentSchool.GetDisplayName() {
+		return displayNameUpdated
+	}
+
+	if schoolNumber != "" && schoolNumber != currentSchool.GetSchoolNumber() {
+		return schoolNumberUpdated
+	}
+
+	return schoolUnchanged
+}
+
+// updateDisplayName updates the school OU in the identity backend
+func (i *LDAP) updateDisplayName(ctx context.Context, dn string, providedDisplayName string) error {
+	logger := i.logger.SubloggerWithRequestID(ctx)
+	newDisplayName := fmt.Sprintf(
+		"%s=%s",
+		i.educationConfig.schoolAttributeMap.displayName,
+		providedDisplayName,
+	)
+
+	mrdn := ldap.NewModifyDNRequest(dn, newDisplayName, true, "")
+	i.logger.Debug().Str("backend", "ldap").
+		Str("dn", mrdn.DN).
+		Str("newrdn", mrdn.NewRDN).
+		Msg("updateDisplayName")
+
+	if err := i.conn.ModifyDN(mrdn); err != nil {
+		var lerr *ldap.Error
+		logger.Debug().Err(err).Msg("error updating school name")
+		if errors.As(err, &lerr) {
+			if lerr.ResultCode == ldap.LDAPResultEntryAlreadyExists {
+				err = errorcode.New(errorcode.NameAlreadyExists, lerr.Error())
+			}
+		}
+		return err
+	}
+
+	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 {
+	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 err := i.conn.Modify(mr); err != nil {
+		var lerr *ldap.Error
+		logger.Debug().Err(err).Msg("error updating school number")
+		if errors.As(err, &lerr) {
+			if lerr.ResultCode == ldap.LDAPResultEntryAlreadyExists {
+				err = errorcode.New(errorcode.NameAlreadyExists, lerr.Error())
+			}
+		}
+		return err
+	}
+
+	return nil
+}
+
+// UpdateEducationSchool updates the supplied school in the identity backend
+func (i *LDAP) UpdateEducationSchool(ctx context.Context, numberOrID string, school libregraph.EducationSchool) (*libregraph.EducationSchool, error) {
+	logger := i.logger.SubloggerWithRequestID(ctx)
+	logger.Debug().Str("backend", "ldap").Msg("UpdateEducationSchool")
+	if !i.writeEnabled {
+		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
+	}
+
+	currentSchool := i.createSchoolModelFromLDAP(e)
+	switch i.UpdateEducationSchoolOperation(school, *currentSchool) {
+	case tooManyValues:
+		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 nil, err
+		}
+	case schoolNumberUpdated:
+		if err := i.updateSchoolNumber(ctx, e.DN, schoolNumber); err != nil {
+			return nil, err
+		}
+	}
+
+	// Read	back school from LDAP
+	e, err = i.getSchoolByNumberOrID(i.getID(e))
+	if err != nil {
+		return nil, err
+	}
+	return i.createSchoolModelFromLDAP(e), nil
+}
+
 // DeleteEducationSchool deletes a given school, identified by id
 func (i *LDAP) DeleteEducationSchool(ctx context.Context, id string) error {
 	logger := i.logger.SubloggerWithRequestID(ctx)
@@ -408,9 +543,9 @@ func (i *LDAP) createSchoolModelFromLDAP(e *ldap.Entry) *libregraph.EducationSch
 		return nil
 	}
 
-	displayName := e.GetEqualFoldAttributeValue(i.educationConfig.schoolAttributeMap.displayName)
-	id := e.GetEqualFoldAttributeValue(i.educationConfig.schoolAttributeMap.id)
-	schoolNumber := e.GetEqualFoldAttributeValue(i.educationConfig.schoolAttributeMap.schoolNumber)
+	displayName := i.getDisplayName(e)
+	id := i.getID(e)
+	schoolNumber := i.getSchoolNumber(e)
 
 	if id != "" && displayName != "" && schoolNumber != "" {
 		school := libregraph.NewEducationSchool()
@@ -422,3 +557,18 @@ func (i *LDAP) createSchoolModelFromLDAP(e *ldap.Entry) *libregraph.EducationSch
 	i.logger.Warn().Str("dn", e.DN).Str("id", id).Str("displayName", displayName).Str("schoolNumber", schoolNumber).Msg("Invalid School. Missing required attribute")
 	return nil
 }
+
+func (i *LDAP) getSchoolNumber(e *ldap.Entry) string {
+	schoolNumber := e.GetEqualFoldAttributeValue(i.educationConfig.schoolAttributeMap.schoolNumber)
+	return schoolNumber
+}
+
+func (i *LDAP) getID(e *ldap.Entry) string {
+	id := e.GetEqualFoldAttributeValue(i.educationConfig.schoolAttributeMap.id)
+	return id
+}
+
+func (i *LDAP) getDisplayName(e *ldap.Entry) string {
+	displayName := e.GetEqualFoldAttributeValue(i.educationConfig.schoolAttributeMap.displayName)
+	return displayName
+}
diff --git a/services/graph/pkg/identity/mocks/education_backend.go b/services/graph/pkg/identity/mocks/education_backend.go
index 031510d3aec..772299ba9ef 100644
--- a/services/graph/pkg/identity/mocks/education_backend.go
+++ b/services/graph/pkg/identity/mocks/education_backend.go
@@ -234,6 +234,29 @@ func (_m *EducationBackend) RemoveUserFromEducationSchool(ctx context.Context, s
 	return r0
 }
 
+// UpdateEducationSchool provides a mock function with given fields: ctx, numberOrID, school
+func (_m *EducationBackend) UpdateEducationSchool(ctx context.Context, numberOrID string, school libregraph.EducationSchool) (*libregraph.EducationSchool, error) {
+	ret := _m.Called(ctx, numberOrID, school)
+
+	var r0 *libregraph.EducationSchool
+	if rf, ok := ret.Get(0).(func(context.Context, string, libregraph.EducationSchool) *libregraph.EducationSchool); ok {
+		r0 = rf(ctx, numberOrID, school)
+	} else {
+		if ret.Get(0) != nil {
+			r0 = ret.Get(0).(*libregraph.EducationSchool)
+		}
+	}
+
+	var r1 error
+	if rf, ok := ret.Get(1).(func(context.Context, string, libregraph.EducationSchool) error); ok {
+		r1 = rf(ctx, numberOrID, school)
+	} else {
+		r1 = ret.Error(1)
+	}
+
+	return r0, r1
+}
+
 // UpdateEducationUser provides a mock function with given fields: ctx, nameOrID, user
 func (_m *EducationBackend) UpdateEducationUser(ctx context.Context, nameOrID string, user libregraph.EducationUser) (*libregraph.EducationUser, error) {
 	ret := _m.Called(ctx, nameOrID, user)
diff --git a/services/graph/pkg/service/v0/educationschools.go b/services/graph/pkg/service/v0/educationschools.go
index c6b8b8c2063..8ac3df947e9 100644
--- a/services/graph/pkg/service/v0/educationschools.go
+++ b/services/graph/pkg/service/v0/educationschools.go
@@ -112,24 +112,31 @@ func (g Graph) PatchEducationSchool(w http.ResponseWriter, r *http.Request) {
 	schoolID := chi.URLParam(r, "schoolID")
 	schoolID, err := url.PathUnescape(schoolID)
 	if err != nil {
-		logger.Debug().Str("id", schoolID).Msg("could not change school: unescaping school id failed")
+		logger.Debug().Str("id", schoolID).Msg("could not update school: unescaping school id failed")
 		errorcode.InvalidRequest.Render(w, r, http.StatusBadRequest, "unescaping school id failed")
 		return
 	}
 
 	if schoolID == "" {
-		logger.Debug().Msg("could not change school: missing school id")
+		logger.Debug().Msg("could not update school: missing school id")
 		errorcode.InvalidRequest.Render(w, r, http.StatusBadRequest, "missing school id")
 		return
 	}
-	changes := libregraph.NewEducationSchool()
-	err = json.NewDecoder(r.Body).Decode(changes)
+
+	school := libregraph.NewEducationSchool()
+	err = json.NewDecoder(r.Body).Decode(school)
 	if err != nil {
-		logger.Debug().Err(err).Interface("body", r.Body).Msg("could not change school: invalid request body")
+		logger.Debug().Err(err).Interface("body", r.Body).Msg("could not update school: invalid request body")
 		errorcode.InvalidRequest.Render(w, r, http.StatusBadRequest, fmt.Sprintf("invalid request body: %s", err.Error()))
 		return
 	}
 
+	if school, err = g.identityEducationBackend.UpdateEducationSchool(r.Context(), schoolID, *school); err != nil {
+		logger.Debug().Err(err).Interface("school", school).Msg("could not update school: backend error")
+		errorcode.GeneralException.Render(w, r, http.StatusInternalServerError, err.Error())
+		return
+	}
+
 	/* TODO requires reva changes
 	e := events.SchoolFeatureChanged{SchoolID: schoolID}
 	if currentUser, ok := ctxpkg.ContextGetUser(r.Context()); ok {
@@ -138,8 +145,8 @@ func (g Graph) PatchEducationSchool(w http.ResponseWriter, r *http.Request) {
 	g.publishEvent(e)
 	*/
 
-	render.Status(r, http.StatusNoContent)
-	render.NoContent(w, r)
+	render.Status(r, http.StatusOK)
+	render.JSON(w, r, school)
 }
 
 // GetEducationSchool implements the Service interface.


Test patch:
diff --git a/services/graph/pkg/identity/ldap_school_test.go b/services/graph/pkg/identity/ldap_school_test.go
index 953c92c1ee0..f27714755f9 100644
--- a/services/graph/pkg/identity/ldap_school_test.go
+++ b/services/graph/pkg/identity/ldap_school_test.go
@@ -46,6 +46,11 @@ var schoolEntry1 = ldap.NewEntry("ou=Test School1",
 		"owncloudUUID":            {"hijk-defg"},
 	})
 
+var filterSchoolSearchByIdExisting = "(&(objectClass=ocEducationSchool)(|(owncloudUUID=abcd-defg)(ocEducationSchoolNumber=abcd-defg)))"
+var filterSchoolSearchByIdNonexistant = "(&(objectClass=ocEducationSchool)(|(owncloudUUID=xxxx-xxxx)(ocEducationSchoolNumber=xxxx-xxxx)))"
+var filterSchoolSearchByNumberExisting = "(&(objectClass=ocEducationSchool)(|(owncloudUUID=0123)(ocEducationSchoolNumber=0123)))"
+var filterSchoolSearchByNumberNonexistant = "(&(objectClass=ocEducationSchool)(|(owncloudUUID=3210)(ocEducationSchoolNumber=3210)))"
+
 func TestCreateEducationSchool(t *testing.T) {
 	lm := &mocks.Client{}
 	lm.On("Add", mock.Anything).
@@ -75,6 +80,66 @@ func TestCreateEducationSchool(t *testing.T) {
 	assert.Equal(t, res_school.GetSchoolNumber(), school.GetSchoolNumber())
 }
 
+func TestUpdateEducationSchoolOperation(t *testing.T) {
+	testSchoolName := "A name"
+	testSchoolNumber := "1234"
+	tests := []struct {
+		name              string
+		displayName       string
+		schoolNumber      string
+		expectedOperation schoolUpdateOperation
+	}{
+		{
+			name:              "Test using school with both number and name",
+			displayName:       testSchoolName,
+			schoolNumber:      testSchoolNumber,
+			expectedOperation: tooManyValues,
+		},
+		{
+			name:              "Test with unchanged number",
+			schoolNumber:      testSchoolNumber,
+			expectedOperation: schoolUnchanged,
+		},
+		{
+			name:              "Test with unchanged name",
+			displayName:       testSchoolName,
+			expectedOperation: schoolUnchanged,
+		},
+		{
+			name:              "Test new name",
+			displayName:       "Something new",
+			expectedOperation: displayNameUpdated,
+		},
+		{
+			name:              "Test new number",
+			schoolNumber:      "9876",
+			expectedOperation: schoolNumberUpdated,
+		},
+	}
+
+	for _, tt := range tests {
+		lm := &mocks.Client{}
+		b, err := getMockedBackend(lm, eduConfig, &logger)
+		assert.Nil(t, err)
+
+		displayName := "A name"
+		schoolNumber := "1234"
+
+		currentSchool := libregraph.EducationSchool{
+			DisplayName:  &displayName,
+			SchoolNumber: &schoolNumber,
+		}
+
+		schoolUpdate := libregraph.EducationSchool{
+			DisplayName:  &tt.displayName,
+			SchoolNumber: &tt.schoolNumber,
+		}
+
+		operation := b.UpdateEducationSchoolOperation(schoolUpdate, currentSchool)
+		assert.Equal(t, tt.expectedOperation, operation)
+	}
+}
+
 func TestDeleteEducationSchool(t *testing.T) {
 	tests := []struct {
 		name                 string
@@ -85,25 +150,25 @@ func TestDeleteEducationSchool(t *testing.T) {
 		{
 			name:                 "Test delete school using schoolId",
 			numberOrId:           "abcd-defg",
-			filter:               "(&(objectClass=ocEducationSchool)(|(owncloudUUID=abcd-defg)(ocEducationSchoolNumber=abcd-defg)))",
+			filter:               filterSchoolSearchByIdExisting,
 			expectedItemNotFound: false,
 		},
 		{
 			name:                 "Test delete school using unknown schoolId",
 			numberOrId:           "xxxx-xxxx",
-			filter:               "(&(objectClass=ocEducationSchool)(|(owncloudUUID=xxxx-xxxx)(ocEducationSchoolNumber=xxxx-xxxx)))",
+			filter:               filterSchoolSearchByIdNonexistant,
 			expectedItemNotFound: true,
 		},
 		{
 			name:                 "Test delete school using schoolNumber",
 			numberOrId:           "0123",
-			filter:               "(&(objectClass=ocEducationSchool)(|(owncloudUUID=0123)(ocEducationSchoolNumber=0123)))",
+			filter:               filterSchoolSearchByNumberExisting,
 			expectedItemNotFound: false,
 		},
 		{
 			name:                 "Test delete school using unknown schoolNumber",
 			numberOrId:           "3210",
-			filter:               "(&(objectClass=ocEducationSchool)(|(owncloudUUID=3210)(ocEducationSchoolNumber=3210)))",
+			filter:               filterSchoolSearchByNumberNonexistant,
 			expectedItemNotFound: true,
 		},
 	}
@@ -154,25 +219,25 @@ func TestGetEducationSchool(t *testing.T) {
 		{
 			name:                 "Test search school using schoolId",
 			numberOrId:           "abcd-defg",
-			filter:               "(&(objectClass=ocEducationSchool)(|(owncloudUUID=abcd-defg)(ocEducationSchoolNumber=abcd-defg)))",
+			filter:               filterSchoolSearchByIdExisting,
 			expectedItemNotFound: false,
 		},
 		{
 			name:                 "Test search school using unknown schoolId",
 			numberOrId:           "xxxx-xxxx",
-			filter:               "(&(objectClass=ocEducationSchool)(|(owncloudUUID=xxxx-xxxx)(ocEducationSchoolNumber=xxxx-xxxx)))",
+			filter:               filterSchoolSearchByIdNonexistant,
 			expectedItemNotFound: true,
 		},
 		{
 			name:                 "Test search school using schoolNumber",
 			numberOrId:           "0123",
-			filter:               "(&(objectClass=ocEducationSchool)(|(owncloudUUID=0123)(ocEducationSchoolNumber=0123)))",
+			filter:               filterSchoolSearchByNumberExisting,
 			expectedItemNotFound: false,
 		},
 		{
 			name:                 "Test search school using unknown schoolNumber",
 			numberOrId:           "3210",
-			filter:               "(&(objectClass=ocEducationSchool)(|(owncloudUUID=3210)(ocEducationSchoolNumber=3210)))",
+			filter:               filterSchoolSearchByNumberNonexistant,
 			expectedItemNotFound: true,
 		},
 	}
@@ -234,7 +299,7 @@ var schoolByIDSearch1 *ldap.SearchRequest = &ldap.SearchRequest{
 	BaseDN:     "",
 	Scope:      2,
 	SizeLimit:  1,
-	Filter:     "(&(objectClass=ocEducationSchool)(|(owncloudUUID=abcd-defg)(ocEducationSchoolNumber=abcd-defg)))",
+	Filter:     filterSchoolSearchByIdExisting,
 	Attributes: []string{"ou", "owncloudUUID", "ocEducationSchoolNumber"},
 	Controls:   []ldap.Control(nil),
 }
diff --git a/services/graph/pkg/service/v0/educationschools_test.go b/services/graph/pkg/service/v0/educationschools_test.go
index 262f736863f..226aa627863 100644
--- a/services/graph/pkg/service/v0/educationschools_test.go
+++ b/services/graph/pkg/service/v0/educationschools_test.go
@@ -315,6 +315,8 @@ var _ = Describe("Schools", func() {
 			newSchoolJson, err := json.Marshal(newSchool)
 			Expect(err).ToNot(HaveOccurred())
 
+			identityEducationBackend.On("UpdateEducationSchool", mock.Anything, mock.Anything, mock.Anything).Return(newSchool, nil)
+
 			r := httptest.NewRequest(http.MethodPatch, "/graph/v1.0/education/schools/schoolid", bytes.NewBuffer(newSchoolJson))
 			rctx := chi.NewRouteContext()
 			rctx.URLParams.Add("schoolID", "school-id")
@@ -322,7 +324,7 @@ var _ = Describe("Schools", func() {
 
 			svc.PatchEducationSchool(rr, r)
 
-			Expect(rr.Code).To(Equal(http.StatusNoContent))
+			Expect(rr.Code).To(Equal(http.StatusOK))
 		})
 	})