Discuss Student Model

[hassanhabib]

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]

ContactNumber, GuradianName, ClassTeacher, CurrentClass, RoleNumber,

[tajwal]

FatherName,Fathers email and mobile and gender of student.

[HatemGamal]

• the student's age when joined, religion, nationality, preferred language
• kin information
• Whether Handicap or not
• Notes to specify any disabilities or others

[devmanzur]

• 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]

• 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]

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]

• A flag to show if the student is active or not. Reasons for being not active: graduation, disciplinary action, death... etc.
• Make a relation to siblings students.

[tajwal]

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]

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]

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]

"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]

@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]

@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]

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

[ahmetsekmen]

@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]

@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]

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

[devmanzur]

we may add Admission Date, BatchId/SessionId, StudentImageUrl

[ShreyasJejurkar]

@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]

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]

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

[ShreyasJejurkar]

@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]

@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]

@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]

@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]

@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

[hassanhabib]

Keepin pics on azure server may can be legally problem for schools.
I would say let's continue with byte.

GDPR and other legal issues must be considered as well.

Definitely - we need to consider the right to be forgotten as one of our priorities when it comes to data - everyone should have the right to have their data completely erased from any system - thank you for bringing this up, great point!

[hassanhabib]

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 :-)

using an IdentityServer could be a better option, in case we are considering mobile apps

We definitely are considering mobile and desktop apps - we want to allow people to have offline mode where they can manage their local data - save notes and then sync when they get back online.

[a-urel]

My final draft:

    public class Student : IAuditable
    {
        /// <summary>
        /// PK
        /// </summary>
        public Guid StudentId { get; set; }

        /// <summary>
        /// Ref. to related IdentityUser
        /// nvarchar(450)
        /// </summary>
        public string UserId { get; set; }

        /// <summary>
        /// Legal status of the student
        /// eg. citizen, guest student, etc.
        /// </summary>
        //public string LegalStatus { get; set; }

        /// <summary>
        /// National Identity Number
        /// </summary>
        public string IdentityNumber { get; set; }


        /// <summary>
        /// Path or BlobName
        /// Some DTOs may have Image or byte[] type fields
        /// </summary>
        public string Picture { get; set; }

        public string FirstName { get; set; }
        public string MiddleName { get; set; }
        public string LastName { get; set; }

        public DateTimeOffset BirthDate { get; set; }

        /// <summary>
        /// NotSpecified, Female, Male, Other
        /// </summary>
        public Gender Gender { get; set; }

        /// <summary>
        /// Nationality, etc other demographic properties can be tags
        /// string or something like ICollection<Tag>...
        /// </summary>
        //public string Tags { get; set; }

        //[Timestamp]
        //public byte[] RowVersion { get; set; }

        public DateTimeOffset CreatedOn { get; set; }
        public string CreatedBy { get; set; }
        public DateTimeOffset ModifiedOn { get; set; }
        public string ModifiedBy { get; set; }
    }

[hassanhabib]

Thank you everyone for your contributions, this is amazing - with the influence of each and every one of you here's the final model that we should start with:

I think this is good enough, just few modifications:

public interface IAuditable {
	DateTimeOffset CreatedDate {get; set;}
	DateTimeOffset UpdatedDate {get; set;}
	Guid CreatedBy {get; set;}
	Guid UpdatedBy {get; set;}
}

public enum Gender {
	Female,
	Male,
	Other
}

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

Now, let's talk about assignments:

Setup

• Student Model Setup & Migrations with EF Core @a-urel Monday 12:00 AM - 11:59 PM PST [done]

Broker

• Students Broker Select By Id function @AgentEnder by Tuesday 12:00 AM - 11:59 PM PST [done]
• Students Broker Select All function @hiraldesai by Tuesday 12:00 AM - 11:59 PM PST [done]
• Students Broker Insert function @tajwal by Tuesday 12:00 AM - 11:59 PM PST [overdue - done @hassanhabib ]
• Students Broker Update function @hidegh by Tuesday 12:00 AM - 11:59 PM PST [overdue - done @hassanhabib ]
• Students Broker Delete function @MChorfa by Tuesday 12:00 AM - 11:59 PM PST [done]

Service

