From 3b6c6feac5a215a9e7a65387b9b576c5cd78a0b7 Mon Sep 17 00:00:00 2001
From: Ross Perry <ross.perry@Rosss-MacBook-Pro-2.local>
Date: Fri, 1 Nov 2024 16:05:01 -0600
Subject: [PATCH 1/6] move functionality to backend

move functionality to backend

dev - meeds pagination

dev - meeds pagination

added pagination

labels functional

naming

logging
---
 seed/migrations/0233_alter_goal_options.py    |  16 ++
 seed/models/goals.py                          |   3 +
 seed/serializers/goals.py                     |  15 +-
 .../portfolio_summary_controller.js           | 216 +++++-------------
 seed/static/seed/js/services/goal_service.js  |  41 ++++
 seed/tests/test_goals.py                      |  31 ++-
 seed/views/v3/goals.py                        | 197 +++++++++++++++-
 7 files changed, 352 insertions(+), 167 deletions(-)
 create mode 100644 seed/migrations/0233_alter_goal_options.py

diff --git a/seed/migrations/0233_alter_goal_options.py b/seed/migrations/0233_alter_goal_options.py
new file mode 100644
index 0000000000..e28621fc4c
--- /dev/null
+++ b/seed/migrations/0233_alter_goal_options.py
@@ -0,0 +1,16 @@
+# Generated by Django 3.2.25 on 2024-11-01 21:40
+
+from django.db import migrations
+
+
+class Migration(migrations.Migration):
+    dependencies = [
+        ("seed", "0232_reportconfiguration"),
+    ]
+
+    operations = [
+        migrations.AlterModelOptions(
+            name="goal",
+            options={"ordering": ["name"]},
+        ),
+    ]
diff --git a/seed/models/goals.py b/seed/models/goals.py
index dc2ce0f50c..f803fcd546 100644
--- a/seed/models/goals.py
+++ b/seed/models/goals.py
@@ -26,6 +26,9 @@ class Goal(models.Model):
     commitment_sqft = models.IntegerField(blank=True, null=True, validators=[MinValueValidator(0)])
     name = models.CharField(max_length=255, unique=True)
 
+    class Meta:
+        ordering = ["name"]
+
     def __str__(self):
         return f"Goal - {self.name}"
 
diff --git a/seed/serializers/goals.py b/seed/serializers/goals.py
index 95afcde5ec..177ff647b1 100644
--- a/seed/serializers/goals.py
+++ b/seed/serializers/goals.py
@@ -16,7 +16,20 @@ class Meta:
 
     def to_representation(self, obj):
         result = super().to_representation(obj)
-        result["level_name_index"] = obj.access_level_instance.depth - 1
+        level_index = obj.access_level_instance.depth - 1
+
+        details = {
+            "level_name_index": level_index,
+            "level_name": obj.organization.access_level_names[level_index],
+            "baseline_cycle_name": obj.baseline_cycle.name,
+            "current_cycle_name": obj.current_cycle.name,
+            "eui_column1_name": obj.eui_column1.display_name,
+            "eui_column2_name": obj.eui_column2.display_name if obj.eui_column2 else None,
+            "eui_column3_name": obj.eui_column3.display_name if obj.eui_column3 else None,
+            "area_column_name": obj.area_column.display_name,
+        }
+        result.update(details)
+
         return result
 
     def validate(self, data):
diff --git a/seed/static/seed/js/controllers/portfolio_summary_controller.js b/seed/static/seed/js/controllers/portfolio_summary_controller.js
index 20ecc0c8a1..43b2f20f55 100644
--- a/seed/static/seed/js/controllers/portfolio_summary_controller.js
+++ b/seed/static/seed/js/controllers/portfolio_summary_controller.js
@@ -85,14 +85,41 @@ angular.module('SEED.controller.portfolio_summary', [])
       // Can only sort based on baseline or current, not both. In the event of a conflict, use the more recent.
       let baseline_first = false;
 
-      const sort_goals = (goals) => goals.sort((a, b) => (a.name.toLowerCase() < b.name.toLowerCase() ? -1 : 1));
+      const load_data = (page) => {
+        $scope.data_loading = true;
+        const per_page = 50;
+        data = {
+          goal_id: $scope.goal.id,
+          page: page,
+          per_page: per_page,
+          baseline_first: baseline_first,
+          access_level_instance_id: $scope.goal.access_level_instance,
+          related_model_sort: $scope.related_model_sort
+        }
+        const column_filters = $scope.column_filters
+        const order_by = $scope.column_sorts
+        goal_service.load_data(data, column_filters, order_by).then((response) => {
+          const data = response.data
+          $scope.inventory_pagination = data.pagination;
+          $scope.property_lookup = data.property_lookup;
+          $scope.data = data.properties;
+          get_all_labels();
+          set_grid_options();
+          $scope.data_valid = Boolean(data.properties);
+          $scope.data_loading = false;
+        })
+      }
+
       // optionally pass a goal name to be set as $scope.goal - used on modal close
       const get_goals = (goal_name = false) => {
         goal_service.get_goals().then((result) => {
-          $scope.goals = _.isEmpty(result.goals) ? [] : sort_goals(result.goals);
+          $scope.goals = result.goals;
           $scope.goal = goal_name ?
             $scope.goals.find((goal) => goal.name === goal_name) :
             $scope.goals[0];
+          format_goal_details();
+          load_summary();
+          load_data(1);
         });
       };
       get_goals();
@@ -104,45 +131,43 @@ angular.module('SEED.controller.portfolio_summary', [])
       };
 
       // If goal changes, reset grid filters and repopulate ui-grids
