Skip to content
This repository was archived by the owner on Feb 21, 2023. It is now read-only.

Feature/school related models#29

Open
florian-lefebvre wants to merge 2 commits into
op-ent:mainfrom
florian-lefebvre:feature/school-related-models
Open

Feature/school related models#29
florian-lefebvre wants to merge 2 commits into
op-ent:mainfrom
florian-lefebvre:feature/school-related-models

Conversation

@florian-lefebvre
Copy link
Copy Markdown
Contributor

No description provided.

@florian-lefebvre florian-lefebvre added the enhancement New feature or request label Oct 21, 2022
@florian-lefebvre florian-lefebvre self-assigned this Oct 21, 2022
Comment thread app/Models/School.ts
public name: string

@jsonColumn()
public contact: SchoolContact
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

it would be better to have another model for the contact information with a relation to the school (hasOne + belongsTo)

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

I was too lazy I have to admit 😅 I will do this thx

Comment thread app/Models/School.ts
public contact: SchoolContact

@jsonColumn()
public address: SchoolAddress
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

same - it would be better to have another model for the adress with a relation to the school (hasOne + belongsTo)

Comment thread app/Models/User.ts
@MRakox
Copy link
Copy Markdown
Contributor

MRakox commented Nov 12, 2022

I forgot to add it in the review but auto-generated timestamps columns seems useless for some models (createdAt and updatedAt)

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

Labels

enhancement New feature or request

Projects

Status: In Progress

Development

Successfully merging this pull request may close these issues.

2 participants