Skip to content
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

Attendence Endpoint #6

Closed
wants to merge 14 commits into from
Closed

Attendence Endpoint #6

wants to merge 14 commits into from

Conversation

ezhang04
Copy link

@ezhang04 ezhang04 commented Apr 2, 2024

Added Attendance Object. Created and Initialized AttendanceRecordRepository database. Created AttendanceController. Added handlers: createAttendance, getAttendanceById, getAttendances, updateAttendance, deleteAttendance.

Relates (unshare if this closes) to #5

…eck Postgresql in UserService and UserServiceImpl
…ce database controller added handlers. please check implementation and return type
@ezrizhu ezrizhu changed the title Eric's changes (🎆 first ever! 🍾) Attendence Endpoint Apr 2, 2024
Copy link
Member

@ezrizhu ezrizhu left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

blocking until this is tested on your local environment, per our last conversation
Related, please also seek to resolve the CI build failure.

PR Description edited to link related issue.
Title changed to reflect the purpose of the PR.

@ezhang04 ezhang04 self-assigned this Apr 3, 2024
@ezhang04
Copy link
Author

ezhang04 commented Apr 3, 2024

CI is only failing on one testcase. likely a database issue. code is NOT ready for review yet

@ezhang04 ezhang04 changed the title Attendence Endpoint [WIP] Attendence Endpoint Apr 3, 2024
@ezhang04
Copy link
Author

ezhang04 commented Apr 3, 2024

@miguel-merlin Can I get feedback on the code I have written thus far. just to know if I am on the right track. Thank You

Copy link
Member

@miguel-merlin miguel-merlin left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@ezhang04 Looks good! Make sure to change AttendanceRecordRepository.java to repository/attendance/AttendanceRecordRepository.java. Keep it up!

@@ -0,0 +1,49 @@
package com.sitblueprint.admin.controller;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Eric, for future reference we try to stick to conventional commit messages: You can find them here

@@ -20,4 +21,14 @@ public interface UserService {
void disableUserById(Long userId);

void resetPassword(Long userId, String password);

Attendance getAttendanceById(Long attendanceId);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: Attendance getAttendanceById(Long userId). I image this endpoint is to get the attendance given a userId

}

@Override
public Attendance createAttendance(Attendance attendance){
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For this endpoint, you have to check if an Attendance record is already related to a user. If that's the case, return an error since only one attendance record can be related to a user.

@miguel-merlin miguel-merlin changed the title [WIP] Attendence Endpoint Attendence Endpoint Apr 4, 2024
@miguel-merlin
Copy link
Member

@ezhang04 Please do git pull origin main on your working branch. There is a new patch that will make the tests succeed.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants