-
Notifications
You must be signed in to change notification settings - Fork 22
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
Adding schools and show/edit details - basic functionality #204
Conversation
# Conflicts: # src/app/app.routing.ts
# 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
…etails-basic # Conflicts: # src/app/schools/school-detail/school-detail.component.html # src/app/schools/schools-list/schools-list.component.html
0edf159
to
e5efeab
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Works fine and can be merged. But we should realy start implementing the proper Child School Relation.
But so far, for making the feature available in a accaptable time, this pr is good.
Thanks for the review. I'll fix the CI build fail and merge it in the next days |
} | ||
|
||
getSchools(): Observable<School[]> { | ||
return Observable.fromPromise(this.entityMapper.loadType<School>(School)); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We are currently updating to Angular 7, where this is no longer allowed. We should instead use
import {from} from 'rxjs';
//...
return from(this.entityMapper.loadType<School>(School));
We changed this everywhere else in our branch already
For the implementation of the ChildSchoolRelation please check out the branch child_school_relation_impl. |
changed new rxjs syntax from Observable.of()
Code Climate has analyzed commit 8a580d3 and detected 10 issues on this pull request. Here's the issue category breakdown:
The test coverage on the diff in this pull request is 48.9% (80% is the threshold). This pull request will bring the total coverage in the repository to 76.6% (-0.8% change). View more on Code Climate. |
see issue #42
This PR is covering part of the changes of PR #156 in order to make the finished, more basic changes available to users sooner.
Visible/Frontend Changes
Architectural/Backend Changes
School
entity has more attributes for various details