hassanhabib / OtripleS

This is an open source schooling system, dedicated to provide a better experience for schools needing a management and communication and tutoring system all in one place. This project is aiming toward directing all the software development funds and hours to families in need, the idea of the project is to allow schools to use the system as long as the software funds in the school are directed towards financially disadvantaged families and students.
325 stars 166 forks source link

Discuss Student Model #1

Closed hassanhabib closed 4 years ago

hassanhabib commented 4 years ago

We need to conceptualize what a foundational student model properties are supposed to have. Currently I'm thinking of the following:

    public class Student
    {
        public Guid Id { get; set; }
        public string FirstName { get; set; }
        public string MiddleName { get; set; }
        public string LastName { get; set; }
        public DateTimeOffset DateOfBirth { get; set; }
        public DateTimeOffset CreatedDate { get; set; }
        public Guid CreatedBy { get; set; }
        public DateTimeOffset UpdatedDate { get; set; }
        public Guid UpdatedBy { get; set; }
    }

If you think we need to add more or remove some of the aforementioned properties let's discuss, let us know what you think. The current model will be locked for development by the end of the week on 28th of June 2020

devmanzur commented 4 years ago

ContactNumber, GuradianName, ClassTeacher, CurrentClass, RoleNumber,

tajwal commented 4 years ago

FatherName,Fathers email and mobile and gender of student.

HatemGamal commented 4 years ago
devmanzur commented 4 years ago
  • the student's age when joined, religion, nationality, preferred language
  • kin information
  • Whether Handicap or not
  • Other Notes to specific any disabilities or others

I'd say the date of birth would be more applicable instead of age.

hassanhabib commented 4 years ago
  • the student's age when joined, religion, nationality, preferred language
  • kin information
  • Whether Handicap or not
  • Other Notes to specific any disabilities or others

I'd say the date of birth would be more applicable instead of age.

I agree the date of birth is more dynamic than a static value that will require updating as time passes. I however, like the idea of indicating whether a student is handicapped or not - this is a good point.

hassanhabib commented 4 years ago

FatherName,Fathers email and mobile and gender of student.

@tajwal I wonder if these values should be in a different related entity, not directly in the main student entity.

HatemGamal commented 4 years ago
tajwal commented 4 years ago

FatherName,Fathers email and mobile and gender of student.

@tajwal I wonder if these values should be in a different related entity, not directly in the main student entity.

Mr @hassanhabib, Agrees with you In case when we wants to have more relatives of a student then we can have an entity which will have one to many relation with student and will store more information like relation(father, brother, mother,..), First name,Last name,mobile,email, and may be more columns.

ShreyasJejurkar commented 4 years ago

Better we separate the AuditFields from domain model like last 4 properties from the above student model. We can introduce a new model namely BaseEntity and can have those fields. So that we can use those fields in other domain models as well like School or Teachers.

public class BaseEntity
{
  public DateTimeOffset CreatedDate { get; set; }
  public Guid CreatedBy { get; set; }
  public DateTimeOffset UpdatedDate { get; set; }
  public Guid UpdatedBy { get; set; }
}

And whenever we need to use this just inherit the domain model from this base class. Like below.

public class Teacher : BaseEntity
{
  public string Name {get;set;}
}
Driedas commented 4 years ago

FatherName,Fathers email and mobile and gender of student.

@tajwal I wonder if these values should be in a different related entity, not directly in the main student entity.

I think that there should be a list of contact people to reach out to in case of issues. Not even just the parents themselves, possibly grandparents or other people (maybe not even necessarily relatives) to be notified as well. I see this more as a m:n contact entity with contact type/relation to student. M;N for cases like having multiple children attending the same school. Can be simplified to 1:N if the identity of the contacts is not important (e. g. parent of child A is the same person as parent of child B)

ahmetsekmen commented 4 years ago

"StudentLevel" (first class , second third ..)

BaseEntity could be like this also

public class BaseEntity { public virtual T Id { get; set; } public DateTimeOffset CreatedDate { get; set; } public Guid CreatedBy { get; set; } public DateTimeOffset UpdatedDate { get; set; } public Guid UpdatedBy { get; set; } }

ShreyasJejurkar commented 4 years ago

@ahmetsekmen better to make Id as 'Guid' only, rather than taking T as generic one. Because all users will be having guid as id, I can confirm this by looking at CreatedBy field as it will store guid of user who created that object or record! So better to have guid explicitly.

ahmetsekmen commented 4 years ago

@MCCshreyas For users(admin,student,teacher..), guid is ok but what if other type of entities which they need audit properties. Maybe we will decide to choose integer Id for those entities. just an idea.

ShreyasJejurkar commented 4 years ago

@ahmetsekmen Can you please provide example of other entity that you are trying to refer? That will be helpful for me to understand!

ahmetsekmen commented 4 years ago

@MCCshreyas BaseEntity; public class BaseEntity< T > { public virtual T Id { get; set; } public DateTimeOffset CreatedDate { get; set; } public Guid CreatedBy { get; set; } public DateTimeOffset UpdatedDate { get; set; } public Guid UpdatedBy { get; set; } }

Here is how we will use; public class Lesson : BaseEntity< int > { public string LessonName { get; set; } public string LessonHour { get; set; } }

For student; public class Student : BaseEntity< Guid > { public string FirsName { get; set; } public string MiddleName { get; set; } public string LastName { get; set; } ... .. }

devmanzur commented 4 years ago

@MCCshreyas BaseEntity; public class BaseEntity< T > { public virtual T Id { get; set; } public DateTimeOffset CreatedDate { get; set; } public Guid CreatedBy { get; set; } public DateTimeOffset UpdatedDate { get; set; } public Guid UpdatedBy { get; set; } }

Here is how we will use; public class Lesson : BaseEntity< int > { public string LessonName { get; set; } public string LessonHour { get; set; } }

For student; public class Student : BaseEntity< Guid > { public string FirsName { get; set; } public string MiddleName { get; set; } public string LastName { get; set; } ... .. }

I guess we all agree that there should be a base entity with the given fields. Let's discuss what other properties do we need in the student entity itself.

devmanzur commented 4 years ago

I guess guardians should be a separate entity itself, with courses, fees etc

devmanzur commented 4 years ago

we may add Admission Date, BatchId/SessionId, StudentImageUrl

ShreyasJejurkar commented 4 years ago

@ahmetsekmen First a fall I need to highlight a thing that we should not use any domain specific property as primary key in database tables. I can see your Lesson domain entity Let's say I have following records.

  1. Reflection
  2. Generics
  3. Functions

But let's say in future if school decided to swap those lessons, now they want Functions as first lesson, how will you do it now? As you have lessonno as primary key, you can't satisfy the new school requirements.

That's what I want to highlight is that we should never ever consider domain properties as primary key for tables although it may be unique. So we should always have our extra property that can acts as primary key. Because you never know when domain requirement changes! 😉

hassanhabib commented 4 years ago

Very good points from everyone - please note that we need to separate additional details entities from information that might be inseparable from the main entity, for instance, a student date of birth is inseparable information, their student identification number, but the guardians, contact information, teachers, classes all of this are related entities not part of the main entity we are trying to conceptualize here.

Also about the audit fields, from my personal experience I highly recommend maintaining each model fields within the model itself, as @ahmetsekmen pointed out, I have run into situations where audit fields are different from one entity to the other - also keeping these fields in the same model file makes it easier to understand what the model entails without having to jump between two files to get the full picture about what a particular model represents - thoughts?

hassanhabib commented 4 years ago

@a-urel might have some input on what a student entity is supposed to look like, care to share @a-urel ?

ShreyasJejurkar commented 4 years ago

@hassanhabib its just a one inheritance chain for Audit fields to domain. If we include every single field of audit in every domain object without a common one, that code will tend to duplicate. And also consider a situation where we are introducing a soft delete feature in Application where IsDelete represent the status of delete then we need to add this fields to every single domain object by going to its individual file, but if we have a common base audit model then we can include that field there and it will start working for all of then domain models provided that they are inherited from it.

