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.

Sign up for GitHub

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

Jump to bottom

chore(#9582): update datasource bind to not return promise #9583

Merged
merged 2 commits into from
Oct 25, 2024
Merged

chore(#9582): update datasource bind to not return promise #9583

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
c7b8f27
Update cht-datasource.service.bind function to return function instea…
jkuester Oct 24, 2024
f7c2fdb
Use explicit returns
jkuester Oct 25, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
22 changes: 19 additions & 3 deletions webapp/src/ts/services/cht-datasource.service.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -97,9 +97,25 @@ export class CHTDatasourceService {
return user?.roles || this.userCtx?.roles;
}

async bind <T>(fn: (ctx: DataContext) => T): Promise<T> {
await this.isInitialized();
return this.dataContext.bind(fn);
/**
* Binds a cht-datasource function to the data context.
* (e.g. `const getPersonWithLineage = this.bind(Person.v1.getWithLineage);`)
* @param fn the function to bind. It should accept a data context as the parameter and return another function that
* results in a `Promise`.
* @returns a "context-aware" version of the function that is bound to the data context and ready to be used
*/
bind<R, F extends (arg?: unknown) => Promise<R>>(fn: (ctx: DataContext) => F):
(...p: Parameters<F>) => ReturnType<F> {
return (...p) => {
return new Promise((resolve, reject) => {
this.isInitialized().then(() => {
const contextualFn = this.dataContext.bind(fn);
contextualFn(...p)
.then(resolve)
.catch(reject);
});
}) as ReturnType<F>;
};
}

async get() {
Expand Down
13 changes: 8 additions & 5 deletions webapp/src/ts/services/transitions/create-user-for-contacts.transition.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,13 +11,18 @@ import { CHTDatasourceService } from '@mm-services/cht-datasource.service';
providedIn: 'root'
})
export class CreateUserForContactsTransition extends Transition {
private readonly getPlace: ReturnType<typeof Place.v1.get>;
private readonly getPerson: ReturnType<typeof Person.v1.get>;

constructor(
private chtDatasourceService: CHTDatasourceService,
chtDatasourceService: CHTDatasourceService,
private createUserForContactsService: CreateUserForContactsService,
private extractLineageService: ExtractLineageService,
private userContactService: UserContactService,
) {
super();
this.getPlace = chtDatasourceService.bind(Place.v1.get);
this.getPerson = chtDatasourceService.bind(Person.v1.get);
}

readonly name = 'create_user_for_contacts';
Expand Down Expand Up @@ -133,8 +138,7 @@ export class CreateUserForContactsTransition extends Transition {
return;
}

const getPlace = await this.chtDatasourceService.bind(Place.v1.get);
return getPlace(Qualifier.byUuid(doc.parent._id));
return this.getPlace(Qualifier.byUuid(doc.parent._id));
}

private async getNewContact(docs: Doc[], newContactId: string) {
Expand All @@ -143,8 +147,7 @@ export class CreateUserForContactsTransition extends Transition {
return newContact as Person.v1.Person;
}

const getPerson = await this.chtDatasourceService.bind(Person.v1.get);
const person = await getPerson(Qualifier.byUuid(newContactId));
const person = await this.getPerson(Qualifier.byUuid(newContactId));
if (!person) {
throw new Error(`The new contact could not be found [${newContactId}].`);
}
Expand Down
10 changes: 7 additions & 3 deletions webapp/src/ts/services/user-contact.service.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,19 +8,23 @@ import { CHTDatasourceService } from '@mm-services/cht-datasource.service';
providedIn: 'root'
})
export class UserContactService {
private readonly getPerson: ReturnType<typeof Person.v1.get>;
private readonly getPersonWithLineage: ReturnType<typeof Person.v1.getWithLineage>;
constructor(
private userSettingsService: UserSettingsService,
private chtDatasourceService: CHTDatasourceService,
chtDatasourceService: CHTDatasourceService,
) {
this.getPerson = chtDatasourceService.bind(Person.v1.get);
this.getPersonWithLineage = chtDatasourceService.bind(Person.v1.getWithLineage);
}

async get({ hydrateLineage = true } = {}) {
const user: any = await this.getUserSettings();
if (!user?.contact_id) {
return null;
}
const getPerson = await this.chtDatasourceService.bind(hydrateLineage ? Person.v1.getWithLineage : Person.v1.get);
return await getPerson(Qualifier.byUuid(user.contact_id));
const getPerson = hydrateLineage ? this.getPersonWithLineage : this.getPerson;
return getPerson(Qualifier.byUuid(user.contact_id));
}

private getUserSettings = async () => {
Expand Down
50 changes: 46 additions & 4 deletions webapp/tests/karma/ts/services/cht-datasource.service.spec.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -137,19 +137,24 @@ describe('CHTScriptApiService service', () => {
sessionService.isOnlineOnly.returns(isOnlineOnly);
const expectedDb = { hello: 'medic' };
dbService.get.resolves(expectedDb);
const innerFn = sinon.stub();
const innerFn = sinon.stub().resolves('hello world');
const outerFn = sinon
.stub()
.returns(innerFn);

const result = await service.bind(outerFn);
const returnedFn = service.bind(outerFn);

expect(outerFn.notCalled).to.be.true;
expect(innerFn.notCalled).to.be.true;

const result = await returnedFn('hello', 'world');

expect(result).to.equal('hello world');
expect(outerFn.calledOnce).to.be.true;
const [dataContext, ...other] = outerFn.args[0];
expect(other).to.be.empty;
expect(dataContext.bind).to.be.a('function');
expect(result).to.equal(innerFn);
expect(innerFn.notCalled).to.be.true;
expect(innerFn.calledOnceWithExactly('hello', 'world')).to.be.true;
expect(changesService.subscribe.calledOnce).to.be.true;
expect(changesService.subscribe.args[0][0].key).to.equal('cht-script-api-settings-changes');
expect(changesService.subscribe.args[0][0].filter).to.be.a('function');
Expand All @@ -161,6 +166,43 @@ describe('CHTScriptApiService service', () => {
expect(dbService.get.callCount).to.equal(isOnlineOnly ? 0 : 1);
});
});

it('surfaces exceptions thrown by bound function', async () => {
const settings = { hello: 'settings' } as const;
settingsService.get.resolves(settings);
const userCtx = { hello: 'world' };
sessionService.userCtx.returns(userCtx);
sessionService.isOnlineOnly.returns(true);
const expectedDb = { hello: 'medic' };
dbService.get.resolves(expectedDb);
const expectedError = new Error('hello world');
const innerFn = sinon.stub().rejects(expectedError);
const outerFn = sinon
.stub()
.returns(innerFn);

const returnedFn = service.bind(outerFn);

expect(outerFn.notCalled).to.be.true;
expect(innerFn.notCalled).to.be.true;

await expect(returnedFn()).to.be.rejectedWith(expectedError);

expect(outerFn.calledOnce).to.be.true;
const [dataContext, ...other] = outerFn.args[0];
expect(other).to.be.empty;
expect(dataContext.bind).to.be.a('function');
expect(innerFn.calledOnceWithExactly()).to.be.true;
expect(changesService.subscribe.calledOnce).to.be.true;
expect(changesService.subscribe.args[0][0].key).to.equal('cht-script-api-settings-changes');
expect(changesService.subscribe.args[0][0].filter).to.be.a('function');
expect(changesService.subscribe.args[0][0].callback).to.be.a('function');
expect(sessionService.userCtx.calledOnceWithExactly()).to.be.true;
expect(settingsService.get.calledOnceWithExactly()).to.be.true;
expect(http.get.calledOnceWithExactly('/extension-libs', { responseType: 'json' })).to.be.true;
expect(sessionService.isOnlineOnly.calledOnceWithExactly(userCtx)).to.be.true;
expect(dbService.get.notCalled).to.be.true;
});
});

describe('v1.hasPermissions()', () => {
Expand Down
57 changes: 35 additions & 22 deletions webapp/tests/karma/ts/services/transitions/create-user-for-contacts.transition.spec.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -79,8 +79,8 @@ describe('Create User for Contacts Transition', () => {
chtDatasourceService = {
bind: sinon.stub()
};
chtDatasourceService.bind.withArgs(Person.v1.get).resolves(getPerson);
chtDatasourceService.bind.withArgs(Place.v1.get).resolves(getPlace);
chtDatasourceService.bind.withArgs(Person.v1.get).returns(getPerson);
chtDatasourceService.bind.withArgs(Place.v1.get).returns(getPlace);
createUserForContactsService = {
isBeingReplaced: sinon.stub(),
setReplaced: sinon.stub(),
Expand All @@ -103,10 +103,8 @@ describe('Create User for Contacts Transition', () => {
});

afterEach(() => {
if (chtDatasourceService.bind.notCalled) {
expect(getPerson.notCalled).to.be.true;
expect(getPlace.notCalled).to.be.true;
}
expect(chtDatasourceService.bind.args).to.deep.equal([[Place.v1.get], [Person.v1.get]]);
sinon.restore();
});

describe('init', () => {
Expand All @@ -116,7 +114,10 @@ describe('Create User for Contacts Transition', () => {
consoleWarn = sinon.stub(console, 'warn');
});

afterEach(() => sinon.restore());
afterEach(() => {
expect(getPerson.notCalled).to.be.true;
expect(getPlace.notCalled).to.be.true;
});

it('returns true when replace forms have been configured', () => {
const settings = { create_user_for_contacts: { replace_forms: ['replace_user'] } };
Expand Down Expand Up @@ -145,6 +146,11 @@ describe('Create User for Contacts Transition', () => {
});

describe('filter', () => {
afterEach(() => {
expect(getPerson.notCalled).to.be.true;
expect(getPlace.notCalled).to.be.true;
});

[
[{ type: 'data_record' }],
[{ type: 'person' }, { type: 'user-settings' }, { type: 'data_record' }],
Expand Down Expand Up @@ -181,7 +187,8 @@ describe('Create User for Contacts Transition', () => {

expect(docs).to.be.empty;
expect(userContactService.get.callCount).to.equal(0);
expect(chtDatasourceService.bind.notCalled).to.be.true;
expect(getPerson.notCalled).to.be.true;
expect(getPlace.notCalled).to.be.true;
expect(createUserForContactsService.setReplaced.callCount).to.equal(0);
expect(createUserForContactsService.isBeingReplaced.callCount).to.equal(0);
});
Expand All @@ -193,7 +200,8 @@ describe('Create User for Contacts Transition', () => {

expect(docs).to.deep.equal([REPLACE_USER_DOC]);
expect(userContactService.get.callCount).to.equal(1);
expect(chtDatasourceService.bind.notCalled).to.be.true;
expect(getPerson.notCalled).to.be.true;
expect(getPlace.notCalled).to.be.true;
expect(createUserForContactsService.setReplaced.callCount).to.equal(0);
expect(createUserForContactsService.isBeingReplaced.callCount).to.equal(0);
});
Expand All @@ -208,7 +216,8 @@ describe('Create User for Contacts Transition', () => {

expect(docs).to.deep.equal([submittedDocs[0], submittedDocs[2], submittedDocs[3]]);
expect(userContactService.get.callCount).to.equal(1);
expect(chtDatasourceService.bind.notCalled).to.be.true;
expect(getPerson.notCalled).to.be.true;
expect(getPlace.notCalled).to.be.true;
expect(createUserForContactsService.setReplaced.callCount).to.equal(0);
expect(createUserForContactsService.isBeingReplaced.callCount).to.equal(1);
expect(createUserForContactsService.isBeingReplaced.args[0]).to.deep.equal([ORIGINAL_CONTACT]);
Expand All @@ -232,7 +241,6 @@ describe('Create User for Contacts Transition', () => {
}
}]);
expect(userContactService.get.callCount).to.equal(1);
expect(chtDatasourceService.bind.args).to.deep.equal([[Person.v1.get], [Place.v1.get]]);
expect(getPerson.calledOnceWithExactly(Qualifier.byUuid(NEW_CONTACT._id))).to.be.true;
expect(getPlace.calledOnceWithExactly(Qualifier.byUuid(parentPlace._id))).to.be.true;
expect(createUserForContactsService.setReplaced.callCount).to.equal(1);
Expand All @@ -256,7 +264,7 @@ describe('Create User for Contacts Transition', () => {
}
}]);
expect(userContactService.get.callCount).to.equal(1);
expect(chtDatasourceService.bind.args).to.deep.equal([[Place.v1.get]]);
expect(getPerson.notCalled).to.be.true;
expect(getPlace.calledOnceWithExactly(Qualifier.byUuid(parentPlace._id))).to.be.true;
expect(createUserForContactsService.setReplaced.callCount).to.equal(1);
expect(createUserForContactsService.setReplaced.args[0]).to.deep.equal([originalUser, NEW_CONTACT]);
Expand Down Expand Up @@ -311,10 +319,13 @@ describe('Create User for Contacts Transition', () => {
expect(createUserForContactsService.getReplacedBy.args).to.deep.equal([[originalUser], [originalUser]]);
// User replaced again
expect(userContactService.get.callCount).to.equal(1);
expect(chtDatasourceService.bind.args).to.deep.equal([[Place.v1.get]]);
expect(getPerson.notCalled).to.be.true;
expect(getPlace.calledOnceWithExactly(Qualifier.byUuid(parentPlace._id))).to.be.true;
expect(createUserForContactsService.setReplaced.callCount).to.equal(1);
expect(createUserForContactsService.setReplaced.args[0]).to.deep.equal([originalUser, secondNewContact]);
// Hack to keep the afterEach assertion happy since we called resetHistory
chtDatasourceService.bind(Place.v1.get);
chtDatasourceService.bind(Person.v1.get);
});

it('does not assign new contact as primary contact when original contact was not primary', async () => {
Expand All @@ -329,7 +340,6 @@ describe('Create User for Contacts Transition', () => {
expect(docs).to.deep.equal([REPLACE_USER_DOC, originalUser]);
expect(parentPlace.contact).to.deep.equal({ _id: 'different-contact', });
expect(userContactService.get.callCount).to.equal(1);
expect(chtDatasourceService.bind.args).to.deep.equal([[Person.v1.get], [Place.v1.get]]);
expect(getPerson.calledOnceWithExactly(Qualifier.byUuid(NEW_CONTACT._id))).to.be.true;
expect(getPlace.calledOnceWithExactly(Qualifier.byUuid(parentPlace._id))).to.be.true;
expect(createUserForContactsService.setReplaced.callCount).to.equal(1);
Expand All @@ -347,7 +357,6 @@ describe('Create User for Contacts Transition', () => {

expect(docs).to.deep.equal([REPLACE_USER_DOC, originalUser]);
expect(userContactService.get.callCount).to.equal(1);
expect(chtDatasourceService.bind.args).to.deep.equal([[Person.v1.get], [Place.v1.get]]);
expect(getPerson.calledOnceWithExactly(Qualifier.byUuid(NEW_CONTACT._id))).to.be.true;
expect(getPlace.calledOnceWithExactly(Qualifier.byUuid(PARENT_PLACE._id))).to.be.true;
expect(createUserForContactsService.setReplaced.callCount).to.equal(1);
Expand All @@ -369,8 +378,8 @@ describe('Create User for Contacts Transition', () => {

expect(docs).to.deep.equal([REPLACE_USER_DOC, originalUser]);
expect(userContactService.get.callCount).to.equal(1);
expect(chtDatasourceService.bind.args).to.deep.equal([[Person.v1.get]]);
expect(getPerson.calledOnceWithExactly(Qualifier.byUuid(newContact._id))).to.be.true;
expect(getPlace.notCalled).to.be.true;
expect(createUserForContactsService.setReplaced.callCount).to.equal(1);
expect(createUserForContactsService.setReplaced.args[0]).to.deep.equal([originalUser, newContact]);
expect(createUserForContactsService.isBeingReplaced.callCount).to.equal(2);
Expand All @@ -392,7 +401,8 @@ describe('Create User for Contacts Transition', () => {
}

expect(userContactService.get.callCount).to.equal(1);
expect(chtDatasourceService.bind.notCalled).to.be.true;
expect(getPerson.notCalled).to.be.true;
expect(getPlace.notCalled).to.be.true;
expect(createUserForContactsService.setReplaced.callCount).to.equal(0);
expect(createUserForContactsService.isBeingReplaced.callCount).to.equal(2);
});
Expand All @@ -412,7 +422,8 @@ describe('Create User for Contacts Transition', () => {
}

expect(userContactService.get.callCount).to.equal(1);
expect(chtDatasourceService.bind.notCalled).to.be.true;
expect(getPerson.notCalled).to.be.true;
expect(getPlace.notCalled).to.be.true;
expect(createUserForContactsService.setReplaced.callCount).to.equal(0);
expect(createUserForContactsService.isBeingReplaced.callCount).to.equal(1);
});
Expand All @@ -430,8 +441,8 @@ describe('Create User for Contacts Transition', () => {
}

expect(userContactService.get.callCount).to.equal(1);
expect(chtDatasourceService.bind.args).to.deep.equal([[Person.v1.get]]);
expect(getPerson.calledOnceWithExactly(Qualifier.byUuid(NEW_CONTACT._id))).to.be.true;
expect(getPlace.notCalled).to.be.true;
expect(createUserForContactsService.setReplaced.callCount).to.equal(0);
expect(createUserForContactsService.isBeingReplaced.callCount).to.equal(2);
});
Expand All @@ -448,8 +459,8 @@ describe('Create User for Contacts Transition', () => {
}

expect(userContactService.get.callCount).to.equal(1);
expect(chtDatasourceService.bind.args).to.deep.equal([[Person.v1.get]]);
expect(getPerson.calledOnceWithExactly(Qualifier.byUuid(NEW_CONTACT._id))).to.be.true;
expect(getPlace.notCalled).to.be.true;
expect(createUserForContactsService.setReplaced.callCount).to.equal(0);
expect(createUserForContactsService.isBeingReplaced.callCount).to.equal(2);
});
Expand All @@ -468,7 +479,8 @@ describe('Create User for Contacts Transition', () => {
}

expect(userContactService.get.callCount).to.equal(1);
expect(chtDatasourceService.bind.notCalled).to.be.true;
expect(getPerson.notCalled).to.be.true;
expect(getPlace.notCalled).to.be.true;
expect(createUserForContactsService.setReplaced.callCount).to.equal(0);
expect(createUserForContactsService.isBeingReplaced.callCount).to.equal(1);
});
Expand All @@ -477,7 +489,8 @@ describe('Create User for Contacts Transition', () => {
describe(`when the reports submitted do not include a replace user report, but the user is replaced`, () => {
afterEach(() => {
// Functions from the user replace flow should not be called
expect(chtDatasourceService.bind.notCalled).to.be.true;
expect(getPerson.notCalled).to.be.true;
expect(getPlace.notCalled).to.be.true;
expect(createUserForContactsService.setReplaced.callCount).to.equal(0);
});

Expand Down
Loading