-      $scope.$watch('goal', () => {
+      $scope.$watch('goal', (cur, old) => {
         if ($scope.gridApi) $scope.reset_sorts_filters();
         $scope.data_valid = false;
         if (_.isEmpty($scope.goal)) {
           $scope.valid = false;
           $scope.summary_valid = false;
-        } else {
+        } else if (old.id) {  // prevent duplicate request on page load
           reset_data();
         }
       });
 
+      // RP - backend - new endpoint for details table
       // selected goal details
       const format_goal_details = () => {
         $scope.change_selected_level_index();
-        const get_column_name = (column_id) => $scope.columns.find((col) => col.id === column_id).displayName;
-        const get_cycle_name = (cycle_id) => $scope.cycles.find((col) => col.id === cycle_id).name;
-        const level_name = $scope.level_names[$scope.goal.level_name_index];
         const access_level_instance = $scope.potential_level_instances.find((level) => level.id === $scope.goal.access_level_instance).name;
 
-        const commitment_sqft = $scope.goal.commitment_sqft ? $scope.goal.commitment_sqft.toLocaleString() : 'n/a';
+        const commitment_sqft = $scope.goal.commitment_sqft?.toLocaleString() || 'n/a';
         $scope.goal_details = [
           { // column 1
-            'Baseline Cycle': get_cycle_name($scope.goal.baseline_cycle),
-            'Current Cycle': get_cycle_name($scope.goal.current_cycle),
-            [level_name]: access_level_instance,
+            'Baseline Cycle': $scope.goal.baseline_cycle_name,
+            'Current Cycle': $scope.goal.current_cycle_name,
+            [$scope.goal.level_name]: access_level_instance,
             'Total Properties': null,
             'Commitment Sq. Ft': commitment_sqft
           },
           { // column 2
             'Portfolio Target': `${$scope.goal.target_percentage} %`,
-            'Area Column': get_column_name($scope.goal.area_column),
-            'Primary EUI': get_column_name($scope.goal.eui_column1)
+            'Area Column': $scope.goal.area_column_name,
+            'Primary EUI': $scope.goal.eui_column1_name
           }
         ];
         if ($scope.goal.eui_column2) {
-          $scope.goal_details[1]['Secondary EUI'] = get_column_name($scope.goal.eui_column2);
+          $scope.goal_details[1]['Secondary EUI'] = $scope.goal.eui_column2_name;
         }
         if ($scope.goal.eui_column3) {
-          $scope.goal_details[1]['Tertiary EUI'] = get_column_name($scope.goal.eui_column3);
+          $scope.goal_details[1]['Tertiary EUI'] = $scope.goal.eui_column3_name;
         }
       };
 
@@ -151,6 +176,7 @@ angular.module('SEED.controller.portfolio_summary', [])
         _.delay($scope.updateHeight, 150);
       };
 
+      // RP - backend - could use the existing summary endpoint
       const get_goal_stats = (summary) => {
         const passing_sqft = summary.current ? summary.current.total_sqft : null;
         // show help text if less than {50}% of properties are passing checks
@@ -206,7 +232,7 @@ angular.module('SEED.controller.portfolio_summary', [])
 
       const refresh_data = () => {
         load_summary();
-        load_inventory(1);
+        load_data(1);
       };
 
       const load_summary = () => {
@@ -225,72 +251,9 @@ angular.module('SEED.controller.portfolio_summary', [])
 
       $scope.page_change = (page) => {
         spinner_utility.show();
-        load_inventory(page);
+        load_data(page)
       };
-      const load_inventory = (page) => {
-        $scope.data_loading = true;
 
-        const access_level_instance_id = $scope.goal.access_level_instance;
-        const combined_result = {};
-        const per_page = 50;
-        const current_cycle = { id: $scope.goal.current_cycle };
-        const baseline_cycle = { id: $scope.goal.baseline_cycle };
-        // order of cycle property filter is dynamic based on column_sorts
-        const cycle_priority = baseline_first ? [baseline_cycle, current_cycle] : [current_cycle, baseline_cycle];
-
-        get_paginated_properties(page, per_page, cycle_priority[0], access_level_instance_id, true, null).then((result0) => {
-          $scope.inventory_pagination = result0.pagination;
-          let properties = result0.results;
-          combined_result[cycle_priority[0].id] = properties;
-          const property_ids = properties.map((p) => p.id);
-
-          get_paginated_properties(page, per_page, cycle_priority[1], access_level_instance_id, false, property_ids).then((result1) => {
-            properties = result1.results;
-            // if result0 returns fewer properties than result1, use result1 for ui-grid config
-            if (result1.pagination.num_pages > $scope.inventory_pagination.num_pages) {
-              baseline_first = !baseline_first;
-              $scope.inventory_pagination = result1.pagination;
-            }
-            combined_result[cycle_priority[1].id] = properties;
-            get_all_labels();
-            set_grid_options(combined_result);
-          }).then(() => {
-            $scope.data_loading = false;
-            $scope.data_valid = true;
-          });
-        });
-      };
-
-      const get_paginated_properties = (page, chunk, cycle, access_level_instance_id, include_filters_sorts, include_property_ids = null) => {
-        const fn = inventory_service.get_properties;
-        const [filters, sorts] = include_filters_sorts ? [$scope.column_filters, $scope.column_sorts] : [[], []];
-
-        return fn(
-          page,
-          chunk,
-          cycle,
-          undefined, // profile_id
-          undefined, // include_view_ids
-          undefined, // exclude_view_ids
-          true, // save_last_cycle
-          $scope.organization.id,
-          true, // include_related
-          filters,
-          sorts,
-          false, // ids_only
-          table_column_ids.join(),
-          access_level_instance_id,
-          include_property_ids,
-          $scope.goal.id, // optional param to retrieve goal note details
-          $scope.related_model_sort // optional param to sort on related models
-        );
-      };
-
-      const percentage = (a, b) => {
-        if (!a || b == null) return null;
-        const value = Math.round(((a - b) / a) * 100);
-        return Number.isNaN(value) ? null : value;
-      };
 
       // -------------- LABEL LOGIC -------------
 
@@ -404,83 +367,6 @@ angular.module('SEED.controller.portfolio_summary', [])
 
       // ------------ DATA TABLE LOGIC ---------
 
-      const set_eui_goal = (baseline, current, property, preferred_columns) => {
-        // only check defined columns
-        for (const col of preferred_columns.filter((c) => c)) {
-          if (baseline && _.isNil(property.baseline_eui)) {
-            property.baseline_eui = baseline[col.name];
-          }
-          if (current && _.isNil(property.current_eui)) {
-            property.current_eui = current[col.name];
-          }
-        }
-
-        property.baseline_kbtu = Math.round(property.baseline_sqft * property.baseline_eui) || undefined;
-        property.current_kbtu = Math.round(property.current_sqft * property.current_eui) || undefined;
-        property.eui_change = percentage(property.baseline_eui, property.current_eui);
-      };
-
-      const format_properties = (properties) => {
-        const area = $scope.columns.find((c) => c.id === $scope.goal.area_column);
-        const preferred_columns = [$scope.columns.find((c) => c.id === $scope.goal.eui_column1)];
-        if ($scope.goal.eui_column2) preferred_columns.push($scope.columns.find((c) => c.id === $scope.goal.eui_column2));
-        if ($scope.goal.eui_column3) preferred_columns.push($scope.columns.find((c) => c.id === $scope.goal.eui_column3));
-
-        const baseline_cycle_name = $scope.cycles.find((c) => c.id === $scope.goal.baseline_cycle).name;
-        const current_cycle_name = $scope.cycles.find((c) => c.id === $scope.goal.current_cycle).name;
-        // some fields span cycles (id, name, type)
-        // and others are cycle specific (source EUI, sqft)
-        const current_properties = properties[$scope.goal.current_cycle];
-        const baseline_properties = properties[$scope.goal.baseline_cycle];
-        const flat_properties = baseline_first ?
-          [baseline_properties, current_properties].flat() :
-          [current_properties, baseline_properties].flat();
-
-        // labels are related to property views, but cross cycles displays based on property
-        // create a lookup between property_view.id to property.id
-        $scope.property_lookup = {};
-        for (const p of flat_properties) {
-          $scope.property_lookup[p.property_view_id] = p.id;
-        }
-        const unique_ids = [...new Set(flat_properties.map((property) => property.id))];
-        const combined_properties = [];
-        for (const id of unique_ids) {
-          // find matching properties
-          const baseline = baseline_properties.find((p) => p.id === id) || {};
-          const current = current_properties.find((p) => p.id === id) || {};
-          // set accumulator
-          const property = combine_properties(current, baseline);
-          // add baseline stats
-          if (baseline) {
-            property.baseline_cycle = baseline_cycle_name;
-            property.baseline_sqft = baseline[area.name];
-          }
-          // add current stats
-          if (current) {
-            property.current_cycle = current_cycle_name;
-            property.current_sqft = current[area.name];
-            property.current_view_id = current.property_view_id;
-          }
-          // comparison stats
-          property.sqft_change = percentage(property.current_sqft, property.baseline_sqft);
-          set_eui_goal(baseline, current, property, preferred_columns);
-          combined_properties.push(property);
-        }
-        return combined_properties;
-      };
-
-      const combine_properties = (current, baseline) => {
-        // Given 2 properties, find non-null values and combine into a single property (favoring baseline if baseline_first)
-        const [a, b] = baseline_first ? [baseline, current] : [current, baseline];
-        const c = { ...b };
-        Object.keys(a).forEach((key) => {
-          if (a[key] !== null && a[key] !== undefined) {
-            c[key] = a[key];
-          }
-        });
-        return c;
-      };
-
       const apply_defaults = (cols, ...defaults) => {
         _.map(cols, (col) => _.defaults(col, ...defaults));
       };
@@ -503,6 +389,7 @@ angular.module('SEED.controller.portfolio_summary', [])
         { id: 4, value: 'Are these values correct?' },
         { id: 5, value: 'Other or multiple flags; explain in Additional Notes field' }
       ];
+      // RP - include in backend endpoint? the html is a little weird
       // handle cycle specific columns
       const selected_columns = () => {
         let cols = property_column_names.map((name) => $scope.columns.find((col) => col.column_name === name));
@@ -905,10 +792,9 @@ angular.module('SEED.controller.portfolio_summary', [])
         }
       };
 
-      const set_grid_options = (result) => {
+      const set_grid_options = () => {
         $scope.show_full_labels = { baseline: false, current: false };
         $scope.selected_ids = [];
-        $scope.data = format_properties(result);
         spinner_utility.hide();
         $scope.gridOptions = {
           data: 'data',
@@ -940,14 +826,14 @@ angular.module('SEED.controller.portfolio_summary', [])
               spinner_utility.show();
               _.debounce(() => {
                 updateColumnFilterSort();
-                load_inventory(1);
+                load_data(1);
               }, 500)();
             });
 
             gridApi.core.on.filterChanged($scope, _.debounce(() => {
               spinner_utility.show();
               updateColumnFilterSort();
-              load_inventory(1);
+              load_data(1);
             }, 2000));
 
             const selectionChanged = () => {
@@ -1025,7 +911,7 @@ angular.module('SEED.controller.portfolio_summary', [])
           $scope.selected_option = 'none';
           $scope.selected_count = 0;
           $scope.gridApi.selection.clearSelectedRows();
-          load_inventory();
+          load_data();
         });
       };
 