And also the data coming into this fields will be most likely from a common method most likely it will be dbcontext.SaveChangesAsync(), if by mistake if any one missed a any audit field to include it in domain, it will throw a error at runtime.

So it's better to have common model for Audit rather than copying every single field to every single domain entity. Avoiding duplications. And also it's a single level inheritance, not much. I will prefer jumping between two files rather than duplication. 😉

hassanhabib commented 4 years ago

@MCCshreyas would you entertain the idea of an interface of audit fields? this way consistency is enforced and duplication is only generated, yet visibility of any model's properties are all aggregated in one single file. you don't have to type the audit fields, something like:

public interface IAuditable {
     public DateTimeOffset CreatedDate {get; set;}
     public DateTimeOffset UpdatedDate {get; set;}
}

public class Student: IAuditable {
      public DateTimeOffset CreatedDate {get; set;}
      public DateTimeOffset UpdatedDate {get; set;}
}
ShreyasJejurkar commented 4 years ago

@hassanhabib yeah it's OK. But does that mean that we need to copy and paste IAuditable to every single domain entity, I mean in its file!? 🤔🤔🙄

a-urel commented 4 years ago

@a-urel might have some input on what a student entity is supposed to look like, care to share @a-urel ?

@hassanhabib Since we need to conceptualize 'foundational' student model, we should keep it simple. I think it's useful to assume property values of this class don't change by student's academic status and activities. So it should contain only personal and legal information about students.

hassanhabib commented 4 years ago

@hassanhabib yeah it's OK. But does that mean that we need to copy and paste IAuditable to every single domain entity, I mean in its file!? 🤔🤔🙄

@MCCshreyas nope, you just implement it and that's it - you don't have to copy the interface it's written once and used everywhere

ShreyasJejurkar commented 4 years ago

@hassanhabib again it's two file right? Domain class and IAuditable interface file!

tajwal commented 4 years ago

@MCCshreyas would you entertain the idea of an interface of audit fields? this way consistency is enforced and duplication is only generated, yet visibility of any model's properties are all aggregated in one single file. you don't have to type the audit fields, something like:

public interface IAuditable {
     public DateTimeOffset CreatedDate {get; set;}
     public DateTimeOffset UpdatedDate {get; set;}
}

public class Student: IAuditable {
      public DateTimeOffset CreatedDate {get; set;}
      public DateTimeOffset UpdatedDate {get; set;}
}

Mr. @hassanhabib what about something like below

public interface IBaseModel { DateTimeOffset CreatedDate { get; set; } DateTimeOffset UpdatedDate { get; set; } Guid CreatedBy { get; set; } Guid UpdatedBy { get; set; } bool IsDelete { get; set; } }

public class BaseModel : IBaseModel { [Key] [DatabaseGenerated(DatabaseGeneratedOption.Identity)] public int Id { get; set; }

    [Column(TypeName = "datetime2")]
    public DateTimeOffset CreatedDate { get; set; } = DateTime.Now;

    [Column(TypeName = "datetime2")]
    public DateTimeOffset UpdatedDate { get; set; } = DateTime.Now;
    public Guid CreatedBy { get; set; }
    public Guid UpdatedBy { get; set; }

    [DefaultValue(false)]
    public bool IsDelete { get; set; }
}

Public Class Student:BaseModel { ......... }

a-urel commented 4 years ago

We can decide base classes or auditing strategies later.

Premature optimization is the root of all evil

devmanzur commented 4 years ago
public class Student : IAuditable { 
  public Guid Id { get; set; }
  public string FirstName { get; set; }
  public string MiddleName { get; set; } 
  public string LastName { get; set; }
  public DateTimeOffset DateOfBirth { get; set; }
  public DateTimeOffset AdmissionDate { get; set; }
  public Session Session { get; set; }
  public string ImageUrl { get; set; }
 }
