code ref should serialize to relative (#2154)

* serialize to relative url only when id does not exist

set null only when card.id does not exist

* only use maybeRelativeURL for codeRefField

* fix bug

* client code can set maybeRelativeURL to absolute

* fix test

* attempt to string thru opts for every field

* only try to serialize with primitive. Otherwise, too much influence on nested serialization

* useAbsoluteURL: true optional paramter -- clearer usage of SerializedOpts

* add  test

packages/base/card-api.gts

@@ -696,13 +696,18 @@ class Contains<CardT extends FieldDefConstructor> implements Field<CardT, any> {
   serialize(
     value: InstanceType<CardT>,
     doc: JSONAPISingleResourceDocument,
+    _visited: Set<string>,
+    opts?: SerializeOpts,
   ): JSONAPIResource {
-    let serialized: JSONAPISingleResourceDocument['data'] & {
-      meta: Record<string, any>;
-    } = callSerializeHook(this.card, value, doc);
     if (primitive in this.card) {
+      let serialized: JSONAPISingleResourceDocument['data'] & {
+        meta: Record<string, any>;
+      } = callSerializeHook(this.card, value, doc, undefined, opts);
       return { attributes: { [this.name]: serialized } };
     } else {
+      let serialized: JSONAPISingleResourceDocument['data'] & {
+        meta: Record<string, any>;
+      } = callSerializeHook(this.card, value, doc);
       let resource: JSONAPIResource = {
         attributes: {
           [this.name]: serialized?.attributes,
@@ -2299,8 +2304,9 @@ async function getDeserializedValue<CardT extends BaseDefConstructor>({
 export interface SerializeOpts {
   includeComputeds?: boolean;
   includeUnrenderedFields?: boolean;
-  maybeRelativeURL?: ((possibleURL: string) => string) | null; // setting this to null will force all URL's to be absolute
+  useAbsoluteURL?: boolean;
   omitFields?: [typeof BaseDef];
+  maybeRelativeURL?: (possibleURL: string) => string;
 }
 
 function serializeCardResource(
@@ -2309,7 +2315,10 @@ function serializeCardResource(
   opts?: SerializeOpts,
   visited: Set<string> = new Set(),
 ): LooseCardResource {
-  let adoptsFrom = identifyCard(model.constructor, opts?.maybeRelativeURL);
+  let adoptsFrom = identifyCard(
+    model.constructor,
+    opts?.useAbsoluteURL ? undefined : opts?.maybeRelativeURL,
+  );
   if (!adoptsFrom) {
     throw new Error(`bug: could not identify card: ${model.constructor.name}`);
   }
@@ -2346,28 +2355,22 @@ export function serializeCard(
   let modelRelativeTo = model[relativeTo];
   let data = serializeCardResource(model, doc, {
     ...opts,
-    // if opts.maybeRelativeURL is null that is our indication
-    // that the caller wants all the URL's to be absolute
-    ...(opts?.maybeRelativeURL !== null
-      ? {
-          maybeRelativeURL(possibleURL: string) {
-            let url = maybeURL(possibleURL, modelRelativeTo);
-            if (!url) {
-              throw new Error(
-                `could not determine url from '${maybeRelativeURL}' relative to ${modelRelativeTo}`,
-              );
-            }
-            if (!modelRelativeTo) {
-              return url.href;
-            }
-            const realmURLString = getCardMeta(model, 'realmURL');
-            const realmURL = realmURLString
-              ? new URL(realmURLString)
-              : undefined;
-            return maybeRelativeURL(url, modelRelativeTo, realmURL);
-          },
+    ...{
+      maybeRelativeURL(possibleURL: string) {
+        let url = maybeURL(possibleURL, modelRelativeTo);
+        if (!url) {
+          throw new Error(
+            `could not determine url from '${maybeRelativeURL}' relative to ${modelRelativeTo}`,
+          );
         }
-      : {}),
+        if (!modelRelativeTo) {
+          return url.href;
+        }
+        const realmURLString = getCardMeta(model, 'realmURL');
+        const realmURL = realmURLString ? new URL(realmURLString) : undefined;
+        return maybeRelativeURL(url, modelRelativeTo, realmURL);
+      },
+    },
   });
   merge(doc, { data });
   if (!isSingleCardDocument(doc)) {
@@ -3150,7 +3153,9 @@ export class Box<T> {
 type ElementType<T> = T extends (infer V)[] ? V : never;
 
 function makeRelativeURL(maybeURL: string, opts?: SerializeOpts): string {
-  return opts?.maybeRelativeURL ? opts.maybeRelativeURL(maybeURL) : maybeURL;
+  return opts?.maybeRelativeURL && !opts?.useAbsoluteURL
+    ? opts.maybeRelativeURL(maybeURL)
+    : maybeURL;
 }
 
 declare module 'ember-provide-consume-context/context-registry' {

packages/base/code-ref.gts

@@ -39,7 +39,7 @@ export default class CodeRefField extends FieldDef {
   ) {
     return {
       ...codeRef,
-      ...(opts?.maybeRelativeURL
+      ...(opts?.maybeRelativeURL && !opts?.useAbsoluteURL
         ? { module: opts.maybeRelativeURL(codeRef.module) }
         : {}),
     };

packages/host/app/commands/add-skills-to-room.ts

@@ -27,7 +27,7 @@ export default class AddSkillsToRoomCommand extends HostBaseCommand<
     let roomSkillEventIds = await matrixService.addSkillCardsToRoomHistory(
       skills,
       roomId,
-      { includeComputeds: true, maybeRelativeURL: null },
+      { includeComputeds: true, useAbsoluteURL: true },
     );
     await matrixService.updateStateEvent(
       roomId,

packages/host/app/services/card-service.ts

@@ -213,8 +213,9 @@ export default class CardService extends Service {
         // for a brand new card that has no id yet, we don't know what we are
         // relativeTo because its up to the realm server to assign us an ID, so
         // URL's should be absolute
-        maybeRelativeURL: null, // forces URL's to be absolute.
+        useAbsoluteURL: true,
       });
+
       // send doc over the wire with absolute URL's. The realm server will convert
       // to relative URL's as it serializes the cards
       let realmURL = await this.getRealmURL(card);
@@ -423,7 +424,7 @@ export default class CardService extends Service {
   async copyCard(source: CardDef, destinationRealm: URL): Promise<CardDef> {
     let api = await this.getAPI();
     let serialized = await this.serializeCard(source, {
-      maybeRelativeURL: null, // forces URL's to be absolute.
+      useAbsoluteURL: true,
     });
     delete serialized.data.id;
     let json = await this.saveCardDocument(serialized, destinationRealm);

packages/host/app/services/matrix-service.ts

@@ -590,7 +590,7 @@ export default class MatrixService extends Service {
     cards: CardDef[],
     roomId: string,
     cardHashes: Map<string, string> = this.cardHashes,
-    opts: CardAPI.SerializeOpts = { maybeRelativeURL: null },
+    opts: CardAPI.SerializeOpts = { useAbsoluteURL: true },
   ): Promise<string[]> {
     if (!cards.length) {
       return [];

packages/host/tests/integration/components/card-catalog-test.gts

@@ -120,7 +120,7 @@ module('Integration | card-catalog', function (hooks) {
           description: 'Spec for PublishingPacket',
           specType: 'card',
           ref: {
-            module: `../publishing-packet`,
+            module: `${testRealmURL}publishing-packet`,
             name: 'PublishingPacket',
           },
         }),

packages/host/tests/integration/realm-indexing-and-querying-test.gts

@@ -706,6 +706,109 @@ module(`Integration | realm indexing and querying`, function (hooks) {
     }
   });
 
+  test('absolute urls will be serialised into relative into relative code-ref fields', async function (assert) {
+    class Person extends CardDef {
+      @field firstName = contains(StringField);
+    }
+
+    let { realm, adapter } = await setupIntegrationTestRealm({
+      loader,
+      contents: {
+        'person.gts': { Person },
+        'person-spec.json': {
+          data: {
+            attributes: {
+              title: 'Person Card',
+              description: 'Spec for Person card',
+              specType: 'card',
+              ref: {
+                module: `${testRealmURL}person`,
+                name: 'Person',
+              },
+            },
+            meta: {
+              adoptsFrom: {
+                module: 'https://cardstack.com/base/spec',
+                name: 'Spec',
+              },
+            },
+          },
+        },
+      },
+    });
+    let indexer = realm.realmIndexQueryEngine;
+    let entry = await indexer.cardDocument(
+      new URL(`${testRealmURL}person-spec`),
+    );
+    if (entry?.type === 'doc') {
+      assert.deepEqual(entry.doc.data, {
+        id: `${testRealmURL}person-spec`,
+        type: 'card',
+        links: {
+          self: `${testRealmURL}person-spec`,
+        },
+        attributes: {
+          title: 'Person Card',
+          description: 'Spec for Person card',
+          moduleHref: `${testRealmURL}person`,
+          name: null,
+          readMe: null,
+          specType: 'card',
+          isCard: true,
+          isField: false,
+          thumbnailURL: null,
+          ref: {
+            module: `./person`,
+            name: 'Person',
+          },
+          containedExamples: [],
+        },
+        relationships: {
+          linkedExamples: {
+            links: {
+              self: null,
+            },
+          },
+        },
+        meta: {
+          adoptsFrom: {
+            module: 'https://cardstack.com/base/spec',
+            name: 'Spec',
+          },
+          lastModified: adapter.lastModifiedMap.get(
+            `${testRealmURL}person-spec.json`,
+          ),
+          resourceCreatedAt: adapter.resourceCreatedAtMap.get(
+            `${testRealmURL}person-spec.json`,
+          ),
+          realmInfo: testRealmInfo,
+          realmURL: testRealmURL,
+        },
+      });
+      let instance = await indexer.instance(
+        new URL(`${testRealmURL}person-spec`),
+      );
+      assert.deepEqual(instance?.searchDoc, {
+        _cardType: 'Spec',
+        description: 'Spec for Person card',
+        id: `${testRealmURL}person-spec`,
+        specType: 'card',
+        moduleHref: `${testRealmURL}person`,
+        ref: `${testRealmURL}person/Person`,
+        title: 'Person Card',
+        linkedExamples: null,
+        containedExamples: null,
+        isCard: true,
+        isField: false,
+      });
+    } else {
+      assert.ok(
+        false,
+        `search entry was an error: ${entry?.error.errorDetail.message}`,
+      );
+    }
+  });
+
   test('can recover from rendering a card that has a template error', async function (assert) {
     {
       class Person extends CardDef {