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.

Sign up for GitHub

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

Jump to bottom

Data models #4

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

Data models #4

wants to merge 8 commits into from

Conversation

Albert-2
Copy link

Short description of the Issue.

Add the data models for the database given here

Proposed Changes

The changes i.e. data models are added in models folder

Further comments

@Albert-2 Albert-2 mentioned this pull request Jan 26, 2024
Open
@SuhailAhmed2627
Copy link
Contributor

@Albert-2 We only need models that are given in the link. I believe you have added the enums as models as well. We can specify enums in mongoose models. Look into it and update

SuhailAhmed2627
SuhailAhmed2627 requested changes Jan 29, 2024
View reviewed changes
Copy link
Contributor

@SuhailAhmed2627 SuhailAhmed2627 left a comment •
edited
Loading

Choose a reason for hiding this comment

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

Firstly, Great Job 🥳
Here are some changes:

  • You have created all the schemas in the models folder. Kindly read the documentation to know the difference between schemas and models. Though used interchangeably, we are going to use it as specified in the docs.
  • Add schema should be of named as entity.schema.ts where entity should be singular.
  • Make sure to declare entity collection type as well. Refer this
  • Go through all the comments.
  • Write a better PR, Refer this

server/models/admins.models.ts Outdated Show resolved Hide resolved
server/models/access.models.ts Outdated Show resolved Hide resolved
server/models/constants.models.ts Outdated Show resolved Hide resolved
server/models/gender.models.ts Outdated Show resolved Hide resolved
server/models/messes.models.ts Outdated Show resolved Hide resolved
server/models/userDetails.models.ts Outdated Show resolved Hide resolved
server/models/users.models.ts Outdated Show resolved Hide resolved
@Albert-2
Copy link
Author

Albert-2 commented Jan 29, 2024 •
edited
Loading

Hey @SuhailAhmed2627 ....!!
Done with changes you have asked for
Thank you for being cooperative
really appreciated....!!:)

@SuhailAhmed2627
Copy link
Contributor

SuhailAhmed2627 commented Jan 30, 2024 •
edited
Loading

  • You have created all the schemas in the models folder. Kindly read the documentation to know the difference between schemas and models. Though used interchangeably, we are going to use it as specified in the docs.
  • Add schema should be of named as entity.schema.ts where entity should be singular.
  • Make sure to declare entity collection type as well. Refer this

The above tasks are still pending. @Albert-2

@Albert-2
Copy link
Author

hey @SuhailAhmed2627
I've move and renamed the schemas according to what you have asked
Thank You....!:)

@SuhailAhmed2627
Copy link
Contributor

hey @SuhailAhmed2627 I've move and renamed the schemas according to what you have asked Thank You....!:)

Great, Just remove the unwanted models you have added

@Albert-2
Copy link
Author

image

can you please tell me which models you are talking about

@SuhailAhmed2627
Copy link
Contributor

@Albert-2 You have created schemas which are not given in here. Such as access and gender

@Albert-2
Copy link
Author

hey @SuhailAhmed2627
I am so sorry i am not getting it cause i have not added any gender schema in the first place and about the access schema
is this what you want

image

@SuhailAhmed2627
Copy link
Contributor

image

Exactly Like this! Awesome and don't be sorry for anything!

@Albert-2
Copy link
Author

Thank you so much @SuhailAhmed2627
Actually am very new for this backend/database things hence saw it more as a learning opportunity and thankfully you were also so co-operative too....!:)
Thank you once again..!

SuhailAhmed2627
SuhailAhmed2627 requested changes Jan 31, 2024
View reviewed changes
Copy link
Contributor

@SuhailAhmed2627 SuhailAhmed2627 left a comment

Choose a reason for hiding this comment

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

Few Changes:

  • Go through all the comments
  • Update the user schema according to the dbdiagram
  • All schema declaration, collection, and type of collection should be singular

server/schemas/admin.schema.ts Outdated Show resolved Hide resolved
server/schemas/constant.schema.ts Outdated Show resolved Hide resolved
server/schemas/mess.schema.ts Outdated Show resolved Hide resolved
yarn.lock Outdated
Copy link
Contributor

