Addresses Review Comments

Author: gbdubs

db/sqldb/portfolio.go

@@ -10,6 +10,8 @@ import (
 	"github.com/jackc/pgx/v5/pgtype"
 )
 
+// Curious why this query uses array aggregation in its nested queries?
+// See https://github.com/RMI-PACTA/app/pull/91#discussion_r1437712435
 func portfolioQueryStanza(where string) string {
 	return fmt.Sprintf(`
 	WITH selected_portfolio_ids AS (
@@ -232,7 +234,6 @@ func rowToPortfolio(row rowScanner) (*pacta.Portfolio, error) {
 	if err := checkSizesEquivalent("initiatives", len(initiativesIDs), len(initiativesAddedByIDs), len(initiativesCreatedAts)); err != nil {
 		return nil, err
 	}
-	seenInitiativeIDs := make(map[pacta.InitiativeID]bool)
 	for i := range initiativesIDs {
 		if !initiativesIDs[i].Valid && !initiativesCreatedAts[i].Valid {
 			continue // skip nulls
@@ -243,11 +244,6 @@ func rowToPortfolio(row rowScanner) (*pacta.Portfolio, error) {
 		if !initiativesCreatedAts[i].Valid {
 			return nil, fmt.Errorf("initiative createdAt must be non-null")
 		}
-		id := pacta.InitiativeID(initiativesIDs[i].String)
-		if seenInitiativeIDs[id] {
-			continue // Skip duplicates, see MULTI_LEFT_JOIN note.
-		}
-		seenInitiativeIDs[id] = true
 		var addedBy *pacta.User
 		if initiativesAddedByIDs[i].Valid {
 			addedBy = &pacta.User{ID: pacta.UserID(initiativesAddedByIDs[i].String)}

db/sqldb/portfolio_group.go

@@ -138,7 +138,7 @@ func rowToPortfolioGroup(row rowScanner) (*pacta.PortfolioGroup, error) {
 	if err != nil {
 		return nil, fmt.Errorf("scanning into portfolio_group row: %w", err)
 	}
-	if err := checkSizesEquivalent("portfolio group memberhsip", len(mid), len(mca)); err != nil {
+	if err := checkSizesEquivalent("portfolio group membership", len(mid), len(mca)); err != nil {
 		return nil, err
 	}
 	for i := range mid {

frontend/composables/useURLParams.ts

@@ -39,16 +39,33 @@ export const useURLParams = () => {
     void router.replace(qs)
   }
 
+  const fromQueryReactive = (key: string): WritableComputedRef<string | undefined> => {
+    return computed({
+      get: () => getVal(router.currentRoute.value.query, key),
+      set: (val: string | undefined) => { setVal(key, val) },
+    })
+  }
+
+  const fromQueryReactiveWithDefault = (key: string, def: string): WritableComputedRef<string> => {
+    const fqr = fromQueryReactive(key)
+    return computed({
+      get: () => fqr.value ?? def,
+      set: (val: string) => {
+        if (val === def) {
+          fqr.value = undefined
+        } else {
+          fqr.value = val
+        }
+      },
+    })
+  }
+
   return {
     fromQuery: (key: string): string | undefined => {
       return getVal(route.query, key)
     },
-    fromQueryReactive: (key: string): WritableComputedRef<string | undefined> => {
-      return computed({
-        get: () => getVal(router.currentRoute.value.query, key),
-        set: (val: string | undefined) => { setVal(key, val) },
-      })
-    },
+    fromQueryReactive,
+    fromQueryReactiveWithDefault,
     fromParams: (key: string): string | undefined => {
       return getVal(route.params, key)
     },

frontend/pages/my-data.vue

@@ -1,16 +1,16 @@
 <script setup lang="ts">
 const prefix = 'pages/my-data'
 
-const { fromQueryReactive } = useURLParams()
+const { fromQueryReactiveWithDefault } = useURLParams()
 const { t } = useI18n()
 const pactaClient = usePACTA()
 const { loading: { withLoading } } = useModal()
 
 const tt = (s: string) => t(`${prefix}.${s}`)
 
-const selectedPortfolioIdsQP = fromQueryReactive('pids')
-const selectedPortfolioGroupIdsQP = fromQueryReactive('pgids')
-const tabQP = fromQueryReactive('tab')
+const selectedPortfolioIdsQP = fromQueryReactiveWithDefault('pids', '')
+const selectedPortfolioGroupIdsQP = fromQueryReactiveWithDefault('pgids', '')
+const tabQP = fromQueryReactiveWithDefault('tab', 'p')
 
 const [
   { data: incompleteUploadsData, refresh: refreshIncompleteUploadsApi },
@@ -46,26 +46,12 @@ const refreshAll = async () => {
 }
 
 const selectedPortfolioIds = computed<string[]>({
-  get: () => (selectedPortfolioIdsQP.value ?? '').split(','),
-  set: (v: string[]) => {
-    if (v.length === 0) {
-      selectedPortfolioIdsQP.value = undefined
-    } else {
-      v.sort()
-      selectedPortfolioIdsQP.value = v.join(',')
-    }
-  },
+  get: () => selectedPortfolioIdsQP.value.split(','),
+  set: (v: string[]) => { selectedPortfolioIdsQP.value = v.join(',') },
 })
 const selectedPortfolioGroupIds = computed<string[]>({
-  get: () => (selectedPortfolioGroupIdsQP.value ?? '').split(','),
-  set: (v: string[]) => {
-    if (v.length === 0) {
-      selectedPortfolioGroupIdsQP.value = undefined
-    } else {
-      v.sort()
-      selectedPortfolioGroupIdsQP.value = v.join(',')
-    }
-  },
+  get: () => selectedPortfolioGroupIdsQP.value.split(','),
+  set: (v: string[]) => { selectedPortfolioGroupIdsQP.value = v.join(',') },
 })
 interface TabToIndexMap {
   iu: number
@@ -91,7 +77,7 @@ const tabToIndexMap = computed(() => {
 })
 const activeIndex = computed<number>({
   get: () => {
-    const tab = (tabQP.value ?? 'p') as keyof TabToIndexMap
+    const tab = tabQP.value as keyof TabToIndexMap
     const result = tabToIndexMap.value[tab]
     if (result === undefined) {
       console.error(`Unknown tab ${tab}`)