Merge pull request #1129 from hinashi/fixed/join_modal_slow

Fixed slow display of join_attr modal

Author: userlocalhost

apiclient/typescript-fetch/package.json

@@ -1,6 +1,6 @@
 {
   "name": "@dmm-com/airone-apiclient-typescript-fetch",
-  "version": "0.0.13",
+  "version": "0.0.14",
   "description": "AirOne APIv2 client in TypeScript",
   "main": "src/autogenerated/index.ts",
   "scripts": {

entity/api_v2/serializers.py

@@ -194,7 +194,7 @@ class EntityAttrSerializer(serializers.ModelSerializer):
 
     class Meta:
         model = EntityAttr
-        fields = ("id", "name", "type", "referral")
+        fields = ("id", "name", "type")
 
 
 class EntitySerializer(serializers.ModelSerializer):
@@ -649,4 +649,4 @@ def _do_import(resource, iter_data: Any):
 
 
 class EntityAttrNameSerializer(serializers.ListSerializer):
-    child = EntityAttrSerializer()
+    child = serializers.CharField()

entity/api_v2/views.py

@@ -1,7 +1,6 @@
 from distutils.util import strtobool
-from typing import Dict, List
 
-from django.db.models import Count, F
+from django.db.models import F
 from django.http import Http404
 from django.http.response import HttpResponse, JsonResponse
 from django_filters.rest_framework import DjangoFilterBackend
@@ -13,7 +12,6 @@
 from rest_framework.permissions import BasePermission, IsAuthenticated
 from rest_framework.request import Request
 from rest_framework.response import Response
-from rest_framework.serializers import Serializer
 
 from airone.lib.acl import ACLType, get_permitted_objects
 from airone.lib.drf import ObjectNotExistsError, YAMLParser, YAMLRenderer
@@ -263,44 +261,40 @@ def get_object(self) -> dict:
 @extend_schema(
     parameters=[
         OpenApiParameter("entity_ids", OpenApiTypes.STR, OpenApiParameter.QUERY),
+        OpenApiParameter("referral_attr", OpenApiTypes.STR, OpenApiParameter.QUERY),
     ],
 )
 class EntityAttrNameAPI(generics.GenericAPIView):
     serializer_class = EntityAttrNameSerializer
 
     def get_queryset(self):
         entity_ids = list(filter(None, self.request.query_params.get("entity_ids", "").split(",")))
+        referral_attr = self.request.query_params.get("referral_attr")
 
-        if len(entity_ids) == 0:
-            return EntityAttr.objects.filter(is_active=True)
-
-        else:
+        entity_attrs = EntityAttr.objects.filter(is_active=True)
+        if len(entity_ids) != 0:
             entities = Entity.objects.filter(id__in=entity_ids, is_active=True)
             if len(entity_ids) != len(entities):
                 # the case invalid entity-id was specified
                 raise ValidationError("Target Entity doesn't exist")
-            else:
-                # filter only names appear in all specified entities
-                return EntityAttr.objects.filter(parent_entity__in=entities, is_active=True)
+
+            # filter only names appear in all specified entities
+            entity_attrs = entity_attrs.filter(parent_entity__in=entities)
+
+        if referral_attr:
+            referral_entity_ids = (
+                entity_attrs.filter(name=referral_attr)
+                .exclude(referral__id__isnull=True)
+                .values_list("referral", flat=True)
+                .distinct()
+            )
+            entity_attrs = EntityAttr.objects.filter(
+                parent_entity__in=referral_entity_ids, is_active=True
+            )
+
+        return entity_attrs.values_list("name", flat=True).order_by("name").distinct()
 
     def get(self, request: Request) -> Response:
         queryset = self.get_queryset()
-
-        entity_ids: List[int] = list(
-            filter(None, self.request.query_params.get("entity_ids", "").split(","))
-        )
-        names_query = queryset.values("name")
-        if len(entity_ids) > 0:
-            names_query = names_query.annotate(count=Count("name")).filter(count=len(entity_ids))
-
-        # Compile each attribute of referrals by attribute name
-        serializer: Serializer = self.get_serializer(queryset)
-        results: Dict[str, Dict] = {}
-        for attrname in names_query.values_list("name", flat=True):
-            for attrinfo in [x for x in serializer.data if x["name"] == attrname]:
-                if attrname in results:
-                    results[attrname]["referral"] += attrinfo["referral"]
-                else:
-                    results[attrname] = attrinfo
-
-        return Response(results.values())
+        serializer: serializers.Serializer = self.get_serializer(queryset)
+        return Response(serializer.data)

entity/tests/test_api_v2.py

@@ -3473,6 +3473,10 @@ def test_export(self):
     def test_get_entity_attr_names(self):
         user = self.admin_login()
 
+        self.ref_entity.delete()
+        self.entity.attrs.all().delete()
+        self.entity.delete()
+
         entity_info = {
             "test_entity1": ["foo", "bar", "fuga"],
             "test_entity2": ["bar", "hoge", "fuga"],
@@ -3495,26 +3499,37 @@ def test_get_entity_attr_names(self):
 
                 entity.attrs.add(attr)
 
-        self.ref_entity.delete()
-        self.entity.attrs.all().delete()
-        self.entity.delete()
+        entities = Entity.objects.filter(name__contains="test_entity")
+
+        entity3 = self.create_entity(
+            user,
+            "test_entity3",
+            [
+                {
+                    "name": "puyo",
+                    "type": AttrTypeValue["object"],
+                    "ref": entities.first(),
+                }
+            ],
+        )
 
         # get partially
-        entities = Entity.objects.filter(name__contains="test_entity")
         resp = self.client.get(
-            "/entity/api/v2/attrs?entity_ids=%s" % ",".join([str(x.id) for x in entities])
+            "/entity/api/v2/attrs?entity_ids=%s" % ",".join([str(x.id) for x in entities[:2]])
         )
         self.assertEqual(resp.status_code, 200)
-        self.assertEqual([x["name"] for x in resp.json()], sorted(["bar", "fuga"]))
-        self.assertEqual(
-            [x["referral"] for x in resp.json() if x["type"] == AttrType.OBJECT][0],
-            list(entities.values_list("id", flat=True)),
+        self.assertEqual(resp.json(), ["bar", "foo", "fuga", "hoge"])
+
+        resp = self.client.get(
+            "/entity/api/v2/attrs?entity_ids=%s&referral_attr=%s" % (entity3.id, "puyo")
         )
+        self.assertEqual(resp.status_code, 200)
+        self.assertEqual(resp.json(), ["bar", "foo", "fuga"])
 
         # get all attribute infomations are returned collectly
         resp = self.client.get("/entity/api/v2/attrs")
         self.assertEqual(resp.status_code, 200)
-        self.assertEqual([x["name"] for x in resp.json()], ["foo", "bar", "fuga", "hoge"])
+        self.assertEqual(resp.json(), ["bar", "foo", "fuga", "hoge", "puyo"])
 
         # invalid entity_id(s)
         resp = self.client.get("/entity/api/v2/attrs?entity_ids=9999")

frontend/src/components/entry/AdvancedSearchJoinModal.tsx

@@ -12,35 +12,36 @@ import { aironeApiClient } from "repository/AironeApiClient";
 import { formatAdvancedSearchParams } from "services/entry/AdvancedSearch";
 
 interface Props {
+  targetEntityIds: number[];
+  searchAllEntities: boolean;
   targetAttrname: string;
-  referralIds: number[] | undefined;
   joinAttrs: AdvancedSearchJoinAttrInfo[];
-  setJoinAttrname: (name: string) => void;
+  handleClose: () => void;
 }
 
 export const AdvancedSearchJoinModal: FC<Props> = ({
+  targetEntityIds,
+  searchAllEntities,
   targetAttrname,
-  referralIds,
   joinAttrs,
-  setJoinAttrname,
+  handleClose,
 }) => {
   const history = useHistory();
   // This is join attributes that have been already been selected before.
   const currentAttrInfo: AdvancedSearchJoinAttrInfo | undefined =
     joinAttrs.find((attr) => attr.name === targetAttrname);
 
-  const [selectedAttrNames, setSelectedAttrNames] = useState<string[]>([]);
+  const [selectedAttrNames, setSelectedAttrNames] = useState<Array<string>>(
+    currentAttrInfo?.attrinfo.map((attr) => attr.name) ?? []
+  );
 
   const referralAttrs = useAsyncWithThrow(async () => {
-    if (referralIds !== undefined && referralIds.length > 0) {
-      return await aironeApiClient.getEntityAttrs(referralIds);
-    }
-    return [];
-  }, [referralIds]);
-
-  const closeModal = () => {
-    setJoinAttrname("");
-  };
+    return await aironeApiClient.getEntityAttrs(
+      targetEntityIds,
+      searchAllEntities,
+      targetAttrname
+    );
+  }, [targetEntityIds, searchAllEntities, targetAttrname]);
 
   const handleUpdatePageURL = () => {
     // to prevent duplication of same name parameter
@@ -71,19 +72,18 @@ export const AdvancedSearchJoinModal: FC<Props> = ({
       pathname: location.pathname,
       search: "?" + params.toString(),
     });
-    history.go(0);
   };
 
   return (
     <AironeModal
       title={"結合するアイテムの属性名"}
       open={targetAttrname !== ""}
-      onClose={() => closeModal()}
+      onClose={handleClose}
     >
       <Autocomplete
-        options={referralAttrs.value?.map((x) => x.name) || []}
-        defaultValue={currentAttrInfo?.attrinfo.map((x) => x.name) || []}
-        onChange={(_, value: string[]) => {
+        options={referralAttrs.value ?? []}
+        value={selectedAttrNames}
+        onChange={(_, value: Array<string>) => {
           setSelectedAttrNames(value);
         }}
         renderInput={(params) => (
@@ -105,7 +105,7 @@ export const AdvancedSearchJoinModal: FC<Props> = ({
           variant="outlined"
           color="primary"
           sx={{ mx: "4px" }}
-          onClick={() => closeModal()}
+          onClick={handleClose}
         >
           キャンセル
         </Button>

frontend/src/components/entry/SearchResults.test.tsx

@@ -25,9 +25,9 @@ test("should render a component with essential props", function () {
           /* do nothing */
         }}
         hasReferral={false}
-        setJoinAttrname={() => {
-          /* do nothing */
-        }}
+        entityIds={[]}
+        joinAttrs={[]}
+        searchAllEntities={false}
       />,
       { wrapper: TestWrapper }
     )

frontend/src/components/entry/SearchResults.tsx

@@ -1,4 +1,7 @@
-import { AdvancedSearchResult } from "@dmm-com/airone-apiclient-typescript-fetch";
+import {
+  AdvancedSearchJoinAttrInfo,
+  AdvancedSearchResult,
+} from "@dmm-com/airone-apiclient-typescript-fetch";
 import {
   Box,
   Checkbox,
@@ -13,7 +16,7 @@ import {
   Typography,
 } from "@mui/material";
 import { styled } from "@mui/material/styles";
-import React, { Dispatch, FC, SetStateAction, useMemo } from "react";
+import React, { FC, useMemo } from "react";
 import { Link } from "react-router-dom";
 
 import { SearchResultsTableHead } from "./SearchResultsTableHead";
@@ -55,7 +58,9 @@ interface Props {
   defaultAttrsFilter?: AttrsFilter;
   bulkOperationEntryIds: Array<number>;
   handleChangeBulkOperationEntryId: (id: number, checked: boolean) => void;
-  setJoinAttrname: Dispatch<SetStateAction<string>>;
+  entityIds: number[];
+  searchAllEntities: boolean;
+  joinAttrs: AdvancedSearchJoinAttrInfo[];
 }
 
 export const SearchResults: FC<Props> = ({
@@ -68,7 +73,9 @@ export const SearchResults: FC<Props> = ({
   defaultAttrsFilter = {},
   bulkOperationEntryIds,
   handleChangeBulkOperationEntryId,
-  setJoinAttrname,
+  entityIds,
+  searchAllEntities,
+  joinAttrs,
 }) => {
   // NOTE attrTypes are guessed by the first element on the results. So if it has no appropriate attr,
   // the type guess doesn't work well. We should improve attr type API if more accurate type is needed.
@@ -94,7 +101,9 @@ export const SearchResults: FC<Props> = ({
               defaultEntryFilter={defaultEntryFilter}
               defaultReferralFilter={defaultReferralFilter}
               defaultAttrsFilter={defaultAttrsFilter}
-              setJoinAttrname={setJoinAttrname}
+              entityIds={entityIds}
+              searchAllEntities={searchAllEntities}
+              joinAttrs={joinAttrs}
             />
             <TableBody>
               {results.values?.map((result) => (