@@ -1049,7 +935,7 @@ angular.module('SEED.controller.portfolio_summary', [])
           $scope.selected_count = 0;
           $scope.gridApi.selection.clearSelectedRows();
           load_summary();
-          load_inventory();
+          load_data();
         });
       };
 
@@ -1119,11 +1005,13 @@ angular.module('SEED.controller.portfolio_summary', [])
           enableSorting: false,
           minRowsToShow: 1,
           onRegisterApi: (gridApi) => {
+            console.log('registerd')
             $scope.summaryGridApi = gridApi;
           }
         };
       };
 
+      // --- DATA QUALITY ---
       $scope.run_data_quality_check = () => {
         spinner_utility.show();
         data_quality_service.start_data_quality_checks([], [], $scope.goal.id)
@@ -1146,7 +1034,7 @@ angular.module('SEED.controller.portfolio_summary', [])
                     });
                     spinner_utility.hide();
                     load_summary();
-                    load_inventory();
+                    load_data();
                   });
               });
           })
diff --git a/seed/static/seed/js/services/goal_service.js b/seed/static/seed/js/services/goal_service.js
index 819b98cddf..a16f44f143 100644
--- a/seed/static/seed/js/services/goal_service.js
+++ b/seed/static/seed/js/services/goal_service.js
@@ -78,6 +78,47 @@ angular.module('SEED.service.goal', []).factory('goal_service', [
       .then((response) => response)
       .catch((response) => response);
 
+
+
+    const format_column_filters = (column_filters) => {
+      if (!column_filters) {
+        return {};
+      }
+      const filters = {};
+      for (const { name, operator, value } of column_filters) {
+        filters[`${name}__${operator}`] = value;
+      }
+      return filters;
+    };
+
+    const format_column_sorts = (column_sorts) => {
+      if (!column_sorts) {
+        return [];
+      }
+
+      const result = [];
+      for (const { name, direction } of column_sorts) {
+        const direction_operator = direction === 'desc' ? '-' : '';
+        result.push(`${direction_operator}${name}`);
+      }
+
+      return {order_by: result};
+    };
+
+    goal_service.load_data = (data, filters, sorts) => {
+      const params = {
+        organization_id: user_service.get_organization().id,
+        ...format_column_filters(filters),
+        ...format_column_sorts(sorts)
+      }
+      return $http.put(
+      `/api/v3/goals/${data.goal_id}/data/`,
+      data,
+      { params }
+    )
+      .then((response) => response)
+      .catch((response) => response)}
+
     return goal_service;
   }
 ]);
diff --git a/seed/tests/test_goals.py b/seed/tests/test_goals.py
index 49f8bcfb14..0704ed0e64 100644
--- a/seed/tests/test_goals.py
+++ b/seed/tests/test_goals.py
@@ -91,7 +91,7 @@ def setUp(self):
         self.view11 = self.property_view_factory.get_property_view(prprty=self.property1, state=self.state_11, cycle=self.cycle1)
         self.view13 = self.property_view_factory.get_property_view(prprty=self.property1, state=self.state_13, cycle=self.cycle3)
         self.view2 = self.property_view_factory.get_property_view(prprty=self.property2, state=self.state_2, cycle=self.cycle2)
-        self.view21 = self.property_view_factory.get_property_view(prprty=self.property3, state=self.state_31, cycle=self.cycle1)
+        self.view31 = self.property_view_factory.get_property_view(prprty=self.property3, state=self.state_31, cycle=self.cycle1)
         self.view33 = self.property_view_factory.get_property_view(prprty=self.property3, state=self.state_33, cycle=self.cycle3)
         self.view41 = self.property_view_factory.get_property_view(prprty=self.property4, state=self.state_41, cycle=self.cycle1)
 
@@ -486,3 +486,32 @@ def test_portfolio_summary(self):
         }
 
         assert summary == exp_summary
