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

Preserve custom type of auto-generated id property #4270

Merged
merged 1 commit into from
Feb 18, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
6 changes: 3 additions & 3 deletions packages/repository-tests/src/crud/create-retrieve.suite.ts
Original file line number Diff line number Diff line change
Expand Up @@ -12,9 +12,9 @@ import {
property,
} from '@loopback/repository';
import {expect, toJSON} from '@loopback/testlab';
import {MixedIdType} from '../helpers.repository-tests';
import {
deleteAllModelsInDefaultDataSource,
MixedIdType,
withCrudCtx,
} from '../helpers.repository-tests';
import {
Expand Down Expand Up @@ -89,15 +89,15 @@ export function createRetrieveSuite(
const pens = await repo.create({name: 'Pens', categoryId: 1});
const pencils = await repo.create({name: 'Pencils', categoryId: 2});
const products = await findByForeignKeys(repo, 'categoryId', [1]);
expect(products).deepEqual([pens]);
expect(toJSON(products)).deepEqual(toJSON([pens]));
expect(products).to.not.containDeep(pencils);
});

it('retrieves instances of a model from their foreign key value', async () => {
const pens = await repo.create({name: 'Pens', categoryId: 1});
const pencils = await repo.create({name: 'Pencils', categoryId: 2});
const products = await findByForeignKeys(repo, 'categoryId', [1, 2]);
expect(products).deepEqual([pens, pencils]);
expect(toJSON(products)).deepEqual(toJSON([pens, pencils]));
});
});
}
Original file line number Diff line number Diff line change
Expand Up @@ -14,8 +14,8 @@ import {
import {expect, toJSON} from '@loopback/testlab';
import {
deleteAllModelsInDefaultDataSource,
withCrudCtx,
MixedIdType,
withCrudCtx,
} from '../helpers.repository-tests';
import {
CrudFeatures,
Expand Down Expand Up @@ -91,6 +91,7 @@ export function nestedModelsPropertiesSuite(
@property({
id: true,
generated: true,
useDefaultIdType: true,
})
id: MixedIdType;

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -91,7 +91,7 @@ export function belongsToRelationAcceptance(
description: 'Order that is shipped Tuesday morning',
});
const result = await orderRepo.shipment(order.id);
expect(result).to.deepEqual(shipment);
expect(toJSON(result)).to.deepEqual(toJSON(shipment));
});

it('returns undefined if the source instance does not have the foreign key', async () => {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -310,7 +310,7 @@ export function hasManyRelationAcceptance(
});
const childsParent = await getParentCustomer(child.id);
expect(_.pick(childsParent, ['id', 'name'])).to.eql(
_.pick(parent, ['id', 'name']),
toJSON(_.pick(parent, ['id', 'name'])),
);
});

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,8 @@ import {
withCrudCtx,
} from '../../../helpers.repository-tests';
import {
Address,
AddressRepository,
Customer,
CustomerRepository,
Order,
Expand All @@ -36,21 +38,26 @@ export function hasManyInclusionResolverAcceptance(
);
function suite() {
before(deleteAllModelsInDefaultDataSource);
let addressRepo: AddressRepository;
let customerRepo: CustomerRepository;
let orderRepo: OrderRepository;

before(
withCrudCtx(async function setupRepository(ctx: CrudTestContext) {
// this helper should create the inclusion resolvers and also
// register inclusion resolvers for us
({customerRepo, orderRepo} = givenBoundCrudRepositories(
({customerRepo, orderRepo, addressRepo} = givenBoundCrudRepositories(
ctx.dataSource,
repositoryClass,
features,
));
expect(customerRepo.orders.inclusionResolver).to.be.Function();

await ctx.dataSource.automigrate([Customer.name, Order.name]);
await ctx.dataSource.automigrate([
Customer.name,
Order.name,
Address.name,
]);
}),
);

Expand All @@ -63,15 +70,22 @@ export function hasManyInclusionResolverAcceptance(
const parent = await customerRepo.create({name: 'parent'});
const customer = await customerRepo.create({
name: 'customer',
parentId: parent.id,
//parentId: parent.id,
});
const address = await addressRepo.create({
street: '8200 Warden',
city: 'Markham',
province: 'On',
zipcode: '8200',
customerId: parent.id,
});
const order = await orderRepo.create({
description: 'an order',
customerId: parent.id,
});

const result = await customerRepo.find({
include: [{relation: 'orders'}, {relation: 'customers'}],
Copy link
Contributor

Choose a reason for hiding this comment

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

While I was fixing the CI failure, I think the flag might somehow break how we deal with ObjectId in MongoDB. Before we were able to traverse related instances with ObjectId. Not sure it has something to do with the type.

include: [{relation: 'orders'}, {relation: 'address'}],
});
const expected = [
{
Expand All @@ -84,16 +98,11 @@ export function hasManyInclusionResolverAcceptance(
shipmentInfo: features.emptyValue,
},
],
customers: [
{
...customer,
parentId: parent.id,
},
],
address: address,
},
{
...customer,
parentId: parent.id, // doesn't have any related models
parentId: features.emptyValue, //parent.id, // doesn't have any related models
},
];
expect(toJSON(result)).to.deepEqual(toJSON(expected));
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@ export class Address extends Entity {
@property({
id: true,
generated: true,
useDefaltIdType: true,
})
id: MixedIdType;
@property({
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@ export class Customer extends Entity {
@property({
id: true,
generated: true,
useDefaltIdType: true,
})
id: MixedIdType;

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@ export class Order extends Entity {
@property({
id: true,
generated: true,
useDefaultIdType: true,
})
id: MixedIdType;

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@ export class Shipment extends Entity {
@property({
id: true,
generated: true,
useDefaltIdType: true,
})
id: MixedIdType;

Expand Down
1 change: 1 addition & 0 deletions packages/repository-tests/src/crud/replace-by-id.suite.ts
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,7 @@ export function createSuiteForReplaceById(
type: features.idType,
id: true,
generated: true,
useDefaultIdType: true,
description: 'The unique identifier for a product',
})
id: MixedIdType;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -192,7 +192,12 @@ describe('model decorator', () => {
column: 'QTY',
},
});
expect(meta.id).to.eql({type: 'string', id: true, generated: true});
expect(meta.id).to.eql({
type: 'string',
id: true,
generated: true,
useDefaultIdType: false,
});
expect(meta.isShipped).to.eql({type: Boolean});
});

Expand Down
10 changes: 10 additions & 0 deletions packages/repository/src/model.ts
Original file line number Diff line number Diff line change
Expand Up @@ -106,6 +106,16 @@ export class ModelDefinition {
const definition = (definitionOrType as PropertyDefinition).type
? (definitionOrType as PropertyDefinition)
: {type: definitionOrType};

if (
frbuceta marked this conversation as resolved.
Show resolved Hide resolved
definition.id === true &&
definition.generated === true &&
definition.type !== undefined &&
definition.useDefaultIdType === undefined
) {
definition.useDefaultIdType = false;
}

this.properties[name] = definition;
return this;
}
Expand Down