From ed3bc8dd27e8664fc43ca5b529c53348944634e1 Mon Sep 17 00:00:00 2001
From: Scott Wilson <scott@Scotts-MacBook-Pro-2.local>
Date: Thu, 12 Sep 2024 16:11:58 -0700
Subject: [PATCH 1/2] fix: apply project identity offset/limit separate from
 left joins

---
 .../identity-project/identity-project-dal.ts  | 42 +++++++++++++------
 1 file changed, 30 insertions(+), 12 deletions(-)

diff --git a/backend/src/services/identity-project/identity-project-dal.ts b/backend/src/services/identity-project/identity-project-dal.ts
index 63955fc73..3a73244a4 100644
--- a/backend/src/services/identity-project/identity-project-dal.ts
+++ b/backend/src/services/identity-project/identity-project-dal.ts
@@ -1,10 +1,11 @@
 import { Knex } from "knex";
 
 import { TDbClient } from "@app/db";
-import { TableName } from "@app/db/schemas";
+import { TableName, TIdentities } from "@app/db/schemas";
 import { DatabaseError } from "@app/lib/errors";
-import { ormify, sqlNestRelationships } from "@app/lib/knex";
-import { TListProjectIdentityDTO } from "@app/services/identity-project/identity-project-types";
+import { ormify, selectAllTableCols, sqlNestRelationships } from "@app/lib/knex";
+import { OrderByDirection } from "@app/lib/types";
+import { ProjectIdentityOrderBy, TListProjectIdentityDTO } from "@app/services/identity-project/identity-project-types";
 
 export type TIdentityProjectDALFactory = ReturnType<typeof identityProjectDALFactory>;
 
@@ -117,18 +118,38 @@ export const identityProjectDALFactory = (db: TDbClient) => {
     tx?: Knex
   ) => {
     try {
+      // TODO: scott - optimize, there's redundancy here with project membership and the below query
+      const fetchIdentitySubquery = (tx || db.replicaNode())(TableName.Identity)
+        .where((qb) => {
+          if (filter.search) {
+            void qb.whereILike(`${TableName.Identity}.name`, `%${filter.search}%`);
+          }
+        })
+        .join(
+          TableName.IdentityProjectMembership,
+          `${TableName.IdentityProjectMembership}.identityId`,
+          `${TableName.Identity}.id`
+        )
+        .where(`${TableName.IdentityProjectMembership}.projectId`, projectId)
+        .offset(filter.offset ?? 0)
+        .limit(filter.limit ?? 100)
+        .orderBy(
+          `${TableName.Identity}.${filter.orderBy ?? ProjectIdentityOrderBy.Name}`,
+          filter.orderDirection ?? OrderByDirection.ASC
+        )
+        .select(selectAllTableCols(TableName.Identity))
+        .as(TableName.Identity); // required for subqueries
+
       const query = (tx || db.replicaNode())(TableName.IdentityProjectMembership)
         .where(`${TableName.IdentityProjectMembership}.projectId`, projectId)
         .join(TableName.Project, `${TableName.IdentityProjectMembership}.projectId`, `${TableName.Project}.id`)
-        .join(TableName.Identity, `${TableName.IdentityProjectMembership}.identityId`, `${TableName.Identity}.id`)
+        .join<TIdentities, TIdentities>(fetchIdentitySubquery, (bd) => {
+          bd.on(`${TableName.IdentityProjectMembership}.identityId`, `${TableName.Identity}.id`);
+        })
         .where((qb) => {
           if (filter.identityId) {
             void qb.where("identityId", filter.identityId);
           }
-
-          if (filter.search) {
-            void qb.whereILike(`${TableName.Identity}.name`, `%${filter.search}%`);
-          }
         })
         .join(
           TableName.IdentityProjectMembershipRole,
@@ -166,10 +187,7 @@ export const identityProjectDALFactory = (db: TDbClient) => {
           db.ref("name").as("projectName").withSchema(TableName.Project)
         );
 
-      if (filter.limit) {
-        void query.offset(filter.offset ?? 0).limit(filter.limit);
-      }
-
+      // TODO: scott - joins seem to reorder identities so need to order again, for the sake of urgency will optimize at a later point
       if (filter.orderBy) {
         switch (filter.orderBy) {
           case "name":

From 93f27a7ee8e314c2aa133d254dd921c22f617404 Mon Sep 17 00:00:00 2001
From: Scott Wilson <scott@Scotts-MacBook-Pro-2.local>
Date: Thu, 12 Sep 2024 16:19:22 -0700
Subject: [PATCH 2/2] improvement: make limit conditional

---
 .../src/services/identity-project/identity-project-dal.ts   | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/backend/src/services/identity-project/identity-project-dal.ts b/backend/src/services/identity-project/identity-project-dal.ts
index 3a73244a4..3e7ca7946 100644
--- a/backend/src/services/identity-project/identity-project-dal.ts
+++ b/backend/src/services/identity-project/identity-project-dal.ts
@@ -131,8 +131,6 @@ export const identityProjectDALFactory = (db: TDbClient) => {
           `${TableName.Identity}.id`
         )
         .where(`${TableName.IdentityProjectMembership}.projectId`, projectId)
-        .offset(filter.offset ?? 0)
-        .limit(filter.limit ?? 100)
         .orderBy(
           `${TableName.Identity}.${filter.orderBy ?? ProjectIdentityOrderBy.Name}`,
           filter.orderDirection ?? OrderByDirection.ASC
@@ -140,6 +138,10 @@ export const identityProjectDALFactory = (db: TDbClient) => {
         .select(selectAllTableCols(TableName.Identity))
         .as(TableName.Identity); // required for subqueries
 
+      if (filter.limit) {
+        void fetchIdentitySubquery.offset(filter.offset ?? 0).limit(filter.limit);
+      }
+
       const query = (tx || db.replicaNode())(TableName.IdentityProjectMembership)
         .where(`${TableName.IdentityProjectMembership}.projectId`, projectId)
         .join(TableName.Project, `${TableName.IdentityProjectMembership}.projectId`, `${TableName.Project}.id`)