Disassociate users from schools on school delete. (#5343)
* Disassociate users from schools on school delete. This PR alters the `DeleteEducationSchool` to also disassociate the users that were associated with the to-be-deleted school. * Add changelog. * Remove punctuation from changelog. * Remove redundant return statement. * Skip when user not find.
This commit is contained in:
@@ -11,6 +11,7 @@ import (
|
||||
|
||||
"github.com/CiscoM31/godata"
|
||||
libregraph "github.com/owncloud/libre-graph-api-go"
|
||||
"github.com/owncloud/ocis/v2/services/graph/pkg/identity"
|
||||
"github.com/owncloud/ocis/v2/services/graph/pkg/service/v0/errorcode"
|
||||
|
||||
"github.com/go-chi/chi/v5"
|
||||
@@ -195,17 +196,32 @@ func (g Graph) DeleteEducationSchool(w http.ResponseWriter, r *http.Request) {
|
||||
return
|
||||
}
|
||||
|
||||
logger.Debug().Str("schoolID", schoolID).Msg("Getting users of school")
|
||||
users, err := g.identityEducationBackend.GetEducationSchoolUsers(r.Context(), schoolID)
|
||||
if err != nil {
|
||||
logger.Debug().Err(err).Msg("could not get school users: backend error")
|
||||
renderInternalServerError(w, r, err)
|
||||
return
|
||||
}
|
||||
|
||||
for _, user := range users {
|
||||
logger.Debug().Str("schoolID", schoolID).Str("userID", *user.Id).Msg("calling delete member on backend")
|
||||
if err := g.identityEducationBackend.RemoveUserFromEducationSchool(r.Context(), schoolID, *user.Id); err != nil {
|
||||
if errors.Is(err, identity.ErrNotFound) {
|
||||
logger.Debug().Str("schoolID", schoolID).Str("userID", *user.Id).Msg("user not found")
|
||||
continue
|
||||
}
|
||||
logger.Debug().Err(err).Msg("could not delete school member: backend error")
|
||||
renderInternalServerError(w, r, err)
|
||||
}
|
||||
}
|
||||
|
||||
logger.Debug().Str("id", schoolID).Msg("calling delete school on backend")
|
||||
err = g.identityEducationBackend.DeleteEducationSchool(r.Context(), schoolID)
|
||||
|
||||
if err != nil {
|
||||
logger.Debug().Err(err).Msg("could not delete school: backend error")
|
||||
var errcode errorcode.Error
|
||||
if errors.As(err, &errcode) {
|
||||
errcode.Render(w, r)
|
||||
} else {
|
||||
errorcode.GeneralException.Render(w, r, http.StatusInternalServerError, err.Error())
|
||||
}
|
||||
renderInternalServerError(w, r, err)
|
||||
return
|
||||
}
|
||||
|
||||
@@ -221,6 +237,15 @@ func (g Graph) DeleteEducationSchool(w http.ResponseWriter, r *http.Request) {
|
||||
render.NoContent(w, r)
|
||||
}
|
||||
|
||||
func renderInternalServerError(w http.ResponseWriter, r *http.Request, err error) {
|
||||
var errcode errorcode.Error
|
||||
if errors.As(err, &errcode) {
|
||||
errcode.Render(w, r)
|
||||
} else {
|
||||
errorcode.GeneralException.Render(w, r, http.StatusInternalServerError, err.Error())
|
||||
}
|
||||
}
|
||||
|
||||
// GetEducationSchoolUsers implements the Service interface.
|
||||
func (g Graph) GetEducationSchoolUsers(w http.ResponseWriter, r *http.Request) {
|
||||
logger := g.logger.SubloggerWithRequestID(r.Context())
|
||||
|
||||
@@ -50,7 +50,6 @@ var _ = Describe("Schools", func() {
|
||||
)
|
||||
|
||||
BeforeEach(func() {
|
||||
|
||||
identityEducationBackend = &identitymocks.EducationBackend{}
|
||||
gatewayClient = &mocks.GatewayClient{}
|
||||
newSchool = libregraph.NewEducationSchool()
|
||||
@@ -336,6 +335,7 @@ var _ = Describe("Schools", func() {
|
||||
|
||||
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)
|
||||
@@ -345,6 +345,28 @@ var _ = Describe("Schools", func() {
|
||||
Expect(rr.Code).To(Equal(http.StatusNoContent))
|
||||
identityEducationBackend.AssertNumberOfCalls(GinkgoT(), "DeleteEducationSchool", 1)
|
||||
})
|
||||
|
||||
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)
|
||||
|
||||
Expect(rr.Code).To(Equal(http.StatusNoContent))
|
||||
identityEducationBackend.AssertNumberOfCalls(GinkgoT(), "DeleteEducationSchool", 1)
|
||||
identityEducationBackend.AssertNumberOfCalls(GinkgoT(), "RemoveUserFromEducationSchool", 2)
|
||||
identityEducationBackend.AssertNumberOfCalls(GinkgoT(), "GetEducationSchoolUsers", 1)
|
||||
})
|
||||
})
|
||||
|
||||
Describe("GetEducationSchoolUsers", func() {
|
||||
|
||||
Reference in New Issue
Block a user