+
+    def test_goal_data(self):
+        self.login_as_root_member()
+        url = reverse_lazy("api:v3:goals-data", args=[self.root_goal.id]) + "?organization_id=" + str(self.org.id)
+        data = {
+            "goal_id": self.root_goal.id,
+            "page": 1,
+            "per_page": 50,
+            "baseline_first": True,
+            "access_level_instance_id": self.org.root.id,
+            "related_model_sort": False,
+        }
+        response = self.client.put(url, data=json.dumps(data), content_type="application/json")
+        assert response.status_code == 200
+        data = response.json()
+        assert list(data.keys()) == ["pagination", "properties", "property_lookup"]
+
+        data = {
+            "goal_id": self.root_goal.id,
+            "page": 2,
+            "per_page": 1,
+            "baseline_first": True,
+            "access_level_instance_id": self.org.root.id,
+            "related_model_sort": False,
+        }
+        response = self.client.put(url, data=json.dumps(data), content_type="application/json")
+        data = response.json()
+        assert len(data["properties"]) == 1
+        assert data["property_lookup"] == {str(self.view31.id): self.property3.id, str(self.view33.id): self.property3.id}
diff --git a/seed/views/v3/goals.py b/seed/views/v3/goals.py
index 5ae4595f4d..564040e369 100644
--- a/seed/views/v3/goals.py
+++ b/seed/views/v3/goals.py
@@ -3,19 +3,26 @@
 See also https://github.com/SEED-platform/seed/blob/main/LICENSE.md
 """
 
+import math
+
+from django.core.paginator import EmptyPage, PageNotAnInteger, Paginator
+from django.db.utils import DataError
 from django.http import JsonResponse
 from django.utils.decorators import method_decorator
+from pint import Quantity
 from rest_framework import status
 from rest_framework.decorators import action
 
 from seed.decorators import ajax_request_class
 from seed.lib.superperms.orgs.decorators import has_hierarchy_access, has_perm_class
-from seed.models import AccessLevelInstance, Goal, GoalNote, HistoricalNote, Organization, Property
+from seed.models import AccessLevelInstance, Column, Goal, GoalNote, HistoricalNote, Organization, Property, TaxLotProperty
 from seed.serializers.goals import GoalSerializer
+from seed.serializers.pint import apply_display_unit_preferences
 from seed.utils.api import OrgMixin
 from seed.utils.api_schema import swagger_auto_schema_org_query_param
 from seed.utils.goal_notes import get_permission_data
 from seed.utils.goals import get_or_create_goal_notes, get_portfolio_summary
+from seed.utils.search import FilterError, build_related_model_filters_and_sorts, build_view_filters_and_sorts
 from seed.utils.viewsets import ModelViewSetWithoutPatch
 
 
@@ -149,3 +156,191 @@ def bulk_update_goal_notes(self, request, pk):
             result = goal_notes.update(**data)
 
         return JsonResponse({"status": "success", "message": f"Updated {result} properties"})
+
+    @ajax_request_class
+    @swagger_auto_schema_org_query_param
+    @has_perm_class("requires_viewer")
+    @has_hierarchy_access(goal_id_kwarg="pk")
+    @action(detail=True, methods=["PUT"])
+    def data(self, request, pk):
+        """
+        Gets goal data for the main grid
+        """
+        # Init a bunch of values
+        org_id = int(self.get_organization(request))
+        try:
+            org = Organization.objects.get(pk=org_id)
+            goal = Goal.objects.get(pk=pk)
+        except (Organization.DoesNotExist, Goal.DoesNotExist):
+            return JsonResponse({"status": "error", "message": "No such resource."})
+        page = request.data.get("page")
+        per_page = request.data.get("per_page")
+        baseline_first = request.data.get("baseline_first")
+        access_level_instance_id = request.data.get("access_level_instance_id")
+        related_model_sort = request.data.get("related_model_sort")
+        inventory_type = "property"
+        access_level_instance = AccessLevelInstance.objects.get(pk=access_level_instance_id)
+        columns_from_database = Column.retrieve_all(
+            org_id=org_id,
+            inventory_type=inventory_type,
+            only_used=False,
+            include_related=False,
+        )
+        # need metric 1
+        # need metric 2
+        show_columns = list(Column.objects.filter(organization_id=org_id).values_list("id", flat=True))
+        key1, key2 = ("baseline", "current") if baseline_first else ("current", "baseline")
+
+        cycle1 = getattr(goal, f"{key1}_cycle")
+        cycle2 = getattr(goal, f"{key2}_cycle")
+        views1 = cycle1.propertyview_set.filter(
+            property__access_level_instance__lft__gte=access_level_instance.lft,
+            property__access_level_instance__rgt__lte=access_level_instance.rgt,
+        )
+        try:
+            # Sorts initiated from Portfolio Summary that contain related model names (goal_note, historical_note) require custom handling
+            if related_model_sort:
+                filters, annotations, order_by = build_related_model_filters_and_sorts(request.query_params, columns_from_database)
+            else:
+                filters, annotations, order_by = build_view_filters_and_sorts(
+                    request.query_params, columns_from_database, inventory_type, org.access_level_names
+                )
+        except FilterError as e:
+            return JsonResponse({"status": "error", "message": f"Error filtering: {e!s}"}, status=status.HTTP_400_BAD_REQUEST)
+
+        try:
+            import logging
+            logging.error(">>> v1a %s", views1.count())
+            logging.error(">>> v1a %s", views1)
+            logging.error(">>> filters %s", filters)
+            logging.error(">>> annotations %s", annotations)
+            logging.error(">>> order_by %s", order_by)
+            views1 = views1.annotate(**annotations).filter(filters).order_by(*order_by)
+            logging.error(">>> v1b %s", views1.count())
+        except ValueError as e:
+            return JsonResponse({"status": "error", "message": f"Error filtering: {e!s}"}, status=status.HTTP_400_BAD_REQUEST)
+
+        # Paginate results
+        paginator = Paginator(views1, per_page)
+        try:
+            views1 = paginator.page(page)
+            page = int(page)
+        except PageNotAnInteger:
+            views1 = paginator.page(1)
+            page = 1
+        except EmptyPage:
+            views1 = paginator.page(paginator.num_pages)
+            page = paginator.num_pages
+        except DataError as e:
+            return JsonResponse(
+                {
+                    "status": "error",
+                    "message": f"Error filtering - your data might not match the column settings data type: {e!s}",
+                },
+                status=status.HTTP_400_BAD_REQUEST,
+            )
+        except IndexError as e:
+            return JsonResponse(
+                {"status": "error", "message": f"Error filtering - Clear filters and try again: {e!s}"}, status=status.HTTP_400_BAD_REQUEST
+            )
+
+        property_ids = [v.property_id for v in views1]
+        # fetch cycle 2 properties
+        views2 = cycle2.propertyview_set.filter(
+            property__id__in=property_ids,
+            property__access_level_instance__lft__gte=access_level_instance.lft,
+            property__access_level_instance__rgt__lte=access_level_instance.rgt,
+        )
+
+        properties1 = TaxLotProperty.serialize(views1, show_columns, columns_from_database, False, pk)
+        properties2 = TaxLotProperty.serialize(views2, show_columns, columns_from_database, False, pk)
+        # collapse pint Qunatity units to their magnitudes
+        properties1 = [apply_display_unit_preferences(org, x) for x in properties1]
+        properties2 = [apply_display_unit_preferences(org, x) for x in properties2]
+
+        area_name = f"{goal.area_column.column_name}_{goal.area_column.id}"
+        eui_columns = [f"{col.column_name}_{col.id}" for col in goal.eui_columns()]
+
+        # lookup for pv.id to p.id
+        property_lookup = {}
+        for p in properties1 + properties2:
+            property_lookup[p["property_view_id"]] = p["id"]
+
+        properties = []
+        for p1 in properties1:
+            p2 = next((p for p in properties2 if p["id"] == p1["id"]), None)
+            property = combine_properties(p1, p2)
+
+            sqft1 = p1.get(area_name)
+            sqft2 = p2.get(area_name) if p2 else None
+
+            # add cycle specific and aggregated goal stats
+            property[f"{key1}_cycle"] = cycle1.name
+            property[f"{key2}_cycle"] = cycle2.name
+            property[f"{key1}_sqft"] = convert_quantity(sqft1)
+            property[f"{key2}_sqft"] = convert_quantity(sqft2)
+            property[f"{key1}_eui"] = get_preferred(p1, eui_columns)
+            property[f"{key2}_eui"] = get_preferred(p2, eui_columns)
+            property["baseline_kbtu"] = get_kbtu(property, "baseline")
+            property["current_kbtu"] = get_kbtu(property, "current")
+            property["sqft_change"] = percentage(property["current_sqft"], property["baseline_sqft"])
+            property["eui_change"] = percentage(property["baseline_eui"], property["current_eui"])
+
+            properties.append(property)
+        # UNIT ERRORS, need to convert quantity to number
+        # not mine, but others. check taxlot property serialize unit conversion.
+        # SHOULD REALLY HAVE A SHORT LIST OF COLUMNS
+
+        # PAGINATION
+        # FILTERS
+
+        return JsonResponse(
+            {
+                "pagination": {
+                    "page": page,
+                    "start": paginator.page(page).start_index(),
+                    "end": paginator.page(page).end_index(),
+                    "num_pages": paginator.num_pages,
+                    "has_next": paginator.page(page).has_next(),
+                    "has_previous": paginator.page(page).has_previous(),
+                    "total": paginator.count,
+                },
+                "properties": properties,
+                "property_lookup": property_lookup,
+            }
+        )
+
+
+def combine_properties(p1, p2):
+    if not p2:
+        return p1
+    combined = p1.copy()
+    for key, value in p2.items():
+        if value is not None:
+            combined[key] = value
+    return combined
+
+
+def percentage(a, b):
+    if not a or b is None:
+        return None
+    value = round(((a - b) / a) * 100)
+    return None if math.isnan(value) else value
+
+
+def get_preferred(p, columns):
+    if not p:
+        return
+    for col in columns:
+        return convert_quantity(p[col])
+
+
+def convert_quantity(value):
+    if isinstance(value, Quantity):
+        value = value.m
+    return value
+
+
+def get_kbtu(prop, key):
+    if prop[f"{key}_sqft"] is not None and prop[f"{key}_eui"] is not None:
+        return round(prop[f"{key}_sqft"] * prop[f"{key}_eui"])

From 758ca3da0e4f15bf6e1db7cd104759f1ab289ecb Mon Sep 17 00:00:00 2001
From: Ross Perry <ross.perry@Rosss-MacBook-Pro-2.local>
Date: Fri, 8 Nov 2024 10:32:16 -0700
Subject: [PATCH 2/6] sorting on related functional

clean up
---
 seed/models/data_quality.py                   |  8 --
 .../portfolio_summary_controller.js           | 26 +++---
 seed/static/seed/js/services/goal_service.js  | 19 ++---
 seed/tests/test_goals.py                      | 82 +++++++++++++++++++
 seed/utils/inventory_filter.py                | 17 ++--
 seed/utils/search.py                          | 71 ++++++++--------
 seed/views/v3/goals.py                        | 26 ++----
 7 files changed, 151 insertions(+), 98 deletions(-)

diff --git a/seed/models/data_quality.py b/seed/models/data_quality.py
index 09c32e392e..7e45d5f4fe 100644
--- a/seed/models/data_quality.py
+++ b/seed/models/data_quality.py
@@ -977,10 +977,6 @@ def append_to_apply_labels():
                         self.add_result_range_error(row["current"].id, rule, data_type, value)
                         self.update_status_label(PropertyViewLabel, rule, current_view.id, row["current"].id)
 
-                # other rule condition types
-                else:
-                    logging.error(">>> OTHER")
-
             else:  # Within Cycle
                 for cycle_key in ["baseline", "current"]:
                     state = row["baseline"] if cycle_key == "baseline" else row["current"]
@@ -1005,10 +1001,6 @@ def append_to_apply_labels():
                             self.add_result_is_null(state.id, rule, data_type, value)
                             self.update_status_label(PropertyViewLabel, rule, view.id, state.id)
 
-                    # other rule condition types.
-                    else:
-                        logging.error(">>> OTHER")
-
             goal_note.passed_checks = all(results)
 
         # if there are multiple rules with the same label, determine if they are all passing to add or remove the label
diff --git a/seed/static/seed/js/controllers/portfolio_summary_controller.js b/seed/static/seed/js/controllers/portfolio_summary_controller.js
index 43b2f20f55..503be52e5d 100644
--- a/seed/static/seed/js/controllers/portfolio_summary_controller.js
+++ b/seed/static/seed/js/controllers/portfolio_summary_controller.js
@@ -88,18 +88,18 @@ angular.module('SEED.controller.portfolio_summary', [])
       const load_data = (page) => {
         $scope.data_loading = true;
         const per_page = 50;
-        data = {
+        const data = {
           goal_id: $scope.goal.id,
-          page: page,
-          per_page: per_page,
-          baseline_first: baseline_first,
+          page,
+          per_page,
+          baseline_first,
           access_level_instance_id: $scope.goal.access_level_instance,
           related_model_sort: $scope.related_model_sort
-        }
-        const column_filters = $scope.column_filters
-        const order_by = $scope.column_sorts
+        };
+        const column_filters = $scope.column_filters;
+        const order_by = $scope.column_sorts;
         goal_service.load_data(data, column_filters, order_by).then((response) => {
-          const data = response.data
+          const data = response.data;
           $scope.inventory_pagination = data.pagination;
           $scope.property_lookup = data.property_lookup;
           $scope.data = data.properties;
@@ -107,8 +107,8 @@ angular.module('SEED.controller.portfolio_summary', [])
           set_grid_options();
           $scope.data_valid = Boolean(data.properties);
           $scope.data_loading = false;
-        })
-      }
+        });
+      };
 
       // optionally pass a goal name to be set as $scope.goal - used on modal close
       const get_goals = (goal_name = false) => {
@@ -137,7 +137,7 @@ angular.module('SEED.controller.portfolio_summary', [])
         if (_.isEmpty($scope.goal)) {
           $scope.valid = false;
           $scope.summary_valid = false;
-        } else if (old.id) {  // prevent duplicate request on page load
+        } else if (old.id) { // prevent duplicate request on page load
           reset_data();
         }
       });
@@ -251,10 +251,9 @@ angular.module('SEED.controller.portfolio_summary', [])
 
       $scope.page_change = (page) => {
         spinner_utility.show();
-        load_data(page)
+        load_data(page);
       };
 
-
       // -------------- LABEL LOGIC -------------
 
       $scope.max_label_width = 750;
@@ -1005,7 +1004,6 @@ angular.module('SEED.controller.portfolio_summary', [])
           enableSorting: false,
           minRowsToShow: 1,
           onRegisterApi: (gridApi) => {
-            console.log('registerd')
             $scope.summaryGridApi = gridApi;
           }
         };
diff --git a/seed/static/seed/js/services/goal_service.js b/seed/static/seed/js/services/goal_service.js
index a16f44f143..802811cea0 100644
--- a/seed/static/seed/js/services/goal_service.js
+++ b/seed/static/seed/js/services/goal_service.js
@@ -78,8 +78,6 @@ angular.module('SEED.service.goal', []).factory('goal_service', [
       .then((response) => response)
       .catch((response) => response);
 
-
-
     const format_column_filters = (column_filters) => {
       if (!column_filters) {
         return {};
@@ -102,7 +100,7 @@ angular.module('SEED.service.goal', []).factory('goal_service', [
         result.push(`${direction_operator}${name}`);
       }
 
-      return {order_by: result};
+      return { order_by: result };
     };
 
     goal_service.load_data = (data, filters, sorts) => {
@@ -110,14 +108,15 @@ angular.module('SEED.service.goal', []).factory('goal_service', [
         organization_id: user_service.get_organization().id,
         ...format_column_filters(filters),
         ...format_column_sorts(sorts)
-      }
+      };
       return $http.put(
-      `/api/v3/goals/${data.goal_id}/data/`,
-      data,
-      { params }
-    )
-      .then((response) => response)
-      .catch((response) => response)}
+        `/api/v3/goals/${data.goal_id}/data/`,
+        data,
+        { params }
+      )
+        .then((response) => response)
+        .catch((response) => response);
+    };
 
     return goal_service;
   }
diff --git a/seed/tests/test_goals.py b/seed/tests/test_goals.py
index 0704ed0e64..1c73c52a94 100644
--- a/seed/tests/test_goals.py
+++ b/seed/tests/test_goals.py
@@ -515,3 +515,85 @@ def test_goal_data(self):
         data = response.json()
         assert len(data["properties"]) == 1
         assert data["property_lookup"] == {str(self.view31.id): self.property3.id, str(self.view33.id): self.property3.id}
+
+    def test_related_filter(self):
+        alphabet = ["a", "c", "b"]
+        questions = ["Is this value correct?", "Are these values correct?", "Other or multiple flags; explain in Additional Notes field"]
+        booleans = [True, False, True]
+        for idx, goal_note in enumerate(self.root_goal.goalnote_set.all()):
+            goal_note.resolution = alphabet[idx]
+            goal_note.question = questions[idx]
+            goal_note.passed_checks = booleans[idx]
+            goal_note.new_or_acquired = booleans[idx]
+            goal_note.save()
+
+        for idx, historical_note in enumerate(HistoricalNote.objects.filter(property__in=self.root_goal.properties())):
+            historical_note.text = alphabet[idx]
+            historical_note.save()
+
+        goal_note = self.root_goal.goalnote_set.first()
+        goal_note.new_or_acquired = True
+        goal_note.passed_checks = True
+        goal_note.save()
+
+        # sort resolution ascending
+        params = f"?organization_id={self.org.id}&order_by=property__goal_note__resolution"
+        path = reverse_lazy("api:v3:goals-data", args=[self.root_goal.id])
+        url = path + params
+        data = {
+            "goal_id": self.root_goal.id,
+            "page": 1,
+            "per_page": 50,
+            "baseline_first": True,
+            "access_level_instance_id": self.org.root.id,
+            "related_model_sort": True,
+        }
+        response = self.client.put(url, data=json.dumps(data), content_type="application/json")
+        assert response.status_code == 200
+        response = response.json()
+        resolutions = [p["goal_note"]["resolution"] for p in response["properties"]]
+        assert resolutions == ["a", "b", "c"]
+
+        # sort resolution descending
+        params = f"?organization_id={self.org.id}&order_by=-property__goal_note__resolution"
+        url = path + params
+        response = self.client.put(url, data=json.dumps(data), content_type="application/json")
+        response = response.json()
+        resolutions = [p["goal_note"]["resolution"] for p in response["properties"]]
+        assert resolutions == ["c", "b", "a"]
+
+        # sort historical note text
+        params = f"?organization_id={self.org.id}&order_by=property__historical_note__text"
+        url = path + params
+        response = self.client.put(url, data=json.dumps(data), content_type="application/json")
+        response = response.json()
+        historical_notes = [p["historical_note"]["text"] for p in response["properties"]]
+        assert historical_notes == ["a", "b", "c"]
+
+        # sort question
+        params = f"?organization_id={self.org.id}&order_by=property__goal_note__question"
+        url = path + params
+        response = self.client.put(url, data=json.dumps(data), content_type="application/json")
+        response = response.json()
+        questions = [p["goal_note"]["question"] for p in response["properties"]]
+        assert questions == [
+            "Are these values correct?",
+            "Is this value correct?",
+            "Other or multiple flags; explain in Additional Notes field",
+        ]
+
+        # sort passsed checks
+        params = f"?organization_id={self.org.id}&order_by=property__goal_note__passed_checks"
+        url = path + params
+        response = self.client.put(url, data=json.dumps(data), content_type="application/json")
+        response = response.json()
+        passed_checks = [p["goal_note"]["passed_checks"] for p in response["properties"]]
+        assert passed_checks == [True, True, False]
+
+        # sort new or acquired desc
+        params = f"?organization_id={self.org.id}&order_by=-property__goal_note__new_or_acquired"
+        url = path + params
+        response = self.client.put(url, data=json.dumps(data), content_type="application/json")
+        response = response.json()
+        passed_checks = [p["goal_note"]["passed_checks"] for p in response["properties"]]
+        assert passed_checks == [False, True, True]
diff --git a/seed/utils/inventory_filter.py b/seed/utils/inventory_filter.py
index cbc41891eb..26163168ef 100644
--- a/seed/utils/inventory_filter.py
+++ b/seed/utils/inventory_filter.py
@@ -25,7 +25,7 @@
     TaxLotView,
 )
 from seed.serializers.pint import apply_display_unit_preferences
-from seed.utils.search import FilterError, build_related_model_filters_and_sorts, build_view_filters_and_sorts
+from seed.utils.search import FilterError, build_view_filters_and_sorts
 
 
 def get_filtered_results(request: Request, inventory_type: Literal["property", "taxlot"], profile_id: int) -> JsonResponse:
@@ -37,8 +37,6 @@ def get_filtered_results(request: Request, inventory_type: Literal["property", "
     # check if there is a query parameter for the profile_id. If so, then use that one
     profile_id = request.query_params.get("profile_id", profile_id)
     shown_column_ids = request.query_params.get("shown_column_ids")
-    goal_id = request.data.get("goal_id")
-    related_model_sort = request.data.get("related_model_sort")
 
     if not org_id:
         return JsonResponse(
@@ -103,15 +101,10 @@ def get_filtered_results(request: Request, inventory_type: Literal["property", "
         only_used=False,
         include_related=include_related,
     )
-
     try:
-        # Sorts initiated from Portfolio Summary that contain related model names (goal_note, historical_note) require custom handling
-        if related_model_sort:
-            filters, annotations, order_by = build_related_model_filters_and_sorts(request.query_params, columns_from_database)
-        else:
-            filters, annotations, order_by = build_view_filters_and_sorts(
-                request.query_params, columns_from_database, inventory_type, org.access_level_names
-            )
+        filters, annotations, order_by = build_view_filters_and_sorts(
+            request.query_params, columns_from_database, inventory_type, org.access_level_names
+        )
     except FilterError as e:
         return JsonResponse({"status": "error", "message": f"Error filtering: {e!s}"}, status=status.HTTP_400_BAD_REQUEST)
 
@@ -238,7 +231,7 @@ def get_filtered_results(request: Request, inventory_type: Literal["property", "
             show_columns = None
 
     try:
-        related_results = TaxLotProperty.serialize(views, show_columns, columns_from_database, include_related, goal_id)
+        related_results = TaxLotProperty.serialize(views, show_columns, columns_from_database, include_related)
     except DataError as e:
         return JsonResponse(
             {
diff --git a/seed/utils/search.py b/seed/utils/search.py
index 93248bf586..7735c92038 100644
--- a/seed/utils/search.py
+++ b/seed/utils/search.py
@@ -509,39 +509,44 @@ def build_view_filters_and_sorts(
     return new_filters, annotations, order_by
 
 
-def build_related_model_filters_and_sorts(filters: QueryDict, columns: list[dict]) -> tuple[Q, AnnotationDict, list[str]]:
-    """Primarily used for sorting the Portfolio Summary on related columns like goal_notes and historical_notes"""
-    order_by = []
-    annotations = {}
-    columns_by_name = {}
-    for column in columns:
-        if column["related"]:
-            continue
-        columns_by_name[column["name"]] = column
-
-    column_name = filters.get("order_by")
-    if not column_name:
-        return Q(), {}, ["id"]
-
-    direction = "-" if column_name.startswith("-") else ""
-    column_name = column_name.lstrip("-")
-
-    if "goal_note" in column_name:
-        column_name = column_name.replace("goal_note", "goalnote")
+def filter_views_on_related(views1, goal, filters, cycle1):
+    p_ids = views1.values_list("property_id", flat=True)
+    order_by = filters.get("order_by").replace("property__", "")
+    direction = "-" if order_by.startswith("-") else ""
+    order_by = order_by.lstrip("-")
+    goal_note = "goal_note" in order_by
+    historical_note = "historical_note" in order_by
+    order_by = order_by.replace("goal_note__", "").replace("historical_note__", "")
+    boolean_column = order_by in ["passed_checks", "new_or_acquired"]
+    target = False if boolean_column else ""
+    blanks_last = Case(When(**{order_by: target}, then=Value(1)), default=Value(0), output_field=IntegerField())
+
+    views = []
+    if goal_note:
+        goal_notes = (
+            goal.goalnote_set.filter(property__in=p_ids)
+            .annotate(custom_order=blanks_last)
+            .order_by(direction + "custom_order", direction + order_by)
+        )
+        for goal_note in goal_notes:
+            view = goal_note.property.views.filter(cycle=cycle1).first()
+            if view:
+                views.append(view)
+
+    elif historical_note:
+        from seed.models.notes import HistoricalNote
+
+        historical_notes = (
+            HistoricalNote.objects.filter(property__in=p_ids)
+            .annotate(custom_order=blanks_last)
+            .order_by(direction + "custom_order", direction + order_by)
+        )
+        for historical_note in historical_notes:
+            view = historical_note.property.views.filter(cycle=cycle1).first()
+            if view:
+                views.append(view)
 
-    boolean_column = column_name in ["property__goalnote__passed_checks", "property__goalnote__new_or_acquired"]
-    target: Union[bool, str]
-    if boolean_column:
-        target = False
     else:
-        target = ""
-
-    related_model = Case(When(**{column_name: target}, then=Value(1)), default=Value(0), output_field=IntegerField())
-    parsed_annotations = {"related_model": related_model}
-    parsed_sort = [direction + "related_model", direction + column_name]
-
-    if parsed_sort is not None:
-        order_by.extend(parsed_sort)
-        annotations.update(parsed_annotations)
+        views = views1
 
-    return Q(), annotations, order_by
+    return views
diff --git a/seed/views/v3/goals.py b/seed/views/v3/goals.py
index 564040e369..d2df1053fb 100644
--- a/seed/views/v3/goals.py
+++ b/seed/views/v3/goals.py
@@ -22,7 +22,7 @@
 from seed.utils.api_schema import swagger_auto_schema_org_query_param
 from seed.utils.goal_notes import get_permission_data
 from seed.utils.goals import get_or_create_goal_notes, get_portfolio_summary
-from seed.utils.search import FilterError, build_related_model_filters_and_sorts, build_view_filters_and_sorts
+from seed.utils.search import FilterError, build_view_filters_and_sorts, filter_views_on_related
 from seed.utils.viewsets import ModelViewSetWithoutPatch
 
 
@@ -186,8 +186,6 @@ def data(self, request, pk):
             only_used=False,
             include_related=False,
         )
-        # need metric 1
-        # need metric 2
         show_columns = list(Column.objects.filter(organization_id=org_id).values_list("id", flat=True))
         key1, key2 = ("baseline", "current") if baseline_first else ("current", "baseline")
 
@@ -196,27 +194,19 @@ def data(self, request, pk):
         views1 = cycle1.propertyview_set.filter(
             property__access_level_instance__lft__gte=access_level_instance.lft,
             property__access_level_instance__rgt__lte=access_level_instance.rgt,
-        )
+        ).select_related("property")
+
         try:
             # Sorts initiated from Portfolio Summary that contain related model names (goal_note, historical_note) require custom handling
             if related_model_sort:
-                filters, annotations, order_by = build_related_model_filters_and_sorts(request.query_params, columns_from_database)
+                views1 = filter_views_on_related(views1, goal, request.query_params, cycle1)
             else:
                 filters, annotations, order_by = build_view_filters_and_sorts(
                     request.query_params, columns_from_database, inventory_type, org.access_level_names
                 )
+                views1 = views1.annotate(**annotations).filter(filters).order_by(*order_by)
         except FilterError as e:
             return JsonResponse({"status": "error", "message": f"Error filtering: {e!s}"}, status=status.HTTP_400_BAD_REQUEST)
-
-        try:
-            import logging
-            logging.error(">>> v1a %s", views1.count())
-            logging.error(">>> v1a %s", views1)
-            logging.error(">>> filters %s", filters)
-            logging.error(">>> annotations %s", annotations)
-            logging.error(">>> order_by %s", order_by)
-            views1 = views1.annotate(**annotations).filter(filters).order_by(*order_by)
-            logging.error(">>> v1b %s", views1.count())
         except ValueError as e:
             return JsonResponse({"status": "error", "message": f"Error filtering: {e!s}"}, status=status.HTTP_400_BAD_REQUEST)
 
@@ -287,12 +277,6 @@ def data(self, request, pk):
             property["eui_change"] = percentage(property["baseline_eui"], property["current_eui"])
 
             properties.append(property)
-        # UNIT ERRORS, need to convert quantity to number
-        # not mine, but others. check taxlot property serialize unit conversion.
-        # SHOULD REALLY HAVE A SHORT LIST OF COLUMNS
-
-        # PAGINATION
-        # FILTERS
 
         return JsonResponse(
             {

From 64136e7df2925a9cf054c99f4546e76f53b8ba8f Mon Sep 17 00:00:00 2001
From: Ross Perry <ross.perry@Rosss-MacBook-Pro-2.local>
Date: Fri, 22 Nov 2024 09:51:13 -0700
Subject: [PATCH 3/6] cleanup

---
 .../portfolio_summary_controller.js           |  3 --
 .../seed/js/services/inventory_service.js     |  4 --
 seed/utils/goals.py                           | 36 ++++++++++++++++++
 seed/views/v3/goals.py                        | 37 +------------------
 4 files changed, 37 insertions(+), 43 deletions(-)

diff --git a/seed/static/seed/js/controllers/portfolio_summary_controller.js b/seed/static/seed/js/controllers/portfolio_summary_controller.js
index 503be52e5d..d8291ab781 100644
--- a/seed/static/seed/js/controllers/portfolio_summary_controller.js
+++ b/seed/static/seed/js/controllers/portfolio_summary_controller.js
@@ -142,7 +142,6 @@ angular.module('SEED.controller.portfolio_summary', [])
         }
       });
 
-      // RP - backend - new endpoint for details table
       // selected goal details
       const format_goal_details = () => {
         $scope.change_selected_level_index();
@@ -176,7 +175,6 @@ angular.module('SEED.controller.portfolio_summary', [])
         _.delay($scope.updateHeight, 150);
       };
 
-      // RP - backend - could use the existing summary endpoint
       const get_goal_stats = (summary) => {
         const passing_sqft = summary.current ? summary.current.total_sqft : null;
         // show help text if less than {50}% of properties are passing checks
@@ -388,7 +386,6 @@ angular.module('SEED.controller.portfolio_summary', [])
         { id: 4, value: 'Are these values correct?' },
         { id: 5, value: 'Other or multiple flags; explain in Additional Notes field' }
       ];
-      // RP - include in backend endpoint? the html is a little weird
       // handle cycle specific columns
       const selected_columns = () => {
         let cols = property_column_names.map((name) => $scope.columns.find((col) => col.column_name === name));
diff --git a/seed/static/seed/js/services/inventory_service.js b/seed/static/seed/js/services/inventory_service.js
index 8e48e89b83..cd7eaba6fa 100644
--- a/seed/static/seed/js/services/inventory_service.js
+++ b/seed/static/seed/js/services/inventory_service.js
@@ -65,8 +65,6 @@ angular.module('SEED.service.inventory', []).factory('inventory_service', [
       shown_column_ids = null,
       access_level_instance_id = null,
       include_property_ids = null,
-      goal_id = null,
-      related_model_sort = null
     ) => {
       organization_id = organization_id ?? user_service.get_organization().id;
 
@@ -104,8 +102,6 @@ angular.module('SEED.service.inventory', []).factory('inventory_service', [
         profile_id,
         // conditionally add optional params
         ...(access_level_instance_id && { access_level_instance_id }),
-        ...(goal_id && { goal_id }),
-        ...(related_model_sort && { related_model_sort })
       };
       // add access_level_instance if it exists
 
diff --git a/seed/utils/goals.py b/seed/utils/goals.py
index 0c18f22918..98a2d88493 100644
--- a/seed/utils/goals.py
+++ b/seed/utils/goals.py
@@ -2,6 +2,8 @@
 SEED Platform (TM), Copyright (c) Alliance for Sustainable Energy, LLC, and other contributors.
 See also https://github.com/SEED-platform/seed/blob/main/LICENSE.md
 """
