Tweak the way _attributeName works again (#2269)

graphile · Dec 4, 2024 · edca766 · edca766
2 parents 2e85f34 + bee1db4
commit edca766
Show file tree

Hide file tree

Showing 5 changed files with 43 additions and 39 deletions.
diff --git a/.changeset/five-dogs-rest.md b/.changeset/five-dogs-rest.md
@@ -0,0 +1,8 @@
+---
+"graphile-build-pg": patch
+"postgraphile": patch
+---
+
+Hotfix to inflection changes in beta.34 - fixes behavior of
+`orderByAttributeEnum` and removes V4 override of `_joinAttributeNames` which
+shouldn't be necessary (and seems to do more harm than good).
diff --git a/graphile-build/graphile-build-pg/src/plugins/PgAttributesPlugin.ts b/graphile-build/graphile-build-pg/src/plugins/PgAttributesPlugin.ts
@@ -271,12 +271,12 @@ export const PgAttributesPlugin: GraphileConfig.Plugin = {
 
   inflection: {
     add: {
-      _attributeName(options, { attributeName, codec }) {
+      _attributeName(options, { attributeName, codec, skipRowId }) {
         const attribute = codec.attributes[attributeName];
         const name = attribute.extensions?.tags?.name || attributeName;
         // Avoid conflict with 'id' field used for Relay.
         const nonconflictName =
-          name === "id" && !codec.isAnonymous ? "row_id" : name;
+          !skipRowId && name === "id" && !codec.isAnonymous ? "row_id" : name;
         return this.coerceToGraphQLName(nonconflictName);
       },
 

diff --git a/postgraphile/postgraphile/src/plugins/PgV4InflectionPlugin.ts b/postgraphile/postgraphile/src/plugins/PgV4InflectionPlugin.ts
@@ -128,13 +128,20 @@ export const PgV4InflectionPlugin: GraphileConfig.Plugin = {
         }
       },
 
-      _joinAttributeNames(_previous, options, codec, names) {
-        return (
-          names
-            // V5 uses this._attributeName here; V4 needs the old style .attribute
-            .map((attributeName) => this.attribute({ attributeName, codec }))
-            .join("-and-")
-        );
+      // V4 had a bug where even though `id` was renamed to `rowId`, we'd still
+      // use `ID_ASC`/`ID_DESC` when generating ordering - so we have to enable
+      // skipRowId.
+      orderByAttributeEnum(
+        _previous,
+        options,
+        { codec, attributeName, variant },
+      ) {
+        const fieldName = this._attributeName({
+          attributeName,
+          codec,
+          skipRowId: true,
+        });
+        return this.constantCase(`${fieldName}-${variant}`);
       },
     },
   },

diff --git a/postgraphile/postgraphile/src/presets/relay.ts b/postgraphile/postgraphile/src/presets/relay.ts
@@ -37,7 +37,7 @@ export const PgRelayPlugin: GraphileConfig.Plugin = {
       },
       _attributeName(previous, options, details) {
         const name = previous!(details);
-        if (name === "id") return "row_id";
+        if (!details.skipRowId && name === "id") return "row_id";
         return name;
       },
     },

diff --git a/postgraphile/postgraphile/src/presets/v4.ts b/postgraphile/postgraphile/src/presets/v4.ts
@@ -144,17 +144,24 @@ const makeV4Plugin = (options: V4Options): GraphileConfig.Plugin => {
     inflection: {
       ignoreReplaceIfNotExists: ["nodeIdFieldName"],
       replace: {
-        // Don't rename 'id' to 'rowId' (we might undo this in `attribute()` later)
-        _attributeName(previous, options, details) {
-          const { codec, attributeName } = details;
-          const attribute = codec.attributes[attributeName];
-          const baseName = attribute.extensions?.tags?.name || attributeName;
-          const name = previous!(details);
-          if (baseName === "id" && name === "row_id" && !codec.isAnonymous) {
-            return "id";
-          }
-          return name;
-        },
+        ...(classicIds
+          ? null
+          : {
+              // Undo rename of 'id' to 'rowId'
+              _attributeName(previous, options, details) {
+                const name = previous!(details);
+                if (!details.skipRowId && name === "row_id") {
+                  const { codec, attributeName } = details;
+                  const attribute = codec.attributes[attributeName];
+                  const baseName =
+                    attribute.extensions?.tags?.name || attributeName;
+                  if (baseName === "id" && !codec.isAnonymous) {
+                    return "id";
+                  }
+                }
+                return name;
+              },
+            }),
         ...(classicIds ||
         options.skipPlugins?.some((p) => p.name === "NodePlugin")
           ? null
@@ -165,24 +172,6 @@ const makeV4Plugin = (options: V4Options): GraphileConfig.Plugin => {
                 return "nodeId";
               },
             }),
-        ...(classicIds
-          ? {
-              // Do rename 'id' to 'rowId', but do it in `attribute()` not `_attributeName`
-              attribute(previous, options, details) {
-                const { codec, attributeName } = details;
-                const attribute = codec.attributes[attributeName];
-                const baseName =
-                  attribute.extensions?.tags?.name || attributeName;
-                const name = previous!(details);
-                if (baseName === "id" && name === "id" && !codec.isAnonymous) {
-                  return "rowId";
-                }
-                return name;
-              },
-            }
-          : {
-              // No action required
-            }),
       },
     },
     schema: {