hassanhabib commented 4 years ago
public class Student : IAuditable { 
  public Guid Id { get; set; }
  public string FirstName { get; set; }
  public string MiddleName { get; set; } 
  public string LastName { get; set; }
  public DateTimeOffset DateOfBirth { get; set; }
  public DateTimeOffset AdmissionDate { get; set; }
  public Session Session { get; set; }
  public string ImageUrl { get; set; }
 }

@devmanzur looks good, I think Admission date and images should be in external entities, for instance here the U.S they have mutliple yearbook photos, so it should be external. I think the rest is fine, but what is a session, and shouldn't you need a SessionId to fill in that related entitiy?

antoniolinhart commented 4 years ago

Should this class contain contact information like emailAddress or phoneNumber? What are the thoughts on how communication to the students will work, will it only be in-app?

Also, are we only supporting English? If not, nativeLanguage could be used to display the site in their native language. This might be more of a separate setting than a field for this class

Edit: Saw contact info mentioned in other comment about related father contact info, but what about student contact info?

Mohamad-ali-8 commented 4 years ago

If Id for a student is Guid, I would recommend to also add another Reference number, an incremental number that starts somewhere. It is very handy later on for UI, searching purposes, and more meaningful for reporting. I found this very useful in one of the applications that I worked on.

public long ReferenceNumber { get; set; }

We added for it a sequence for DB like this:

   modelBuilder.Entity<Student>()
            .Property(o => o.ReferenceNumber)
            .HasDefaultValueSql("NEXT VALUE FOR dbo.ReferenceNumberSequence");
        modelBuilder
            .HasSequence<int>("ReferenceNumberSequence", schema: "dbo")
            .StartsAt(2365147).IncrementsBy(1);

I know I shouldn't think about the DB yet, but I just wanted to make it clearer.

devmanzur commented 4 years ago
public class Student : IAuditable { 
  public Guid Id { get; set; }
  public string FirstName { get; set; }
  public string MiddleName { get; set; } 
  public string LastName { get; set; }
  public DateTimeOffset DateOfBirth { get; set; }
  public DateTimeOffset AdmissionDate { get; set; }
  public Session Session { get; set; }
  public string ImageUrl { get; set; }
 }

@devmanzur looks good, I think Admission date and images should be in external entities, for instance here the U.S they have mutliple yearbook photos, so it should be external. I think the rest is fine, but what is a session, and shouldn't you need a SessionId to fill in that related entitiy?

@hassanhabib Sessions/Batch to indicate which batch the user belongs to for instance I was session 2016. Batch 87. These attributes help to identify which sessions I was part of. Yeah, I missed the SessionId field.

hiraldesai commented 4 years ago

Demographics information like gender, race, etc.

1) are we planning on storing it? 2) if we are, are they considered as “separable” information to the primary entity?

Driedas commented 4 years ago

What use cases is this model supposed to support? It's difficult to come up with a model when it's not clear what it should be doing... The basic properties are there, requirements will tell us what needs to be added and what relationships there will be.

hassanhabib commented 4 years ago

Demographics information like gender, race, etc.

  1. are we planning on storing it?
  2. if we are, are they considered as “separable” information to the primary entity?

@hiraldesai this is an interesting topic, I think gender and race are inseparable from the main entity, but the question here would be why would we need to have that information at all in a student entity? one might argue maybe for particular curriculum in some countries, particular activities or such, and that might be a legitimate argument but from a global standpoint if we are building a schooling system generic enough to handle basic schooling needs I think these information might be on the side of external information rather than something fundamental to the entity to itself, what do you think?

hassanhabib commented 4 years ago

What use cases is this model supposed to support? It's difficult to come up with a model when it's not clear what it should be doing... The basic properties are there, requirements will tell us what needs to be added and what relationships there will be.

@Driedas that's true. the one requirement we have for this issue is to conceptualize the basic information we need to have in a root entity for students - something to attach all other additional information to, despite what the additional information maybe depending on the requirement. In other words, the student model should be a basic model/entity that contains basic information about a student in school - things every student must have like names, date of birth ... etc.