+import math
+from pint import Quantity
 
 from django.db.models import Case, F, FloatField, IntegerField, Prefetch, Sum, Value, When
 from django.db.models.fields.json import KeyTextTransform
@@ -214,3 +216,37 @@ def get_state_pairs(property_ids, goal_id):
         state_pairs.append({"property": property, "baseline": baseline_state, "current": current_state})
 
     return state_pairs
+
+def combine_properties(p1, p2):
+    if not p2:
+        return p1
+    combined = p1.copy()
+    for key, value in p2.items():
+        if value is not None:
+            combined[key] = value
+    return combined
+
+
+def percentage(a, b):
+    if not a or b is None:
+        return None
+    value = round(((a - b) / a) * 100)
+    return None if math.isnan(value) else value
+
+
+def get_preferred(p, columns):
+    if not p:
+        return
+    for col in columns:
+        return convert_quantity(p[col])
+
+
+def convert_quantity(value):
+    if isinstance(value, Quantity):
+        value = value.m
+    return value
+
+
+def get_kbtu(prop, key):
+    if prop[f"{key}_sqft"] is not None and prop[f"{key}_eui"] is not None:
+        return round(prop[f"{key}_sqft"] * prop[f"{key}_eui"])
diff --git a/seed/views/v3/goals.py b/seed/views/v3/goals.py
index d2df1053fb..579689c022 100644
--- a/seed/views/v3/goals.py
+++ b/seed/views/v3/goals.py
@@ -9,7 +9,6 @@
 from django.db.utils import DataError
 from django.http import JsonResponse
 from django.utils.decorators import method_decorator
