-
Notifications
You must be signed in to change notification settings - Fork 18
feat: Add v3 projects route proof of concept (DEV-5216) #3720
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Conversation
7cfdd63
to
3b4c995
Compare
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## main #3720 +/- ##
===========================================
- Coverage 82.54% 63.26% -19.29%
===========================================
Files 327 385 +58
Lines 22482 24820 +2338
===========================================
- Hits 18558 15702 -2856
- Misses 3924 9118 +5194 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
7be400d
to
37b766f
Compare
|
||
object ProjectsDto { | ||
|
||
final case class ProjectShortcodeParam(value: String) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
no need to duplicate this, you can use Shortcode directly, see other endpoints doing this.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I am aware of this, I tried to re-use as little as possible from our existing code base, in hope to make it as clear as possible how this slice works in isolation. I suppose I should have clarified this in the RFC, but forgot.
Of course if we intend to merge this, we should eliminate duplication where it does not make sense.
Though, particularly with our DTOs, it was pointed out to me by Julien, that we have some cases where the OpenAPI documentation does not align with the actually returned JSON structure, which we may want to look into before re-using them in any new route. (The topic is on the maintenance roadmap, and one instance I experimented around with in #3696)
opaque type ClassIri = String | ||
opaque type ProjectShortcode = String | ||
opaque type ProjectShortname = String | ||
opaque type LanguageCode = String |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
all the above types already exist. IMHO the domain model does not have to be duplicated. What is the thought behind creating a V3 domain model?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Agreed. See comment above.
import org.knora.webapi.slice.admin.domain.model.KnoraProject.Shortname | ||
import org.knora.webapi.slice.v3.projects.domain.model.DomainTypes.* | ||
|
||
trait ProjectsRepo { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We already have a Repository, IMHO duplicating the repo and service is not necessary.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Agreed. See comment above.
Co-Authored-By: Claude <[email protected]>
37b766f
to
f2344c6
Compare
Description
This PR introduces a new V3 API for projects with a modern error handling approach. The implementation includes:
/v3/projects/{id}
endpoint to retrieve project information by ID (shortcode, shortname, or IRI)/v3/projects/{id}/resource-counts
endpoint to get resource instance counts by classThe V3 API is designed to be more developer-friendly with consistent error responses, better documentation, and optimized performance through caching.
Details on the RFC, for which this PoC serves, can be found here.