hassanhabib commented 4 years ago
public class Student : IAuditable { 
  public Guid Id { get; set; }
  public string FirstName { get; set; }
  public string MiddleName { get; set; } 
  public string LastName { get; set; }
  public DateTimeOffset DateOfBirth { get; set; }
  public DateTimeOffset AdmissionDate { get; set; }
  public Session Session { get; set; }
  public string ImageUrl { get; set; }
 }

@devmanzur looks good, I think Admission date and images should be in external entities, for instance here the U.S they have mutliple yearbook photos, so it should be external. I think the rest is fine, but what is a session, and shouldn't you need a SessionId to fill in that related entitiy?

@hassanhabib Sessions/Batch to indicate which batch the user belongs to for instance I was session 2016. Batch 87. These attributes help to identify which sessions I was part of. Yeah, I missed the SessionId field.

In that case the session and batch are outside of the scope of this model, we are not there yet, we might come back and say let's do these things, but for now, we just need a simple basic student model to contain foundational information that are inseparable from any student entity anywhere in the world. You might argue that sessions and batches are inseparable, and I would argue that there are exchange students that don't necessarily abide by these rules and don't necessarily need that information, what do you think?

hassanhabib commented 4 years ago

If Id for a student is Guid, I would recommend to also add another Reference number, an incremental number that starts somewhere. It is very handy later on for UI, searching purposes, and more meaningful for reporting. I found this very useful in one of the applications that I worked on.

public long ReferenceNumber { get; set; }

We added for it a sequence for DB like this:

   modelBuilder.Entity<Student>()
            .Property(o => o.ReferenceNumber)
            .HasDefaultValueSql("NEXT VALUE FOR dbo.ReferenceNumberSequence");
        modelBuilder
            .HasSequence<int>("ReferenceNumberSequence", schema: "dbo")
            .StartsAt(2365147).IncrementsBy(1);

I know I shouldn't think about the DB yet, but I just wanted to make it clearer.

thanks @Mohamad-ali-8 - this is a good point - in that case why don't we just use a ulong as the data type for student ids instead of Guids?

This is open for everyone to discuss.

a-urel commented 4 years ago

Actually student is a person, and we will have other person types like parents, teachers, personnel, etc. Maybe we will see that a base abstract Person class is needed in the future. But I think it's too early for this type normalizations.

hassanhabib commented 4 years ago

Should this class contain contact information like emailAddress or phoneNumber? What are the thoughts on how communication to the students will work, will it only be in-app?

Also, are we only supporting English? If not, nativeLanguage could be used to display the site in their native language. This might be more of a separate setting than a field for this class

Edit: Saw contact info mentioned in other comment about related father contact info, but what about student contact info?

@antoniolinhart I think contact information are external entity information, simply because a given student might have multiple phone numbers for one student, maybe multiple e-mail addresses, contact information through other platforms, we definitely are going to need all of that, but I'm not sure if should be part of the main entity.

hassanhabib commented 4 years ago

It looks like the model that @devmanzur put together seems to be the closest to what we are going to go with, let me reiterate with a slight change:

public class Student : IAuditable { 
  public Guid Id { get; set; }
  public string FirstName { get; set; }
  public string MiddleName { get; set; } 
  public string LastName { get; set; }
  public DateTimeOffset DateOfBirth { get; set; }
  public DateTimeOffset CreatedDate {get; set;}
  public DateTimeOffset UpdatedDate {get; set;}
  public Guid CreatedBy {get; set;}
  public Guid UpdatedBy {get; set;}
 }

The one issue we want to discuss now, is what should the data type be for an Id? with reference to what @Mohamad-ali-8 talked about earlier.

Driedas commented 4 years ago

Demographics information like gender, race, etc.

  1. are we planning on storing it?
  2. if we are, are they considered as “separable” information to the primary entity?

@hiraldesai this is an interesting topic, I think gender and race are inseparable, but the question here would be why would we need to have that information at all in a student entity? one might argue maybe for particular curriculum in some countries, particular activities or such, and that might be a legitimate argument but from a global standpoint if we are building a schooling system generic enough to handle basic schooling needs I think these information might be on the side of external information rather than something fundamental to the entity to itself, what do you think?

