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

Refactor getNotesOfChild to EntityMapper #221

Closed
codingfabi opened this issue Jan 31, 2019 · 4 comments
Closed

Refactor getNotesOfChild to EntityMapper #221

codingfabi opened this issue Jan 31, 2019 · 4 comments
Assignees
Labels
Type: Refactoring / Technical Enh. Technical Enhancement without changes for the user

Comments

@codingfabi
Copy link
Contributor

codingfabi commented Jan 31, 2019

I wanted to implement a function to get all Health Checks for a certain loaded child.
I wanted to do it familiar to the way we load notes of a child into the ChildDetailComponent.
But i dont know how to implement a index in pouchDB that i can query.

We should refactor and rethink the following:

 getNotesOfChild(childId: string): Observable<Note[]> {
    const promise = this.db.query('notes_index/by_child', {key: childId, include_docs: true})
      .then(loadedEntities => {
        return loadedEntities.rows.map(loadedRecord => {
          const entity = new Note('');
          entity.load(loadedRecord.doc);
          return entity;
        });
      });

    return Observable.fromPromise(promise);
  }

In my opinion, this belongs into the Entity Mapper and should be made generic.

@codingfabi codingfabi added Type: Refactoring / Technical Enh. Technical Enhancement without changes for the user discuss labels Jan 31, 2019
@liwde
Copy link
Contributor

liwde commented Jan 31, 2019

https://pouchdb.com/guides/queries.html

These indices affect two separate use cases:

  1. Statistics
  2. Traversing relations between entities

I'm fine with statistics not being part of the EntityMapper, but traversing relations is a general problem: Health Checks, Notes, ChildSchoolRelationships, ...

Shouln't the Entity to be queried by provide the map-function?

export class Note extends Entity {
  static ENTITY_TYPE = 'Note';
  static dbViews : any = [/* All Design Docs relevant for the Entity */]
}

The EntityMapper may then be queried like this:

this._entityMapper.getRelational<Child, Note>(childId);

At the point, this method is called, the EntityMapper should make sure that for both Child and Note the dbViews have been added to PouchDB and use the right index to execute the Query and construct the Entities.

(There needs to be put some more thought into having metadata for the dbViews so that the EntityMapper can pick the correct one, but this seems like a solution to me)

@liwde
Copy link
Contributor

liwde commented Jan 31, 2019

@TheSlimvReal How did you implement this for the ChildSchoolRelations?

@sleidig
Copy link
Member

sleidig commented Feb 3, 2019

The idea behind our architecture of Entity and EntityMapper is the Data Mapper pattern, i.e. to keep the Entity class/classes free of any database related concerns and simply focus on the "business logic" in them.

Therefore, I would try to put that stuff into EntityMapper (originally, the idea also was also to implement subclasses of EntityMapper for different Entity type, e.g. a ChildMapper. But so far there was no real use for it).

The index for Notes is triggered in the constructor of ChildrenService: https://github.com/NGO-DB/ndb-core/blob/67aad641c397fe130f8d58dec1975a90fc3ffc8d/src/app/children/children.service.ts#L163-L177
For now, you could add another similar function there. But the whole ChildrenService class is becoming a mess of duplicate code. We definitely need a generic system to replace all that (related issue: #108)

@sleidig sleidig added this to the [v2.7] milestone Jun 18, 2019
@sleidig sleidig modified the milestones: [v2.8], NEXT Nov 30, 2019
@Schottkyc137 Schottkyc137 mentioned this issue Dec 11, 2019
9 tasks
@sleidig
Copy link
Member

sleidig commented Feb 20, 2020

This is actually an older duplicate of issues #261(EntitySchema should transform Entity relations) and #262 (EntitySchema should automatically create indices for related/linked entities)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Type: Refactoring / Technical Enh. Technical Enhancement without changes for the user
Projects
None yet
Development

No branches or pull requests

3 participants