• Student Service Register function @Driedas by Wednesday 12:00 AM - 11:59 PM PST [overdue - done @viralpandya ]
• Student Service Modify function @Akinnagbe by Wednesday 12:00 AM - 11:59 PM PST [overdue - done @hassanhabib ]
• Student Service Retrieve by Id function @ahmetsekmen by Wednesday 12:00 AM - 11:59 PM PST [overdue - done @viralpandya ]
• Student Service Retrieve All function @HatemGamal by Wednesday 12:00 AM - 11:59 PM PST
• Student Service Delete by Id function @eriadhami by Wednesday 12:00 AM - 11:59 PM PST [done]

Controller

• Students Controller Post endpoint @MCCshreyas by Thursday 12:00 AM - 11:59 PM PST [overdue - done @viralpandya ]
• Students Controller Get by Id endpoint @antoniolinhart by Thursday 12:00 AM - 11:59 PM PST [override - done @viralpandya ]
• Students Controller Get All endpoint @levirgon by Thursday 12:00 AM - 11:59 PM PST
• Students Controller Update endpoint @devmanzur by Thursday 12:00 AM - 11:59 PM PST [overdue - done @hassanhabib ]
• Students Controller Delete endpoint @Mohamad-ali-8 by Thursday 12:00 AM - 11:59 PM PST [overdue - done @hassanhabib ]

Acceptance Tests

• Students Post Acceptance Test @tobidub by Friday 12:00 AM - 11:59 PM PST [done]
• Students Update Acceptance Test @gauriramesh by Friday 12:00 AM - 11:59 PM PST
• Students Get All Acceptance Test @AliCharper by Friday 12:00 AM - 11:59 PM PST

Few Rules:

1. If you have a service task, make sure you commit a failing test, then commit making it pass and so on until your feature is complete.
2. all code must be PR'ed - do not push straight to master without the team looking at your PR first.
3. Take a look at how some of the things are done here: https://github.com/hassanhabib/SchoolEM and let me know if you have any questions about the implementation details.
4. This WIP Coding Standard https://github.com/hassanhabib/CSharpCodingStandard will be our guidance, it is a work in progress but it should give you a pretty good idea bout how our code should look like from guidelines and standardization standpoint.
5. Do not start your task before the designated time and make sure you finalize it at the expected time so others can build on top of your work.

If you haven't had an assignment this week please make sure you engage in discussion for next week so I know you are active in this repo and you are going to be able to contribute to the project.

As soon as all these tasks are done, this issue will be closed.

Thank you all from the bottom of my heart for your contributions, if you are stuck or unsure how to do a task, reach out to me literally on any social media platform you like, LinkedIn, Facebook, Whatsapp, Twitter or even Instagram or just ping me on gitter and I will be sure to response within 2 - 4 hours max.

[devmanzur]

@hassanhabib will we allow all properties to be updated?

[hassanhabib]

@hassanhabib will we allow all properties to be updated?

@devmanzur yes, except of course the Id for obvious reasons

[devmanzur]

@hassanhabib will we allow all properties to be updated?

@devmanzur yes, except of course the Id for obvious reasons

Understood, Thank you.

[Mohamad-ali-8]

I will be working on the Delete Endpoint. Are we soft deleting or doing a physical delete?
Another not related question, are we going to use System versioned temporal tables?

using Microsoft.EntityFrameworkCore.Migrations;

namespace DHHS.RA.Data.Migrations
{
    public partial class MakeStudentTableTemporal : Migration
    {
        protected override void Up(MigrationBuilder migrationBuilder)
        {
            migrationBuilder.EnsureSchema("History");
            migrationBuilder.AddTemporalTableSupport("Student", "History");
        }

        protected override void Down(MigrationBuilder migrationBuilder)
        {
            migrationBuilder.EnsureSchema("History");
            migrationBuilder.TurnOffTemporalTableSupport("Student");
        }
    }
}

[Driedas]

I think that in only very exceptional circumstances it makes sense to delete something, if ever. How about a status property to both student and teacher? Something like that will be useful anyway when students leave the school for any reason. Not sure what the values would be, but maybe something like Registered/Active/Graduated/Cancelled

[a-urel]

I think that in only very exceptional circumstances it makes sense to delete something, if ever. How about a status property to both student and teacher? Something like that will be useful anyway when students leave the school for any reason. Not sure what the values would be, but maybe something like Registered/Active/Graduated/Cancelled