-from pint import Quantity
 from rest_framework import status
 from rest_framework.decorators import action
 
@@ -21,7 +20,7 @@
 from seed.utils.api import OrgMixin
 from seed.utils.api_schema import swagger_auto_schema_org_query_param
 from seed.utils.goal_notes import get_permission_data
-from seed.utils.goals import get_or_create_goal_notes, get_portfolio_summary
+from seed.utils.goals import get_or_create_goal_notes, get_portfolio_summary, combine_properties, percentage, get_preferred, convert_quantity, get_kbtu
 from seed.utils.search import FilterError, build_view_filters_and_sorts, filter_views_on_related
 from seed.utils.viewsets import ModelViewSetWithoutPatch
 
@@ -294,37 +293,3 @@ def data(self, request, pk):
             }
         )
 
-
-def combine_properties(p1, p2):
-    if not p2:
-        return p1
-    combined = p1.copy()
-    for key, value in p2.items():
-        if value is not None:
-            combined[key] = value
-    return combined
-
-
-def percentage(a, b):
-    if not a or b is None:
-        return None
-    value = round(((a - b) / a) * 100)
-    return None if math.isnan(value) else value
-
-
-def get_preferred(p, columns):
-    if not p:
-        return
-    for col in columns:
-        return convert_quantity(p[col])
-
-
-def convert_quantity(value):
-    if isinstance(value, Quantity):
-        value = value.m
-    return value
-
-
-def get_kbtu(prop, key):
-    if prop[f"{key}_sqft"] is not None and prop[f"{key}_eui"] is not None:
-        return round(prop[f"{key}_sqft"] * prop[f"{key}_eui"])

