Pull request update/250205

c6d5bbb OSN-275. Update error message alignment daf0563 OSN-601. Update chart legend anchor position to top-right 7f33866 OSN-229. Add validation for user_id on creating recipient + fixed swagger
hystax · Feb 5, 2025 · 3a73eb9 · 3a73eb9 · nitin-invincibleOcean · Feb 6, 2025
2 parents 51e09ee + c6d5bbb
commit 3a73eb9
Show file tree

Hide file tree

Showing 8 changed files with 82 additions and 28 deletions.
diff --git a/katara/katara_service/controllers/recipient.py b/katara/katara_service/controllers/recipient.py
@@ -1,11 +1,13 @@
 import json
 import logging
+from requests.exceptions import HTTPError
 from sqlalchemy import and_, exists
 
 from tools.optscale_exceptions.common_exc import (
     ConflictException,
     WrongArgumentsException,
 )
+from optscale_client.auth_client.client_v2 import Client as AuthClient
 from optscale_client.rest_api_client.client_v2 import Client as RestClient
 
 from katara.katara_service.controllers.base import BaseController
@@ -23,11 +25,19 @@
 class RecipientController(BaseController):
     def __init__(self, db_session=None, config=None, engine=None):
         super().__init__(db_session, config, engine)
+        self._auth_cl = None
         self._rest_cl = None
 
     def _get_model_type(self):
         return Recipient
 
+    @property
+    def auth_cl(self):
+        if self._auth_cl is None:
+            self._auth_cl = AuthClient(url=self._config.auth_url(),
+                                       secret=self._config.cluster_secret())
+        return self._auth_cl
+
     @property
     def rest_cl(self):
         if self._rest_cl is None:
@@ -61,6 +71,12 @@ def _validate(self, recipient, is_new=True, **kwargs):
                 RolePurpose(recipient.role_purpose)
             except ValueError:
                 raise WrongArgumentsException(Err.OKA0025, ["role_purpose"])
+        if recipient.user_id:
+            try:
+                _, user = self.auth_cl.user_get(recipient.user_id)
+            except HTTPError:
+                raise WrongArgumentsException(Err.OKA0004,
+                                              ["User", recipient.user_id])
 
     def delete(self, **kwargs):
         payload_dict = kwargs.pop("payload")

diff --git a/katara/katara_service/handlers/v2/recipients.py b/katara/katara_service/handlers/v2/recipients.py
@@ -84,7 +84,7 @@ async def patch(self, recipient_id, **kwargs):
             Modifies a recipient with specified id \n\n
             Required permission: CLUSTER_SECRET
         parameters:
-        -   name: id
+        -   name: recipient_id
             in: path
             description: Recipient ID
             required: true
@@ -264,6 +264,7 @@ async def post(self, **url_params):
             400:
                 description: |
                     Wrong arguments:
+                    - OKA0004: User does not exist
                     - OKA0009: Incorrect request body received
                     - OKA0012: Unexpected parameters
                     - OKA0016: Incorrect meta format

diff --git a/katara/katara_service/tests/unittests/test_api_base.py b/katara/katara_service/tests/unittests/test_api_base.py
@@ -37,6 +37,7 @@ def db_session(self):
     def setUp(self):
         super().setUp()
         secret = gen_id()
+        patch('optscale_client.config_client.client.Client.auth_url').start()
         patch('optscale_client.config_client.client.Client.cluster_secret',
               return_value=secret).start()
         http_provider = FetchMethodHttpProvider(self.fetch, rethrow=False)

diff --git a/katara/katara_service/tests/unittests/test_api_recipient.py b/katara/katara_service/tests/unittests/test_api_recipient.py
@@ -1,6 +1,8 @@
 import json
 import uuid
-
+from unittest.mock import patch
+from requests.exceptions import HTTPError
+from requests.models import Response
 from katara.katara_service.tests.unittests.test_api_base import TestBase
 
 
@@ -90,8 +92,11 @@ def test_recipient_create(self):
         self.assertEqual(recipient['scope_id'], payload['scope_id'])
 
     def test_recipient_create_user_id(self):
+        user_id = str(uuid.uuid4())
+        patch('optscale_client.auth_client.client_v2.Client.user_get',
+              return_value=(200, {'user_id': user_id})).start()
         payload = {
-            "user_id": str(uuid.uuid4()),
+            "user_id": user_id,
             "scope_id": str(uuid.uuid4()),
         }
         code, recipient = self.client.recipient_create(**payload)
@@ -100,6 +105,23 @@ def test_recipient_create_user_id(self):
         self.assertEqual(recipient['user_id'], payload['user_id'])
         self.assertEqual(recipient['scope_id'], payload['scope_id'])
 
