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

[WIP] school detail and school list feature #156

Closed
wants to merge 20 commits into from

Conversation

SuppenGeist
Copy link

@SuppenGeist SuppenGeist commented Jul 26, 2018

see #42
Implement the school details view

@codingfabi codingfabi changed the title [WIP] School detail feature [WIP] School detail and school list feature Aug 9, 2018
@SuppenGeist SuppenGeist changed the title [WIP] School detail and school list feature School detail and school list feature Sep 20, 2018
@Lorsbyjm Lorsbyjm changed the title School detail and school list feature [WIP] school detail and school list feature Sep 27, 2018
Lorsbyjm and others added 13 commits September 27, 2018 20:30
# Conflicts:
#	npm-shrinkwrap.json
#	src/app/app.routing.ts
#	src/app/children/children-list/children-list.component.ts
#	src/app/schools/school-detail/school-detail.component.html
#	src/app/schools/school-detail/school-detail.component.ts
#	src/app/schools/schools-list/schools-list.component.css
#	src/app/schools/schools-list/schools-list.component.html
#	src/app/schools/schools-list/schools-list.component.ts
#	src/app/schools/schoolsShared/school.ts
#	src/app/schools/schoolsShared/schools.services.ts
# Conflicts:
#	src/app/schools/school-detail/school-detail.component.html
…tching students that are visiting this school.
…el where each student saves the school it attends
… class of an entity around. Replaces the construct { new(id: string): T; }
# Conflicts:
#	src/app/schools/school-detail/school-detail.component.html
#	src/app/schools/schools-list/schools-list.component.html
@sleidig
Copy link
Member

sleidig commented Dec 12, 2018

What's the status on this? Is this anywhere near completion?
The social workers have some new schools to add now in December/January. If this still takes a while, maybe I will implement a simplified version for immediate use.

@codingfabi
Copy link
Contributor

I think it is almost completed... I hope we can finish it tomorrow, so stay tuned.

@Lorsbyjm
Copy link
Contributor

Please also look at sentry. There was an error raised in your files

@TheSlimvReal
Copy link
Collaborator

What's the status on this? Is this anywhere near completion?
The social workers have some new schools to add now in December/January. If this still takes a while, maybe I will implement a simplified version for immediate use.

The SchoolDetail screen is finished, but now I also wanted to bring the new ChildSchoolRelations into play. This takes me a little bit more time but I think it would be nice to finally have this implemented.

@@ -0,0 +1,7 @@
import {Entity} from './entity';

export abstract class EntityRelation extends Entity {
Copy link
Member

Choose a reason for hiding this comment

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

In which way is EntityRelation different from Entity? Does it really require a separate class for this?

import {Observable} from '../../../../node_modules/rxjs/Rx';

@Injectable()
export class SchoolsServices {
Copy link
Member

Choose a reason for hiding this comment

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

Naming convention is to use "service" in singular (i.e. file name "schools.service.ts", class name "SchoolsService")

@sleidig
Copy link
Member

sleidig commented Dec 14, 2018

I do understand why this is taking long - quite a complex challenge with the relationship modelling between Child and School.

Have you looked into solutions that keep the relationship loading out of the entity classes (i.e. move/remove Child.getSchools() and School.getStudents())? Maybe having that code in the ChildrenService and SchoolService - or in the EntityMapper?
To move those Entity classes around, save them to the database - and first and foremost to keep them simple - I like the idea that Entity subtype classes only contain business logic. The idea was initially to let an EntityMapper class take care of all the loading, saving and also the resolution of relationships.

@sleidig
Copy link
Member

sleidig commented Dec 14, 2018

I am taking out the completed features of the school details (everything up to a278a7d ) into a separate PR: #204. So that we can make this available to users within the next days already.

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.

5 participants