From acee59751992c5d8481ca476ee8702761699d3fd Mon Sep 17 00:00:00 2001
From: Ross Perry <ross.perry@Rosss-MacBook-Pro-2.local>
Date: Fri, 22 Nov 2024 11:00:39 -0700
Subject: [PATCH 4/6] precommit

---
 seed/utils/goals.py    | 36 ------------------------------------
 seed/views/v3/goals.py | 37 ++++++++++++++++++++++++++++++++++++-
 2 files changed, 36 insertions(+), 37 deletions(-)

diff --git a/seed/utils/goals.py b/seed/utils/goals.py
index 98a2d88493..0c18f22918 100644
--- a/seed/utils/goals.py
+++ b/seed/utils/goals.py
@@ -2,8 +2,6 @@
 SEED Platform (TM), Copyright (c) Alliance for Sustainable Energy, LLC, and other contributors.
 See also https://github.com/SEED-platform/seed/blob/main/LICENSE.md
 """
-import math
-from pint import Quantity
 
 from django.db.models import Case, F, FloatField, IntegerField, Prefetch, Sum, Value, When
 from django.db.models.fields.json import KeyTextTransform
@@ -216,37 +214,3 @@ def get_state_pairs(property_ids, goal_id):
         state_pairs.append({"property": property, "baseline": baseline_state, "current": current_state})
 
     return state_pairs
-
-def combine_properties(p1, p2):
-    if not p2:
-        return p1
-    combined = p1.copy()
-    for key, value in p2.items():
-        if value is not None:
-            combined[key] = value
-    return combined
-
-
-def percentage(a, b):
-    if not a or b is None:
-        return None
-    value = round(((a - b) / a) * 100)
-    return None if math.isnan(value) else value
-
-
-def get_preferred(p, columns):
-    if not p:
-        return
-    for col in columns:
-        return convert_quantity(p[col])
-
-
-def convert_quantity(value):
-    if isinstance(value, Quantity):
-        value = value.m
-    return value
-
-
-def get_kbtu(prop, key):
-    if prop[f"{key}_sqft"] is not None and prop[f"{key}_eui"] is not None:
-        return round(prop[f"{key}_sqft"] * prop[f"{key}_eui"])
diff --git a/seed/views/v3/goals.py b/seed/views/v3/goals.py
index 579689c022..d2df1053fb 100644
--- a/seed/views/v3/goals.py
+++ b/seed/views/v3/goals.py
@@ -9,6 +9,7 @@
 from django.db.utils import DataError
 from django.http import JsonResponse
 from django.utils.decorators import method_decorator
+from pint import Quantity
 from rest_framework import status
 from rest_framework.decorators import action
 
@@ -20,7 +21,7 @@
 from seed.utils.api import OrgMixin
 from seed.utils.api_schema import swagger_auto_schema_org_query_param
 from seed.utils.goal_notes import get_permission_data
-from seed.utils.goals import get_or_create_goal_notes, get_portfolio_summary, combine_properties, percentage, get_preferred, convert_quantity, get_kbtu
+from seed.utils.goals import get_or_create_goal_notes, get_portfolio_summary
 from seed.utils.search import FilterError, build_view_filters_and_sorts, filter_views_on_related
 from seed.utils.viewsets import ModelViewSetWithoutPatch
 
@@ -293,3 +294,37 @@ def data(self, request, pk):
             }
         )
 
+
+def combine_properties(p1, p2):
+    if not p2:
+        return p1
+    combined = p1.copy()
+    for key, value in p2.items():
+        if value is not None:
+            combined[key] = value
+    return combined
+
+
+def percentage(a, b):
+    if not a or b is None:
+        return None
+    value = round(((a - b) / a) * 100)
+    return None if math.isnan(value) else value
+
+
+def get_preferred(p, columns):
+    if not p:
+        return
+    for col in columns:
+        return convert_quantity(p[col])
+
+
+def convert_quantity(value):
+    if isinstance(value, Quantity):
+        value = value.m
+    return value
+
+
+def get_kbtu(prop, key):
+    if prop[f"{key}_sqft"] is not None and prop[f"{key}_eui"] is not None:
+        return round(prop[f"{key}_sqft"] * prop[f"{key}_eui"])

From 61613eaf40bcc313a6edc2a872d36f219114d18e Mon Sep 17 00:00:00 2001
From: Ross Perry <ross.perry@Rosss-MacBook-Pro-2.local>
Date: Fri, 22 Nov 2024 11:04:09 -0700
Subject: [PATCH 5/6] lint

---
 seed/static/seed/js/services/inventory_service.js | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/seed/static/seed/js/services/inventory_service.js b/seed/static/seed/js/services/inventory_service.js
index cd7eaba6fa..d916bcb5a3 100644
--- a/seed/static/seed/js/services/inventory_service.js
+++ b/seed/static/seed/js/services/inventory_service.js
@@ -64,7 +64,7 @@ angular.module('SEED.service.inventory', []).factory('inventory_service', [
       ids_only = null,
       shown_column_ids = null,
       access_level_instance_id = null,
-      include_property_ids = null,
+      include_property_ids = null
     ) => {
       organization_id = organization_id ?? user_service.get_organization().id;
 
@@ -101,7 +101,7 @@ angular.module('SEED.service.inventory', []).factory('inventory_service', [
         // Pass the current profile (if one exists) to limit the column data that is returned
         profile_id,
         // conditionally add optional params
-        ...(access_level_instance_id && { access_level_instance_id }),
+        ...(access_level_instance_id && { access_level_instance_id })
       };
       // add access_level_instance if it exists
 

From 868c004ed675b7fff48ebda5e8faf597edbf6854 Mon Sep 17 00:00:00 2001
From: Ross Perry <ross.perry@Rosss-MacBook-Pro-2.local>
Date: Tue, 3 Dec 2024 13:55:12 -0700
Subject: [PATCH 6/6] return if not none

---
 seed/views/v3/goals.py | 8 +++++---
 1 file changed, 5 insertions(+), 3 deletions(-)

diff --git a/seed/views/v3/goals.py b/seed/views/v3/goals.py
index d2df1053fb..a13a7fb7e6 100644
--- a/seed/views/v3/goals.py
+++ b/seed/views/v3/goals.py
@@ -312,11 +312,13 @@ def percentage(a, b):
     return None if math.isnan(value) else value
 
 
-def get_preferred(p, columns):
-    if not p:
+def get_preferred(prop, columns):
+    if not prop:
         return
     for col in columns:
-        return convert_quantity(p[col])
+        quantity = convert_quantity(prop[col])
+        if quantity is not None:
+            return quantity
 
 
 def convert_quantity(value):