graph:Add stubs for education/classes endpoints #5360
Conversation
rhafer
changed the title
|
Thanks for opening this pull request! The maintainers of this repository would appreciate it if you would create a changelog item based on your changes. |
rhafer
force-pushed
the
education-classes
branch
from
2ed4d04 to
420d4fa
Compare
|
💥 Acceptance test wopiValidatorTests-ocis failed. Further test are cancelled... |
|
💥 Acceptance test localApiTests-apiCors-ocis failed. Further test are cancelled... |
|
💥 Acceptance test Core-API-Tests-ocis-storage-5 failed. Further test are cancelled... |
rhafer
force-pushed
the
education-classes
branch
3 times, most recently
from
c84aaae to
a8105c9
Compare
| } | ||
|
|
||
| // PatchEducationClass implements the Service interface. | ||
| func (g Graph) PatchEducationClass(w http.ResponseWriter, r *http.Request) { |
There was a problem hiding this comment.
If I'm reading the API definition correctly, then this should only update the properties of the class object. Not adding members to it.
There was a problem hiding this comment.
Actually I identended it to support both (the update of single properties is still missing, I'd prefer to add that in a subsequent PR). Similar to what is possible for groups, PATCH can be used to add multiple group members at once. I guess we need to add an example to the Swagger spec to make that clearer.
There was a problem hiding this comment.
Added owncloud/libre-graph-api#84 for that
| identityEducationBackend.On("GetEducationClasses", ctx, mock.Anything).Return([]*libregraph.EducationClass{}, nil) | ||
|
|
||
| r := httptest.NewRequest(http.MethodGet, "/graph/v1.0/education/classe", nil) | ||
| svc.GetEducationClasses(rr, r) |
There was a problem hiding this comment.
General question: When running the service methods like this, we're not actually running through the chi router. Why are we not doing that? It would have caught the problem in the comment above.
There was a problem hiding this comment.
because the plumbing of handlers and middlewares originally was agnostic to http and grpc, see #5209 This brings us back to rewriting the graph service. The webfinger PR uses the correct plumbing if you are interested: #5373 and specifically the service interface in https://github.com/owncloud/ocis/pull/5373/files#diff-ae7a50755d96a63adb77b0843780b8e849d31dabe326d9db3b0593c222aa8d8dR17-R19
rhafer
force-pushed
the
education-classes
branch
2 times, most recently
from
dea6ec7 to
adb0dab
Compare
err_school.go implements the full education interface not just schools. ldap_school.go renamed to ldap_education_school.go for making it consistent with ldap_education_user.go
The acutal backend implementations are still empty.
rhafer
force-pushed
the
education-classes
branch
from
adb0dab to
6a9108b
Compare
|
Kudos, SonarCloud Quality Gate passed! |
Author: Ralf Haferkamp <[email protected]> Date: Wed Jan 11 20:38:18 2023 +0100 graph:Add stubs for education/classes endpoints (#5360) * Renamed files for consistency reasons err_school.go implements the full education interface not just schools. ldap_school.go renamed to ldap_education_school.go for making it consistent with ldap_education_user.go * graph: Add stubs for education/classes endpoints The acutal backend implementations are still empty.
Description
This adds an implementation of the /graph/v1.0/education/classes/ endpoints. It borrows heavily from
the /groups/ endpoints. The actual backend implementation for LDAP will follow in a separate PR.