-
Notifications
You must be signed in to change notification settings - Fork 0
Initial version of Technologies Entity #213
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
Conversation
Check the documentation preview: https://67be2390efa6c68786082d66--niaefeup-backend-docs.netlify.app |
Check the documentation preview: https://67be358a8bf15aa75c87cb54--niaefeup-backend-docs.netlify.app |
Check the documentation preview: https://67c181f18387df63abde3246--niaefeup-backend-docs.netlify.app |
Check the documentation preview: https://68024e2fac75ad3a71bfd592--niaefeup-backend-docs.netlify.app |
Check the documentation preview: https://68024e48f2645c3a88e27178--niaefeup-backend-docs.netlify.app |
Check the documentation preview: https://6807db6af5880f4241786e28--niaefeup-backend-docs.netlify.app |
Check the documentation preview: https://680cf819d7c7104793a55d6c--niaefeup-backend-docs.netlify.app |
…ology list types in Project
Check the documentation preview: https://6841d4583804235e7ba68693--niaefeup-backend-docs.netlify.app |
Check the documentation preview: https://6841d45e1ddac96111a76a92--niaefeup-backend-docs.netlify.app |
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.
Pull Request Overview
Initial implementation of a new Technology entity and integration into projects, including CRUD endpoints, DTOs, services, repository, seeders, tests, and documentation updates.
- Introduce
Technology
JPA entity, repository, service, controller, DTO, and error messages. - Update
Project
model/service/DTO to referenceTechnology
viatechnologiesIds
. - Augment seeders, controller tests, and payload schema documentation for technologies.
Reviewed Changes
Copilot reviewed 18 out of 28 changed files in this pull request and generated 1 comment.
Show a summary per file
File | Description |
---|---|
src/main/kotlin/pt/up/fe/ni/website/backend/model/Technology.kt | Added Technology entity |
src/main/kotlin/pt/up/fe/ni/website/backend/repository/TechnologyRepository.kt | Added repository interface |
src/main/kotlin/pt/up/fe/ni/website/backend/service/TechnologyService.kt | Added service with CRUD operations |
src/main/kotlin/pt/up/fe/ni/website/backend/controller/TechnologyController.kt | Added REST controller for technologies |
src/main/kotlin/pt/up/fe/ni/website/backend/dto/entity/TechnologyDto.kt | Added DTO for technology payload |
src/main/kotlin/pt/up/fe/ni/website/backend/service/activity/ProjectService.kt | Updated to map technologiesIds to entities |
src/main/kotlin/pt/up/fe/ni/website/backend/model/Project.kt | Changed technologies to a list of Technology with JPA mapping |
src/main/kotlin/pt/up/fe/ni/website/backend/dto/entity/ProjectDto.kt | Renamed technologies → technologiesIds |
src/main/kotlin/pt/up/fe/ni/website/backend/service/ErrorMessages.kt | Added technology-related error messages |
src/main/kotlin/pt/up/fe/ni/website/backend/model/seeders/TechnologySeeder.kt | New seeder for technologies |
src/main/kotlin/pt/up/fe/ni/website/backend/model/seeders/ProjectSeeder.kt | Integrated TechnologySeeder and assign technologies |
src/main/kotlin/pt/up/fe/ni/website/backend/model/seeders/ApplicationSeeder.kt | Hooked TechnologySeeder into application seeding |
src/test/kotlin/pt/up/fe/ni/website/backend/controller/TechnologyControllerTest.kt | End-to-end tests for the new endpoints |
src/test/kotlin/pt/up/fe/ni/website/backend/controller/ProjectControllerTest.kt | Updated project tests to assert technology objects |
src/test/kotlin/pt/up/fe/ni/website/backend/utils/documentation/payloadschemas/model/PayloadTechnology.kt | Added tech payload schema |
src/test/kotlin/pt/up/fe/ni/website/backend/utils/documentation/payloadschemas/model/PayloadProject.kt | Updated project payload schema for tech |
Comments suppressed due to low confidence (2)
src/test/kotlin/pt/up/fe/ni/website/backend/controller/TechnologyControllerTest.kt:161
- There's no test covering the case when creating a technology with a duplicate name. Please add a test that submits an existing name and asserts a BadRequest with the
technology with name already exists
error.
@Test
src/main/kotlin/pt/up/fe/ni/website/backend/model/Project.kt:32
- [nitpick] Mapping technologies as
@OneToMany
may not reflect that a technology can belong to multiple projects. Consider using@ManyToMany
with a join table to model this relationship correctly.
@OneToMany(fetch = FetchType.EAGER)
src/main/kotlin/pt/up/fe/ni/website/backend/service/TechnologyService.kt
Show resolved
Hide resolved
Check the documentation preview: https://686d5d8a95dbdea3df699606--niaefeup-backend-docs.netlify.app |
Check the documentation preview: https://687b8fea4eebc9b90dca3bbc--niaefeup-backend-docs.netlify.app |
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.
This is looking good! I left a few minor suggestions to help maintain consistency across the codebase.
Also, I noticed that image file extensions are being removed from filenames when making PUT
or POST
requests via Postman. I haven't had time to investigate further, could you check if you experience the same issue?
src/main/kotlin/pt/up/fe/ni/website/backend/controller/TechnologyController.kt
Outdated
Show resolved
Hide resolved
src/main/kotlin/pt/up/fe/ni/website/backend/model/Technology.kt
Outdated
Show resolved
Hide resolved
src/main/kotlin/pt/up/fe/ni/website/backend/service/TechnologyService.kt
Outdated
Show resolved
Hide resolved
src/main/kotlin/pt/up/fe/ni/website/backend/service/TechnologyService.kt
Outdated
Show resolved
Hide resolved
src/main/kotlin/pt/up/fe/ni/website/backend/controller/TechnologyController.kt
Outdated
Show resolved
Hide resolved
src/main/kotlin/pt/up/fe/ni/website/backend/controller/TechnologyController.kt
Outdated
Show resolved
Hide resolved
src/main/kotlin/pt/up/fe/ni/website/backend/controller/TechnologyController.kt
Outdated
Show resolved
Hide resolved
src/main/kotlin/pt/up/fe/ni/website/backend/service/TechnologyService.kt
Show resolved
Hide resolved
Check the documentation preview: https://689238f4d66a7765bccb4d6d--niaefeup-backend-docs.netlify.app |
Nice work! Did you have the opportunity to investigate this further @pedroafmonteiro ?
|
Check the documentation preview: https://68a4805ff7f92d00b77de2ce--niaefeup-backend-docs.netlify.app |
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 re-tested the issue mentioned earlier, and it’s now working fine 🤷. I just left a few minor comments, once those are addressed this should be ready to merge!
src/main/kotlin/pt/up/fe/ni/website/backend/service/TechnologyService.kt
Outdated
Show resolved
Hide resolved
src/main/kotlin/pt/up/fe/ni/website/backend/service/TechnologyService.kt
Outdated
Show resolved
Hide resolved
src/main/kotlin/pt/up/fe/ni/website/backend/controller/TechnologyController.kt
Outdated
Show resolved
Hide resolved
Check the documentation preview: https://68a4dba58329f83164a20f0e--niaefeup-backend-docs.netlify.app |
Check the documentation preview: https://68b1f1afb03f1c1b5546a3a4--niaefeup-backend-docs.netlify.app |
Check the documentation preview: https://68b1f7d2adb06d5631d8def0--niaefeup-backend-docs.netlify.app |
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.
Great work! ✅ 💯
Check the documentation preview: |
Closes #211
Review checklist