Skip to content

Commit

Permalink
Merge pull request #2078 from cardstack/cs-7821-contained-fields-shou…
Browse files Browse the repository at this point in the history 
…ld-always-be-included-in-search-doc

Do not exclude unused contained computed fields from search doc
  • Loading branch information
@lukemelia
lukemelia authored Jan 23, 2025
2 parents af16d7f + 8f0e0dc commit 563dac1
Show file tree
Hide file tree
Showing 5 changed files with 51 additions and 14 deletions.
24 changes: 14 additions & 10 deletions packages/base/card-api.gts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1699,7 +1699,10 @@ export class BaseDef {
}
return Object.fromEntries(
Object.entries(
getFields(value, { includeComputeds: true, usedFieldsOnly: true }),
getFields(value, {
includeComputeds: true,
usedLinksToFieldsOnly: true,
}),
).map(([fieldName, field]) => {
let rawValue = peekAtField(value, fieldName);
if (field?.fieldType === 'linksToMany') {
Expand Down Expand Up @@ -1967,7 +1970,7 @@ export function subscribeToChanges(
changeSubscribers.add(subscriber);

let fields = getFields(fieldOrCard, {
usedFieldsOnly: true,
usedLinksToFieldsOnly: true,
includeComputeds: false,
});
Object.keys(fields).forEach((fieldName) => {
Expand Down Expand Up @@ -2010,7 +2013,7 @@ export function unsubscribeFromChanges(
changeSubscribers.delete(subscriber);

let fields = getFields(fieldOrCard, {
usedFieldsOnly: true,
usedLinksToFieldsOnly: true,
includeComputeds: false,
});
Object.keys(fields).forEach((fieldName) => {
Expand Down Expand Up @@ -2308,7 +2311,7 @@ function serializeCardResource(
let { includeUnrenderedFields: remove, ...fieldOpts } = opts ?? {};
let { id: removedIdField, ...fields } = getFields(model, {
...fieldOpts,
usedFieldsOnly: !opts?.includeUnrenderedFields,
usedLinksToFieldsOnly: !opts?.includeUnrenderedFields,
});
let fieldResources = Object.entries(fields)
.filter(([_fieldName, field]) =>
Expand Down Expand Up @@ -2830,7 +2833,7 @@ export async function recompute(
Object.keys(
getFields(model, {
includeComputeds: true,
usedFieldsOnly: !opts?.recomputeAllFields,
usedLinksToFieldsOnly: !opts?.recomputeAllFields,
}),
),
);
Expand Down Expand Up @@ -2936,15 +2939,15 @@ export async function getIfReady<T extends BaseDef, K extends keyof T>(

export function getFields(
card: typeof BaseDef,
opts?: { usedFieldsOnly?: boolean; includeComputeds?: boolean },
opts?: { usedLinksToFieldsOnly?: boolean; includeComputeds?: boolean },
): { [fieldName: string]: Field<BaseDefConstructor> };
export function getFields<T extends BaseDef>(
card: T,
opts?: { usedFieldsOnly?: boolean; includeComputeds?: boolean },
opts?: { usedLinksToFieldsOnly?: boolean; includeComputeds?: boolean },
): { [P in keyof T]?: Field<BaseDefConstructor> };
export function getFields(
cardInstanceOrClass: BaseDef | typeof BaseDef,
opts?: { usedFieldsOnly?: boolean; includeComputeds?: boolean },
opts?: { usedLinksToFieldsOnly?: boolean; includeComputeds?: boolean },
): { [fieldName: string]: Field<BaseDefConstructor> } {
let obj: object | null;
let usedFields: string[] = [];
Expand Down Expand Up @@ -2979,9 +2982,10 @@ export function getFields(
!['contains', 'containsMany'].includes(maybeField.fieldType)
) {
if (
opts?.usedFieldsOnly &&
opts?.usedLinksToFieldsOnly &&
!usedFields.includes(maybeFieldName) &&
!maybeField.isUsed
!maybeField.isUsed &&
!['contains', 'containsMany'].includes(maybeField.fieldType)
) {
return [];
}
Expand Down
1 change: 1 addition & 0 deletions packages/host/app/lib/current-run.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -505,6 +505,7 @@ export class CurrentRun {
identityContext,
},
);
await api.flushLogs();
isolatedHtml = unwrap(
sanitizeHTML(
await this.#renderCard({
Expand Down
20 changes: 18 additions & 2 deletions packages/host/tests/integration/realm-indexing-and-querying-test.gts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -654,6 +654,7 @@ module(`Integration | realm indexing and querying`, function (hooks) {
module: `./person`,
name: 'Person',
},
thumbnailURL: null,
demo: {},
},
meta: {
Expand Down Expand Up @@ -683,6 +684,7 @@ module(`Integration | realm indexing and querying`, function (hooks) {
moduleHref: `${testRealmURL}person`,
realmName: 'Unnamed Workspace',
ref: `${testRealmURL}person/Person`,
thumbnailURL: null,
title: 'Person Card',
});
} else {
Expand Down Expand Up @@ -1780,7 +1782,7 @@ module(`Integration | realm indexing and querying`, function (hooks) {
}
});

test(`search doc includes 'contains' and used 'linksTo' fields`, async function (assert) {
test(`search doc includes 'contains' and used 'linksTo' fields, including contained computed fields`, async function (assert) {
let { realm } = await setupIntegrationTestRealm({
loader,
contents: {
Expand Down Expand Up @@ -1832,9 +1834,11 @@ module(`Integration | realm indexing and querying`, function (hooks) {
email: '[email protected]',
posts: 100,
title: 'Hassan Abdel-Rahman',
description: 'Person',
fullName: 'Hassan Abdel-Rahman',
_cardType: 'Person',
},
`search doc does not include fullName field`,
`search doc includes fullName field`,
);
});

Expand Down Expand Up @@ -1906,7 +1910,9 @@ module(`Integration | realm indexing and querying`, function (hooks) {
{
_cardType: 'Post',
author: {
description: 'Person',
fullName: ' ',
title: ' ',
},
id: `${testRealmURL}Post/1`,
title: '50 Ways to Leave Your Laptop',
Expand Down Expand Up @@ -2031,8 +2037,11 @@ module(`Integration | realm indexing and querying`, function (hooks) {
_cardType: 'Catalog Entry',
id: `${testRealmURL}CatalogEntry/booking`,
demo: {
description: null,
hosts: null,
posts: null,
sponsors: null,
thumbnailURL: null,
title: null,
venue: null,
},
Expand All @@ -2041,6 +2050,7 @@ module(`Integration | realm indexing and querying`, function (hooks) {
moduleHref: 'http://localhost:4202/test/booking',
realmName: 'Unnamed Workspace',
ref: 'http://localhost:4202/test/booking/Booking',
thumbnailURL: null,
title: 'Booking',
});
// we should be able to perform a structured clone of the search doc (this
Expand Down Expand Up @@ -2414,6 +2424,7 @@ module(`Integration | realm indexing and querying`, function (hooks) {
isField: false,
moduleHref: `${testModuleRealm}pet-person`,
realmName: 'Unnamed Workspace',
thumbnailURL: null,
},
relationships: {
'demo.friend': { links: { self: null } },
Expand Down Expand Up @@ -2518,6 +2529,7 @@ module(`Integration | realm indexing and querying`, function (hooks) {
ref: `${testModuleRealm}pet-person/PetPerson`,
demo: {
firstName: 'Hassan',
description: 'A person with pets',
pets: [
{
id: `${testRealmURL}Pet/mango`,
Expand All @@ -2536,11 +2548,14 @@ module(`Integration | realm indexing and querying`, function (hooks) {
thumbnailURL: null,
},
],
thumbnailURL: null,
title: 'Hassan Pet Person',
friend: null,
},
isField: false,
moduleHref: `${testModuleRealm}pet-person`,
realmName: 'Unnamed Workspace',
thumbnailURL: null,
});
} else {
assert.ok(
Expand Down Expand Up @@ -3402,6 +3417,7 @@ module(`Integration | realm indexing and querying`, function (hooks) {
{
id: vanGoghID,
firstName: 'Van Gogh',
title: 'Van Gogh',
friends: [
{
id: hassanID,
Expand Down
16 changes: 16 additions & 0 deletions packages/host/tests/integration/realm-test.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -223,12 +223,14 @@ module('Integration | realm', function (hooks) {
type: 'card',
id: `${testRealmURL}dir/owner`,
attributes: {
description: 'Person',
email: null,
posts: null,
thumbnailURL: null,
firstName: 'Hassan',
lastName: 'Abdel-Rahman',
title: 'Hassan Abdel-Rahman',
fullName: 'Hassan Abdel-Rahman',
},
meta: {
adoptsFrom: {
Expand Down Expand Up @@ -339,11 +341,13 @@ module('Integration | realm', function (hooks) {
type: 'card',
id: `http://localhost:4202/test/hassan`,
attributes: {
description: 'Person',
email: null,
posts: null,
thumbnailURL: null,
firstName: 'Hassan',
lastName: 'Abdel-Rahman',
fullName: 'Hassan Abdel-Rahman',
title: 'Hassan Abdel-Rahman',
},
meta: {
Expand Down Expand Up @@ -592,12 +596,14 @@ module('Integration | realm', function (hooks) {
type: 'card',
id: `${testRealmURL}dir/owner`,
attributes: {
description: 'Person',
email: null,
posts: null,
thumbnailURL: null,
firstName: 'Hassan',
lastName: 'Abdel-Rahman',
title: 'Hassan Abdel-Rahman',
fullName: 'Hassan Abdel-Rahman',
},
meta: {
adoptsFrom: {
Expand Down Expand Up @@ -951,8 +957,10 @@ module('Integration | realm', function (hooks) {
endTime: '2023-02-19T02:00:00.000Z',
hosts: [
{
description: 'Person',
firstName: 'Hassan',
lastName: null,
fullName: 'Hassan ',
title: 'Hassan ',
email: null,
posts: null,
Expand Down Expand Up @@ -1157,11 +1165,13 @@ module('Integration | realm', function (hooks) {
id: `${testRealmURL}dir/friend`,
links: { self: `${testRealmURL}dir/friend` },
attributes: {
description: 'Person',
email: null,
posts: null,
thumbnailURL: null,
firstName: 'Hassan',
lastName: 'Abdel-Rahman',
fullName: 'Hassan Abdel-Rahman',
title: 'Hassan Abdel-Rahman',
},
meta: {
Expand Down Expand Up @@ -2011,7 +2021,9 @@ module('Integration | realm', function (hooks) {
attributes: {
firstName: 'Mariko',
lastName: 'Abdel-Rahman',
fullName: 'Mariko Abdel-Rahman',
title: 'Mariko Abdel-Rahman',
description: 'Person',
email: null,
posts: null,
thumbnailURL: null,
Expand Down Expand Up @@ -2980,7 +2992,9 @@ module('Integration | realm', function (hooks) {
attributes: {
firstName: 'Mariko',
lastName: 'Abdel-Rahman',
fullName: 'Mariko Abdel-Rahman',
title: 'Mariko Abdel-Rahman',
description: 'Person',
email: null,
posts: null,
thumbnailURL: null,
Expand Down Expand Up @@ -3047,12 +3061,14 @@ module('Integration | realm', function (hooks) {
type: 'card',
id: `http://localhost:4202/test/hassan`,
attributes: {
description: 'Person',
email: null,
posts: null,
thumbnailURL: null,
firstName: 'Hassan',
lastName: 'Abdel-Rahman',
title: 'Hassan Abdel-Rahman',
fullName: 'Hassan Abdel-Rahman',
},
meta: {
adoptsFrom: {
Expand Down
4 changes: 2 additions & 2 deletions packages/runtime-common/helpers/ai.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -203,7 +203,7 @@ function generateJsonSchemaForContainsFields(
};

const { id: _removedIdField, ...fields } = cardApi.getFields(def, {
usedFieldsOnly: false,
usedLinksToFieldsOnly: false,
});

for (let [fieldName, field] of Object.entries(fields)) {
Expand Down Expand Up @@ -301,7 +301,7 @@ function generateRelationshipFieldsInfo(
fieldName?: string,
) {
const { id: _removedIdField, ...fields } = cardApi.getFields(def, {
usedFieldsOnly: false,
usedLinksToFieldsOnly: false,
});
for (let [fName, fValue] of Object.entries(fields)) {
let flatFieldName = fieldName ? `${fieldName}.${fName}` : fName;
Expand Down

0 comments on commit 563dac1

Please sign in to comment.