-
Notifications
You must be signed in to change notification settings - Fork 42
Fix: PUT mentor api #460
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
Fix: PUT mentor api #460
Conversation
src/main/java/com/wcc/platform/domain/platform/member/MemberDto.java
Outdated
Show resolved
Hide resolved
...in/java/com/wcc/platform/repository/postgres/mentorship/PostgresMenteeSectionRepository.java
Show resolved
Hide resolved
What is currently not working as expected is the update of fields within objects (like menteeSection, skills, resources). In this case, MentorDto is only checking if they are empty, but it doesn't check in detail if each underlying field is empty or not and what to ignore for update. Therefore, if empty fields are provided within these objects, they will overwrite existing content. If we want this, I wouldn't anyway include the checks in the MentorDto, but on the lower level (for ex, PostgressMenteeSectionReposiotory, PostgresMentorRepository). Let me know what you think! |
src/main/java/com/wcc/platform/domain/platform/mentorship/Mentor.java
Outdated
Show resolved
Hide resolved
src/main/java/com/wcc/platform/domain/platform/mentorship/MentorDto.java
Show resolved
Hide resolved
src/test/java/com/wcc/platform/domain/platform/mentorship/MentorDtoTest.java
Show resolved
Hide resolved
src/main/java/com/wcc/platform/repository/postgres/PostgresMenteeSectionRepository.java
Outdated
Show resolved
Hide resolved
src/test/java/com/wcc/platform/repository/postgres/PostgresMenteeSectionRepositoryTest.java
Outdated
Show resolved
Hide resolved
…guages input in MentorTest for test
src/main/java/com/wcc/platform/domain/platform/mentorship/Mentor.java
Outdated
Show resolved
Hide resolved
src/main/java/com/wcc/platform/domain/platform/mentorship/MentorDto.java
Outdated
Show resolved
Hide resolved
…or.java Co-authored-by: Adriana Zencke Zimmermann <dricazenck@gmail.com>
…orDto.java Co-authored-by: Adriana Zencke Zimmermann <dricazenck@gmail.com>
|
@nycotin could you update your branch and solve the conflicts so we can merge the changes? |
…x-put-mentor-api # Conflicts: # src/main/java/com/wcc/platform/domain/platform/mentorship/MentorDto.java # src/main/java/com/wcc/platform/repository/postgres/mentorship/PostgresMenteeSectionRepository.java # src/main/java/com/wcc/platform/service/MentorshipService.java
|
@dricazenck I have merged main and now I am getting quite some issue from pmdAll that are unrelated to what I have done for this branch: There is also a refactoring of MentorshipServiceTest that will take me quite some time, so I am wondering if I wouldn't make more sense to just merge this so the fix is done and then take care the pmd issues/refactoring in another branch? |
|
@nycotin Looks like it is everything working from github actions with PMD and testing. So I will go ahead and merge. I will pull the changes locally to see what is going one. Thank you!! 🚀 |

Description
Screenshots
Change Type
Pull request checklist
Please check if your PR fulfills the following requirements: