From 62fadab614b5be5dfbf55582997ff522892bc3fa Mon Sep 17 00:00:00 2001 From: Ross Perry Date: Fri, 8 Nov 2024 10:32:16 -0700 Subject: [PATCH] sorting on related functional --- seed/models/data_quality.py | 8 -- .../portfolio_summary_controller.js | 27 +++--- 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 | 18 ++-- 7 files changed, 152 insertions(+), 90 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..7268664e8d 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,7 @@ angular.module('SEED.controller.portfolio_summary', []) enableSorting: false, minRowsToShow: 1, onRegisterApi: (gridApi) => { - console.log('registerd') + 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..754c7bd841 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 @@ -196,27 +196,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)