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

feat: student schema changes #41

Open
wants to merge 8 commits into
base: main
Choose a base branch
from

Conversation

shikharvashistha
Copy link

Greetings of the day,

This pull adds student schema changes to shiksha platform. Kindly review the pull and let me know in case of any changes if required.

Best
@shikharvashistha

BUILD

image

@coolbung
Copy link
Collaborator

coolbung commented Jun 2, 2022

Hi, the student schema is currently implemented as DTOs here - https://github.com/shiksha-platform/backend-v2/tree/main/src/student/dto

We have deprecated the use of interfaces. Please propose the changes to the DTO itself

@shikharvashistha
Copy link
Author

Hi, the student schema is currently implemented as DTOs here - https://github.com/shiksha-platform/backend-v2/tree/main/src/student/dto

We have deprecated the use of interfaces. Please propose the changes to the DTO itself

Hi @coolbung, the DTO's needs to updated according to the new schema right?

@coolbung
Copy link
Collaborator

coolbung commented Jun 2, 2022

For working on one of the adapter projects, you'd need to create a new adapter like the default one here
https://github.com/shiksha-platform/backend-v2/tree/main/src/adapters

Unless you wish to add any new fields / attributes to the user schema you don't need to modify the user schema.

@shikharvashistha
Copy link
Author

Hi, @coolbung. I've updated as desired, kindly review the changes and let me know of changes if required.

@shikharvashistha
Copy link
Author

Hi @coolbung, can you kindly review the changes and provide me with the feedback if any?

config Outdated Show resolved Hide resolved
@coolbung
Copy link
Collaborator

Awesome start, and thank you so much for this contribution @shikharvashistha ! I have put inline comments in the code, please review those. Also, please create postgres adapters for school, student, teacher & group entities as well. These entities are pre-requisites for attendance.

@coolbung
Copy link
Collaborator

coolbung commented Jul 4, 2022

Hey @shikharvashistha -- will be great if you can update the code so we can get this merged. We're all looking forward to your contribution to the project !

@shikharvashistha
Copy link
Author

shikharvashistha commented Jul 4, 2022

Hey @shikharvashistha -- will be great if you can update the code so we can get this merged. We're all looking forward to your contribution to the project !

Right now engaged in some university work will update asap

PS : @coolbung Updated

@shikharvashistha shikharvashistha requested a review from coolbung July 5, 2022 09:57
@coolbung
Copy link
Collaborator

coolbung commented Jul 7, 2022

Hey @shikharvashistha -- can you add the adapters for other entities : Student, School, Teacher & Group which is required for Attendance.

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.

2 participants