Choose a reason for hiding this comment

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

Why is there a change in yarn.lock? Revert this

Copy link
Contributor

Choose a reason for hiding this comment

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

@Albert-2

@Albert-2
Copy link
Author

hey @SuhailAhmed2627
Done changes ...!!
Thanks....!!:)

SuhailAhmed2627
SuhailAhmed2627 requested changes Jan 31, 2024
View reviewed changes
yarn.lock Outdated
Copy link
Contributor

Choose a reason for hiding this comment

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

@Albert-2

@Albert-2
Copy link
Author

Albert-2 commented Jan 31, 2024 •
edited
Loading

@SuhailAhmed2627
i have reverted all changes from yarn.lock file as you asked
now it is same as your main yarn.lock fil

SuhailAhmed2627
SuhailAhmed2627 reviewed Feb 1, 2024
View reviewed changes
yarn.lock Outdated
@@ -7547,4 +7547,4 @@ __metadata:
resolution: "yocto-queue@npm:0.1.0"
checksum: dceb44c28578b31641e13695d200d34ec4ab3966a5729814d5445b194933c096b7ced71494ce53a0e8820685d1d010df8b2422e5bf2cdea7e469d97ffbea306f
languageName: node
linkType: hard
linkType: hard
Copy link
Contributor

Choose a reason for hiding this comment

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

Fully revert this file. It shows some whitespace changes

@SuhailAhmed2627
Copy link
Contributor

@Albert-2 Revert yarn.lock
@RaghavKhullar Could you take a look at this? Looks good to me

@Albert-2
Copy link
Author

Albert-2 commented Feb 1, 2024

@SuhailAhmed2627
I literally copy pasted your yarn.lock file and pasted it and make a PR

@SuhailAhmed2627
Copy link
Contributor

@SuhailAhmed2627
I literally copy pasted your yarn.lock file and pasted it and make a PR

In github, file changes are showing a change. It's probably some whitespace change. Kindly revert it

@Albert-2
Copy link
Author

Albert-2 commented Feb 1, 2024

@SuhailAhmed2627
i am so sorry i literally tried every possible way to revert it
but can't able do it....!!
please consider this PR
Thank You...!

@SuhailAhmed2627
Copy link
Contributor

@Albert-2 Did you try deleting your yarn.lock, and downloading the yarn.lock from the main branch?

@Albert-2
Copy link
Author

Albert-2 commented Feb 1, 2024

@SuhailAhmed2627
No I didn't
but tried copy pasting from your main to my local yarn.lock file

@SuhailAhmed2627
Copy link
Contributor

@Albert-2 Try downloading, Copy-Pasting might have only led to the problem

@Albert-2
Copy link
Author

Albert-2 commented Feb 1, 2024

@SuhailAhmed2627
Done it's just a one whitespace

@SuhailAhmed2627
Copy link
Contributor

Great, I am approving this PR. Once @RaghavKhullar takes a look, then we are good to merge!

@SuhailAhmed2627 SuhailAhmed2627 self-requested a review February 1, 2024 13:09
@Albert-2
Copy link
Author

Albert-2 commented Feb 1, 2024

hey @SuhailAhmed2627
Thank you so much......!! :)

RaghavKhullar
RaghavKhullar requested changes Feb 2, 2024
View reviewed changes
server/schemas/mess.schema.ts Outdated Show resolved Hide resolved
server/schemas/mess.schema.ts Outdated Show resolved Hide resolved
@Albert-2
Copy link
Author

Albert-2 commented Feb 3, 2024

hey @RaghavKhullar
Done....!!
Thank You...!:)

@Albert-2
Copy link
Author

Albert-2 commented Feb 6, 2024

@RaghavKhullar

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@RaghavKhullar RaghavKhullar RaghavKhullar requested changes

@SuhailAhmed2627 SuhailAhmed2627 SuhailAhmed2627 approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@Albert-2 @SuhailAhmed2627 @RaghavKhullar