+    def test_recipient_create_user_id_not_exists(self):
+        def raise_404(*_args, **_kwargs):
+            err = HTTPError('User not found')
+            err.response = Response()
+            err.response.status_code = 404
+            raise err
+
+        patch('optscale_client.auth_client.client_v2.Client.user_get',
+              side_effect=raise_404).start()
+        payload = {
+            "user_id": str(uuid.uuid4()),
+            "scope_id": str(uuid.uuid4()),
+        }
+        code, response = self.client.recipient_create(**payload)
+        self.assertEqual(code, 400)
+        self.assertEqual(response['error']['error_code'], 'OKA0004')
+
     def test_recipient_create_unassigned(self):
         payload = {
             "scope_id": str(uuid.uuid4()),

diff --git a/ngui/ui/src/components/forms/CreateS3DuplicateFinderCheckForm/FormElements/BucketsField.tsx b/ngui/ui/src/components/forms/CreateS3DuplicateFinderCheckForm/FormElements/BucketsField.tsx
@@ -16,7 +16,12 @@ const FIELD_NAME = FIELD_NAMES.BUCKETS;
 
 const MAX_SELECTED_BUCKETS = 100;
 
-const TableField = ({ buckets, value, dataSources, onChange, errors }) => {
+const VALIDATION_TYPE = {
+  AT_LEAST_ONE_SELECTED: "atLeastOneSelected",
+  MAX_BUCKETS: "maxBuckets"
+} as const;
+
+const TableField = ({ buckets, value, dataSources, onChange }) => {
   const tableData = useMemo(
     () =>
       buckets.map((bucket) => {
@@ -54,25 +59,22 @@ const TableField = ({ buckets, value, dataSources, onChange, errors }) => {
   );
 
   return (
-    <>
-      <Table
-        columns={columns}
-        withSearch
-        enableSearchQueryParam={false}
-        data={tableData}
-        memoBodyCells
-        withSelection
-        rowSelection={value}
-        getRowId={(row) => getCloudResourceIdentifier(row)}
-        onRowSelectionChange={onChange}
-        pageSize={10}
-        enablePaginationQueryParam={false}
-        localization={{
-          emptyMessageId: "noBuckets"
-        }}
-      />
-      {!!errors[FIELD_NAME] && <FormHelperText error>{errors[FIELD_NAME].message}</FormHelperText>}
-    </>
+    <Table
+      columns={columns}
+      withSearch
+      enableSearchQueryParam={false}
+      data={tableData}
+      memoBodyCells
+      withSelection
+      rowSelection={value}
+      getRowId={(row) => getCloudResourceIdentifier(row)}
+      onRowSelectionChange={onChange}
+      pageSize={10}
+      enablePaginationQueryParam={false}
+      localization={{
+        emptyMessageId: "noBuckets"
+      }}
+    />
   );
 };
 
@@ -102,13 +104,13 @@ const BucketsField = ({ buckets, dataSources, isLoading }) => {
         name={FIELD_NAME}
         rules={{
           validate: {
-            atLeastOneSelected: (value) =>
+            [VALIDATION_TYPE.AT_LEAST_ONE_SELECTED]: (value) =>
               isEmptyObject(value)
                 ? intl.formatMessage({
                     id: "atLeastOneBucketMustBeSelected"
                   })
                 : true,
-            maxBuckets: (value) => {
+            [VALIDATION_TYPE.MAX_BUCKETS]: (value) => {
               const bucketsCount = Object.keys(value).length;
               return bucketsCount > MAX_SELECTED_BUCKETS
                 ? intl.formatMessage(
@@ -137,6 +139,7 @@ const BucketsField = ({ buckets, dataSources, isLoading }) => {
                   <strong>{Object.keys(selectedBuckets).join(", ")}</strong>
                 </Typography>
               )}
+              {!!errors[FIELD_NAME] && <FormHelperText error>{errors[FIELD_NAME].message}</FormHelperText>}
             </>
           )
         }

diff --git a/ngui/ui/src/containers/AddInstanceToScheduleContainer/AddInstanceToScheduleContainer.tsx b/ngui/ui/src/containers/AddInstanceToScheduleContainer/AddInstanceToScheduleContainer.tsx
@@ -18,6 +18,11 @@ import {
 } from "utils/constants";
 import { getXDaysAgoRange } from "utils/datetime";
 
+type AddInstanceToScheduleContainerProps = {
+  powerScheduleId: string;
+  handleClose: () => void;
+};
+
 const INSTANCES_COUNT_LIMIT = 5000;
 
 const useGetInstances = (range, selectedDataSourceIds, appliedFilters) => {
@@ -46,7 +51,7 @@ const useGetInstances = (range, selectedDataSourceIds, appliedFilters) => {
   };
 };
 
-const useGetAvailableFilter = (range) => {
+const useGetAvailableFilter = (range: { start: number; end: number }) => {
   const { start, end } = range;
 
   const { useGet: useGetFilters } = AvailableFiltersService();
@@ -65,7 +70,7 @@ const useGetAvailableFilter = (range) => {
   };
 };
 
-const AddInstanceToScheduleContainer = ({ powerScheduleId, handleClose }) => {
+const AddInstanceToScheduleContainer = ({ powerScheduleId, handleClose }: AddInstanceToScheduleContainerProps) => {
   const { useAttachInstancesToSchedule } = PowerScheduleService();
 
   const { onAttach, isLoading: isAttachLoading } = useAttachInstancesToSchedule();

diff --git a/ngui/ui/src/theme.ts b/ngui/ui/src/theme.ts
@@ -393,6 +393,12 @@ const getThemeConfig = (settings = {}) => {
       MuiFormHelperText: {
         defaultProps: {
           margin: "dense"
+        },
+        styleOverrides: {
+          contained: () => ({
+            marginLeft: 0,
+            marginRight: 0
+          })
         }
       },
       MuiIconButton: {

diff --git a/ngui/ui/src/utils/constants.ts b/ngui/ui/src/utils/constants.ts
@@ -552,7 +552,7 @@ export const DEFAULT_LINE_CHART_MARGIN = Object.freeze({ top: 20, right: 35, lef
 export const CHART_LEGEND_WIDTH = 200;
 
 export const CHART_LEGEND_LAYOUT_SETTINGS = {
-  anchor: "bottom-right",
+  anchor: "top-right",
   direction: "column",
   translateX: 8,
   itemWidth: 0,