Exactly! States and transitions (state machine) are very useful in school information systems. Like:
Student: Candidate, Active, Graduated...
Semester: Draft (or Future), Active, Past...
Exam: Pending, Completed, Canceled...
...

[Driedas]

@a-urel cool, I like "Candidate" better than "Registered" :-)

[hassanhabib]

I will be working on the Delete Endpoint. Are we soft deleting or doing a physical delete?
Another not related question, are we going to use System versioned temporal tables?

using Microsoft.EntityFrameworkCore.Migrations;

namespace DHHS.RA.Data.Migrations
{
    public partial class MakeStudentTableTemporal : Migration
    {
        protected override void Up(MigrationBuilder migrationBuilder)
        {
            migrationBuilder.EnsureSchema("History");
            migrationBuilder.AddTemporalTableSupport("Student", "History");
        }

        protected override void Down(MigrationBuilder migrationBuilder)
        {
            migrationBuilder.EnsureSchema("History");
            migrationBuilder.TurnOffTemporalTableSupport("Student");
        }
    }
}

@Mohamad-ali-8 that's a good point - however, we want to expose a private endpoint to clean out the database from any records for acceptance testing - soft-deleting will be part of the processing services not these broker-neighboring services, the deleting here is actually for the system not for end-users to be able to delete records.
I think using temporal tables is a good idea, but this is not a part of the initial work we are doing here to get things started.

[hassanhabib]

I think that in only very exceptional circumstances it makes sense to delete something, if ever. How about a status property to both student and teacher? Something like that will be useful anyway when students leave the school for any reason. Not sure what the values would be, but maybe something like Registered/Active/Graduated/Cancelled

Exactly! States and transitions (state machine) are very useful in school information systems. Like:
Student: Candidate, Active, Graduated...
Semester: Draft (or Future), Active, Past...
Exam: Pending, Completed, Canceled...
...

@a-urel That's a good idea, let's propose initial statuses for Students, here's a draft:

public enum StudentStatus {
    Candidate,
    Enrolled,
    Graduated,
    Expelled,
    Deactivated
}

[a-urel]

Actually these states indicate the student's relationship with an academic program, which is usually the exact same thing with student's status. But a student may be registered to both CS and MBA, simultaneously or not. I'm not sure if it's too early to consider these details.

[hassanhabib]

Actually these states indicate the student's relationship with an academic program, which is usually the exact same thing with student's status. But a student may be registered to both CS and MBA, simultaneously or not. I'm not sure if it's too early to consider these details.

@a-urel we are not doing college just schools

[a-urel]

Actually these states indicate the student's relationship with an academic program, which is usually the exact same thing with student's status. But a student may be registered to both CS and MBA, simultaneously or not. I'm not sure if it's too early to consider these details.

@a-urel we are not doing college just schools

In some countries a school may include both primary and secondary school (or secondary and high school) stages.
To support this scenario we can either

• assume these schools as separate organizations (and create a new student record for secondary school) or
• provide many registrations (elementary, secondary,...) for single student.

First approach is simpler but the second one is more flexible.

These are the educational stages we should support:
https://en.wikipedia.org/wiki/Educational_stage

[a-urel]

@hassanhabib I think we can define OtripleS as K-12 software:
https://en.wikipedia.org/wiki/K%E2%80%9312

Maybe another discussion issue would be useful, such as scope or big picture.

[hidegh]

@a-urel exactly, I do miss 2 important things on this project: (was considering to create an ISSUE to address this, but...)

1. the scope - what it should include, features, goals, etc. - so the big picture
2. I personally find the talk about the model too early, architecture and technologies used should be the first, I know @hassanhabib pushed some sample code but:

• I personally do not favor the old MVC folder structure, as the Feature folder approach allows me to keep things belonging together beside each other.
• In the sample: https://github.com/hassanhabib/OtripleS/blob/master/OtripleS.Web.Api/Controllers/StudentsController.cs there's too much code in the controller, would consider MediatR with behaviors for logging, validation, exception handling, etc. - it's much cleaner design.

[hassanhabib]

@hidegh I created an issue for the architecture patterns and technology in this issue: #33

Closing this issue as we currently have full CRUD operations as foundational functionality for Student model.
(We should add student status with a different issue later)