Skip to content

Commit

Permalink
fix: feedback applied
Browse files Browse the repository at this point in the history
  • Loading branch information
Agnes Lin committed Nov 22, 2019
1 parent 8a7f78d commit af3410e
Show file tree
Hide file tree
Showing 4 changed files with 109 additions and 88 deletions.
154 changes: 78 additions & 76 deletions ...ry-tests/src/crud/relations/acceptance/has-many-inclusion-resolver.relation.acceptance.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -187,84 +187,86 @@ export function hasManyInclusionResolverAcceptance(
expect(toJSON(result)).to.deepEqual(toJSON(expected));
});

describe('checks save() and replaceById()', () => {
/* istanbul ignore next */
(features.hasRevisionToken ? it.skip : it)(
'skips the tests for Cloudant as it needs the token',
async () => {
it('returns inclusions after running save() operation', async () => {
// this shows save() works well with func ensurePersistable and ObjectId
const thor = await customerRepo.create({name: 'Thor'});
const odin = await customerRepo.create({name: 'Odin'});

const thorOrder = await orderRepo.create({
customerId: thor.id,
description: 'Pizza',
});

const pizza = await orderRepo.findById(thorOrder.id);
pizza.customerId = odin.id;

await orderRepo.save(pizza);
const odinPizza = await orderRepo.findById(thorOrder.id);

const result = await customerRepo.findById(odin.id, {
include: [{relation: 'orders'}],
});
const expected = {
...odin,
parentId: features.emptyValue,
orders: [
{
...odinPizza,
isShipped: features.emptyValue,
// eslint-disable-next-line @typescript-eslint/camelcase
shipment_id: features.emptyValue,
},
],
};
expect(toJSON(result)).to.containEql(toJSON(expected));
});

it('returns inclusions after running replaceById() operation', async () => {
// this shows replaceById() works well with func ensurePersistable and ObjectId
const thor = await customerRepo.create({name: 'Thor'});
const odin = await customerRepo.create({name: 'Odin'});

const thorOrder = await orderRepo.create({
customerId: thor.id,
description: 'Pizza',
});

const pizza = await orderRepo.findById(thorOrder.id);
pizza.customerId = odin.id;

await orderRepo.replaceById(thorOrder.id, pizza);
const odinPizza = await orderRepo.findById(thorOrder.id);

const result = await customerRepo.find({
include: [{relation: 'orders'}],
});
const expected = [
{...thor, parentId: features.emptyValue},
skipIf(
!features.hasRevisionToken,
it,
'returns inclusions after running save() operation',
async () => {
// this shows save() works well with func ensurePersistable and ObjectId
const thor = await customerRepo.create({name: 'Thor'});
const odin = await customerRepo.create({name: 'Odin'});

const thorOrder = await orderRepo.create({
customerId: thor.id,
description: 'Pizza',
});

const pizza = await orderRepo.findById(thorOrder.id);
pizza.customerId = odin.id;

await orderRepo.save(pizza);
const odinPizza = await orderRepo.findById(thorOrder.id);

const result = await customerRepo.findById(odin.id, {
include: [{relation: 'orders'}],
});
const expected = {
...odin,
parentId: features.emptyValue,
orders: [
{
...odinPizza,
isShipped: features.emptyValue,
// eslint-disable-next-line @typescript-eslint/camelcase
shipment_id: features.emptyValue,
},
],
};
expect(toJSON(result)).to.containEql(toJSON(expected));
},
);

skipIf(
!features.hasRevisionToken,
it,
'returns inclusions after running replaceById() operation',
async () => {
// this shows replaceById() works well with func ensurePersistable and ObjectId
const thor = await customerRepo.create({name: 'Thor'});
const odin = await customerRepo.create({name: 'Odin'});

const thorOrder = await orderRepo.create({
customerId: thor.id,
description: 'Pizza',
});

const pizza = await orderRepo.findById(thorOrder.id.toString());
pizza.customerId = odin.id;

await orderRepo.replaceById(thorOrder.id, pizza);
const odinPizza = await orderRepo.findById(thorOrder.id);

const result = await customerRepo.find({
include: [{relation: 'orders'}],
});
const expected = [
{...thor, parentId: features.emptyValue},
{
...odin,
parentId: features.emptyValue,
orders: [
{
...odin,
parentId: features.emptyValue,
orders: [
{
...odinPizza,
isShipped: features.emptyValue,
// eslint-disable-next-line @typescript-eslint/camelcase
shipment_id: features.emptyValue,
},
],
...odinPizza,
isShipped: features.emptyValue,
// eslint-disable-next-line @typescript-eslint/camelcase
shipment_id: features.emptyValue,
},
];
expect(toJSON(result)).to.deepEqual(toJSON(expected));
});
},
);
});
],
},
];
expect(toJSON(result)).to.deepEqual(toJSON(expected));
},
);