I don't see a use case for race, but I do see one for the sex of the student. This would come into play in coed environments when splitting up children into individual classes (not sure if e. g. the US system works like this, but in the European system that I'm mostly familiar with, kids in every year are split into groups of roughly equal sizes with roughly equal sex representation and stay in those groups for the majority of their time in the school), and for activities like physical education for reasons like gym/locker room capacities (PE is separated by sex, mostly due to locker rooms and different performance expectations, especially during and after puberty).

Driedas commented 4 years ago

@hassanhabib I am in favor of using GUIDs for the root aggregates as IDs 1) the ID can be chosen by the client making the student registration, there is no contention for database generated keys 2) makes synchronization between nodes in a distributed/not always online system easier (e. g. registration of students on a tablet with no internet connection, when done, the set of new students can easily be merged into the existing set) 3) the ID is usually a key in query strings when making queries, and this helps with preventing URL hacking to guess individual student IDs (of course this should not the only security measure, but is a nice addition essentially for free)

Enums and similar can be represented by integers.

hassanhabib commented 4 years ago

@hassanhabib I am in favor of using GUIDs for the root aggregates as IDs

  1. the ID can be chosen by the client making the student registration, there is no contention for database generated keys
  2. makes synchronization between nodes in a distributed/not always online system easier (e. g. registration of students on a tablet with no internet connection, when done, the set of new students can easily be merged into the existing set)
  3. the ID is usually a key in query strings when making queries, and this helps with preventing URL hacking to guess individual student IDs (of course this should not the only security measure, but is a nice addition essentially for free)

Enums and similar can be represented by integers.

@Driedas very strong points - totally agree. anyone else has a counter argument?

a-urel commented 4 years ago

It looks like the model that @devmanzur put together seems to be the closest to what we are going to go with, let me reiterate with a slight change:

public class Student : IAuditable { 
  public Guid Id { get; set; }
  public string FirstName { get; set; }
  public string MiddleName { get; set; } 
  public string LastName { get; set; }
  public DateTimeOffset DateOfBirth { get; set; }
  public DateTimeOffset CreatedDate {get; set;}
  public DateTimeOffset UpdatedDate {get; set;}
  public Guid CreatedBy {get; set;}
  public Guid UpdatedBy {get; set;}
 }

The one issue we want to discuss now, is what should the data type be for an Id? with reference to what @Mohamad-ali-8 talked about earlier.

Id field type of IdentityUser entity is string (nvarchar(450)). BTW, student is also a user and must be either derived from IdentityUser or linked to an IdentityUser.

hassanhabib commented 4 years ago

It looks like the model that @devmanzur put together seems to be the closest to what we are going to go with, let me reiterate with a slight change:

public class Student : IAuditable { 
  public Guid Id { get; set; }
  public string FirstName { get; set; }
  public string MiddleName { get; set; } 
  public string LastName { get; set; }
  public DateTimeOffset DateOfBirth { get; set; }
  public DateTimeOffset CreatedDate {get; set;}
  public DateTimeOffset UpdatedDate {get; set;}
  public Guid CreatedBy {get; set;}
  public Guid UpdatedBy {get; set;}
 }

The one issue we want to discuss now, is what should the data type be for an Id? with reference to what @Mohamad-ali-8 talked about earlier.

Id field type of IdentityUser entity is string (nvarchar(450)). BTW, student is also a user and must be either derived from IdentityUser or linked to an IdentityUser.

@a-urel I like the referencing rather than deriving - decoupling the business entity from a login/auth entity might be the better option - what do you think?

Driedas commented 4 years ago

student/guardian/teacher login credentials should definitely be a separate thing, there are products that already have their own schema for Authentication & Authorization (e. g. Identity server, ASP.NET Identity - or whatever the current iteration of the thing is called)

edit: added teacher, cannot forget about the teachers :-)

a-urel commented 4 years ago

@hassanhabib Will OtripleS satisfy all requirements of all educational institutions, from kindergarten to collage and universities?