it('throws when navigational properties are present when updating model instance with save()', async () => {
// save() calls replaceById so the result will be the same for replaceById
Expand Down
17 changes: 17 additions & 0 deletions packages/repository-tests/src/crud/relations/acceptance/has-many.relation.acceptance.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -247,6 +247,23 @@ export function hasManyRelationAcceptance(
).to.be.rejectedWith(/Navigational properties are not allowed.*"orders"/);
});

it('throws when the instance contains navigational property when operates delete()', async () => {
const customer = await customerRepo.create({name: 'customer'});

await orderRepo.create({
description: 'pizza',
customerId: customer.id,
});

const found = await customerRepo.findById(customer.id, {
include: [{relation: 'orders'}],
});

await expect(customerRepo.delete(found)).to.be.rejectedWith(
'Navigational properties are not allowed in model data (model "Customer" property "orders")',
);
});

context('when targeting the source model', () => {
it('gets the parent entity through the child entity', async () => {
const parent = await customerRepo.create({name: 'parent customer'});
Expand Down
3 changes: 3 additions & 0 deletions packages/repository-tests/src/crud/relations/helpers.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,9 @@ export function givenBoundCrudRepositories(
Order.definition.properties.id.type = features.idType;
Address.definition.properties.id.type = features.idType;
Customer.definition.properties.id.type = features.idType;
Customer.definition.properties.id.mongodb = {
dataType: 'ObjectID',
};
Shipment.definition.properties.id.type = features.idType;
// when running the test suite on MongoDB, we don't really need to setup
// this config for mongo connector to pass the test.
Expand Down
23 changes: 11 additions & 12 deletions packages/repository/src/repositories/legacy-juggler-bridge.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -420,7 +420,8 @@ export class DefaultCrudRepository<
return this.updateById(entity.getId(), entity, options);
}

delete(entity: T, options?: Options): Promise<void> {
async delete(entity: T, options?: Options): Promise<void> {
this.ensurePersistable(entity);
return this.deleteById(entity.getId(), options);
}

Expand Down Expand Up @@ -456,7 +457,7 @@ export class DefaultCrudRepository<
options?: Options,
): Promise<void> {
try {
const payload = this.ensurePersistable(data, {replacement: true});
const payload = this.ensurePersistable(data);
await ensurePromise(this.modelClass.replaceById(id, payload, options));
} catch (err) {
if (err.statusCode === 404) {
Expand Down Expand Up @@ -536,15 +537,13 @@ export class DefaultCrudRepository<

/** Converts an entity object to a JSON object to check if it contains navigational property.
* Throws an error if `entity` contains navigational property.
* Also removed id property for replaceById operation (mongo use case).
*
* @param entity
* @param options when attributw replacement is set to true, delete the id property to operate
* replaceById method.
* @param options
*/
protected ensurePersistable<R extends T>(
entity: R | DataObject<R>,
options: {replacement?: boolean; idProperty?: string} = {},
options: {},
): legacy.ModelData<legacy.PersistedModel> {
// FIXME(bajtos) Ideally, we should call toJSON() to convert R to data object
// Unfortunately that breaks replaceById for MongoDB connector, where we
Expand All @@ -569,12 +568,12 @@ export class DefaultCrudRepository<
);
}
}
if (options.replacement === true) {
const idProperty = this.entityClass.getIdProperties()[0];
if (idProperty) {
delete data[idProperty];
}
}
// if (options.replacement === true) {
// const idProperty = this.entityClass.getIdProperties()[0];
// if (!data[idProperty] === undefined) {
// throw new Error(`id property should not be included in ${data}`);
// }
// }
return data;
}

Expand Down

0 comments on commit af3410e

Please sign in to comment.