korbit-ai[bot] commented on code in PR #32882:
URL: https://github.com/apache/superset/pull/32882#discussion_r2042127834


##########
superset-frontend/src/views/routes.tsx:
##########
@@ -129,6 +134,10 @@
   () => import(/* webpackChunkName: "RolesList" */ 'src/pages/RolesList'),
 );
 
+const UsersList: LazyExoticComponent<any> = lazy(

Review Comment:
   ### Unsafe UsersList Component Type <sub>![category 
Functionality](https://img.shields.io/badge/Functionality-0284c7)</sub>
   
   <details>
     <summary>Tell me more</summary>
   
   ###### What is the issue?
   The UsersList component is typed with 'any', which bypasses TypeScript's 
type checking for the component's props and return type.
   
   ###### Why this matters
   Using 'any' could hide potential runtime errors if the UsersList component's 
interface changes, leading to potential crashes or unexpected behavior in 
production.
   
   ###### Suggested change ∙ *Feature Preview*
   Define a proper interface for the UsersList component and use it in the type 
declaration:
   ```typescript
   interface UsersListProps {
     // define required props here
   }
   
   const UsersList: LazyExoticComponent<ComponentType<UsersListProps>> = lazy(
     () => import(/* webpackChunkName: "UsersList" */ 'src/pages/UsersList'),
   );
   ```
   
   
   ###### Provide feedback to improve future suggestions
   [![Nice 
Catch](https://img.shields.io/badge/👍%20Nice%20Catch-71BC78)](https://app.korbit.ai/feedback/aa91ff46-6083-4491-9416-b83dd1994b51/edc31ade-4c84-4ce6-9b71-844cc7636875/upvote)
 
[![Incorrect](https://img.shields.io/badge/👎%20Incorrect-white)](https://app.korbit.ai/feedback/aa91ff46-6083-4491-9416-b83dd1994b51/edc31ade-4c84-4ce6-9b71-844cc7636875?what_not_true=true)
  [![Not in 
Scope](https://img.shields.io/badge/👎%20Out%20of%20PR%20scope-white)](https://app.korbit.ai/feedback/aa91ff46-6083-4491-9416-b83dd1994b51/edc31ade-4c84-4ce6-9b71-844cc7636875?what_out_of_scope=true)
 [![Not in coding 
standard](https://img.shields.io/badge/👎%20Not%20in%20our%20standards-white)](https://app.korbit.ai/feedback/aa91ff46-6083-4491-9416-b83dd1994b51/edc31ade-4c84-4ce6-9b71-844cc7636875?what_not_in_standard=true)
 
[![Other](https://img.shields.io/badge/👎%20Other-white)](https://app.korbit.ai/feedback/aa91ff46-6083-4491-9416-b83dd1994b51/edc31ade-4c84-4ce6-9b71-844cc7636875)
   </details>
   
   <sub>
   
   💬 Looking for more details? Reply to this comment to chat with Korbit.
   </sub>
   
   <!--- korbi internal id:72de2a3d-5e1d-4313-923a-79779854dfec -->
   
   
   [](72de2a3d-5e1d-4313-923a-79779854dfec)



##########
superset-frontend/src/components/ListView/types.ts:
##########
@@ -74,7 +78,8 @@ export type InnerFilterValue =
   | undefined
   | string[]
   | number[]
-  | { label: string; value: string | number };
+  | { label: string; value: string | number }
+  | [number | null, number | null];

Review Comment:
   ### Ambiguous Numerical Range Type <sub>![category 
Readability](https://img.shields.io/badge/Readability-0284c7)</sub>
   
   <details>
     <summary>Tell me more</summary>
   
   ###### What is the issue?
   The numerical_range filter type was added but its InnerFilterValue type uses 
an array tuple without clear semantics for minimum and maximum values.
   
   ###### Why this matters
   Using a generic tuple type makes the code less self-documenting and 
increases the chance of mixing up the min/max order in the application code, 
potentially leading to incorrect filter behavior.
   
   ###### Suggested change ∙ *Feature Preview*
   Create a dedicated type for numerical range values to make the code more 
maintainable and self-documenting:
   ```typescript
   interface NumericalRange {
     min: number | null;
     max: number | null;
   }
   
   export type InnerFilterValue =
     | string
     | boolean
     | number
     | null
     | undefined
     | string[]
     | number[]
     | { label: string; value: string | number }
     | NumericalRange;
   ```
   
   
   ###### Provide feedback to improve future suggestions
   [![Nice 
Catch](https://img.shields.io/badge/👍%20Nice%20Catch-71BC78)](https://app.korbit.ai/feedback/aa91ff46-6083-4491-9416-b83dd1994b51/58f406a1-d4b2-4136-bac2-d884ee6a8582/upvote)
 
[![Incorrect](https://img.shields.io/badge/👎%20Incorrect-white)](https://app.korbit.ai/feedback/aa91ff46-6083-4491-9416-b83dd1994b51/58f406a1-d4b2-4136-bac2-d884ee6a8582?what_not_true=true)
  [![Not in 
Scope](https://img.shields.io/badge/👎%20Out%20of%20PR%20scope-white)](https://app.korbit.ai/feedback/aa91ff46-6083-4491-9416-b83dd1994b51/58f406a1-d4b2-4136-bac2-d884ee6a8582?what_out_of_scope=true)
 [![Not in coding 
standard](https://img.shields.io/badge/👎%20Not%20in%20our%20standards-white)](https://app.korbit.ai/feedback/aa91ff46-6083-4491-9416-b83dd1994b51/58f406a1-d4b2-4136-bac2-d884ee6a8582?what_not_in_standard=true)
 
[![Other](https://img.shields.io/badge/👎%20Other-white)](https://app.korbit.ai/feedback/aa91ff46-6083-4491-9416-b83dd1994b51/58f406a1-d4b2-4136-bac2-d884ee6a8582)
   </details>
   
   <sub>
   
   💬 Looking for more details? Reply to this comment to chat with Korbit.
   </sub>
   
   <!--- korbi internal id:f4ef2f9a-c697-4ef4-9899-75c20471119a -->
   
   
   [](f4ef2f9a-c697-4ef4-9899-75c20471119a)



##########
superset-frontend/src/features/users/utils.ts:
##########
@@ -0,0 +1,43 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *   http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied.  See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+import { SupersetClient } from '@superset-ui/core';
+import { FormValues } from './types';
+
+export const createUser = async (values: FormValues) => {
+  const { confirmPassword, ...payload } = values;
+  if (payload.active == null) {
+    payload.active = false;
+  }
+  await SupersetClient.post({
+    endpoint: '/api/v1/security/users/',
+    jsonPayload: { ...payload },
+  });
+};
+
+export const updateUser = async (user_Id: number, values: FormValues) => {
+  await SupersetClient.put({
+    endpoint: `/api/v1/security/users/${user_Id}`,
+    jsonPayload: { ...values },
+  });

Review Comment:
   ### Missing Error Propagation in User Update <sub>![category Error 
Handling](https://img.shields.io/badge/Error%20Handling-ea580c)</sub>
   
   <details>
     <summary>Tell me more</summary>
   
   ###### What is the issue?
   The updateUser function doesn't handle potential API errors which could 
occur during the update operation.
   
   ###### Why this matters
   Silent failures in user updates can lead to data inconsistency and poor user 
experience.
   
   ###### Suggested change ∙ *Feature Preview*
   ```typescript
   try {
     await SupersetClient.put({
       endpoint: `/api/v1/security/users/${user_Id}`,
       jsonPayload: { ...values },
     });
   } catch (error) {
     throw new Error(`Failed to update user ${user_Id}: ${error.message || 
error}`);
   }
   ```
   
   
   ###### Provide feedback to improve future suggestions
   [![Nice 
Catch](https://img.shields.io/badge/👍%20Nice%20Catch-71BC78)](https://app.korbit.ai/feedback/aa91ff46-6083-4491-9416-b83dd1994b51/e5168472-6f75-4b46-b489-67142357f893/upvote)
 
[![Incorrect](https://img.shields.io/badge/👎%20Incorrect-white)](https://app.korbit.ai/feedback/aa91ff46-6083-4491-9416-b83dd1994b51/e5168472-6f75-4b46-b489-67142357f893?what_not_true=true)
  [![Not in 
Scope](https://img.shields.io/badge/👎%20Out%20of%20PR%20scope-white)](https://app.korbit.ai/feedback/aa91ff46-6083-4491-9416-b83dd1994b51/e5168472-6f75-4b46-b489-67142357f893?what_out_of_scope=true)
 [![Not in coding 
standard](https://img.shields.io/badge/👎%20Not%20in%20our%20standards-white)](https://app.korbit.ai/feedback/aa91ff46-6083-4491-9416-b83dd1994b51/e5168472-6f75-4b46-b489-67142357f893?what_not_in_standard=true)
 
[![Other](https://img.shields.io/badge/👎%20Other-white)](https://app.korbit.ai/feedback/aa91ff46-6083-4491-9416-b83dd1994b51/e5168472-6f75-4b46-b489-67142357f893)
   </details>
   
   <sub>
   
   💬 Looking for more details? Reply to this comment to chat with Korbit.
   </sub>
   
   <!--- korbi internal id:c6a57fc0-9d15-4458-bac8-10fb211a2dcf -->
   
   
   [](c6a57fc0-9d15-4458-bac8-10fb211a2dcf)



##########
superset-frontend/src/features/users/utils.ts:
##########
@@ -0,0 +1,43 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *   http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied.  See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+import { SupersetClient } from '@superset-ui/core';
+import { FormValues } from './types';
+
+export const createUser = async (values: FormValues) => {
+  const { confirmPassword, ...payload } = values;
+  if (payload.active == null) {
+    payload.active = false;
+  }
+  await SupersetClient.post({
+    endpoint: '/api/v1/security/users/',
+    jsonPayload: { ...payload },
+  });

Review Comment:
   ### Missing Error Propagation in User Creation <sub>![category Error 
Handling](https://img.shields.io/badge/Error%20Handling-ea580c)</sub>
   
   <details>
     <summary>Tell me more</summary>
   
   ###### What is the issue?
   The createUser function catches no errors from the API call which could fail 
due to network issues, validation errors, or server problems.
   
   ###### Why this matters
   When API calls fail silently, debugging becomes difficult and the UI can't 
properly inform users about failures.
   
   ###### Suggested change ∙ *Feature Preview*
   ```typescript
   try {
     await SupersetClient.post({
       endpoint: '/api/v1/security/users/',
       jsonPayload: { ...payload },
     });
   } catch (error) {
     throw new Error(`Failed to create user: ${error.message || error}`);
   }
   ```
   
   
   ###### Provide feedback to improve future suggestions
   [![Nice 
Catch](https://img.shields.io/badge/👍%20Nice%20Catch-71BC78)](https://app.korbit.ai/feedback/aa91ff46-6083-4491-9416-b83dd1994b51/0b16ffe3-65ef-4a83-bbdb-b70f66dbbe71/upvote)
 
[![Incorrect](https://img.shields.io/badge/👎%20Incorrect-white)](https://app.korbit.ai/feedback/aa91ff46-6083-4491-9416-b83dd1994b51/0b16ffe3-65ef-4a83-bbdb-b70f66dbbe71?what_not_true=true)
  [![Not in 
Scope](https://img.shields.io/badge/👎%20Out%20of%20PR%20scope-white)](https://app.korbit.ai/feedback/aa91ff46-6083-4491-9416-b83dd1994b51/0b16ffe3-65ef-4a83-bbdb-b70f66dbbe71?what_out_of_scope=true)
 [![Not in coding 
standard](https://img.shields.io/badge/👎%20Not%20in%20our%20standards-white)](https://app.korbit.ai/feedback/aa91ff46-6083-4491-9416-b83dd1994b51/0b16ffe3-65ef-4a83-bbdb-b70f66dbbe71?what_not_in_standard=true)
 
[![Other](https://img.shields.io/badge/👎%20Other-white)](https://app.korbit.ai/feedback/aa91ff46-6083-4491-9416-b83dd1994b51/0b16ffe3-65ef-4a83-bbdb-b70f66dbbe71)
   </details>
   
   <sub>
   
   💬 Looking for more details? Reply to this comment to chat with Korbit.
   </sub>
   
   <!--- korbi internal id:153b6a03-a5b9-4342-b646-c7249effa2a2 -->
   
   
   [](153b6a03-a5b9-4342-b646-c7249effa2a2)



##########
superset-frontend/src/features/users/utils.ts:
##########
@@ -0,0 +1,43 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *   http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied.  See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+import { SupersetClient } from '@superset-ui/core';
+import { FormValues } from './types';
+
+export const createUser = async (values: FormValues) => {
+  const { confirmPassword, ...payload } = values;
+  if (payload.active == null) {
+    payload.active = false;
+  }
+  await SupersetClient.post({
+    endpoint: '/api/v1/security/users/',
+    jsonPayload: { ...payload },
+  });
+};
+
+export const updateUser = async (user_Id: number, values: FormValues) => {
+  await SupersetClient.put({
+    endpoint: `/api/v1/security/users/${user_Id}`,
+    jsonPayload: { ...values },

Review Comment:
   ### Unnecessary Object Spread in updateUser <sub>![category 
Performance](https://img.shields.io/badge/Performance-4f46e5)</sub>
   
   <details>
     <summary>Tell me more</summary>
   
   ###### What is the issue?
   Unnecessary object spread operator used in updateUser when passing values 
object directly would suffice.
   
   ###### Why this matters
   Creating a new object through spread when the original object could be used 
directly adds unnecessary object creation overhead.
   
   ###### Suggested change ∙ *Feature Preview*
   Remove the spread operator and pass values directly:
   ```typescript
   jsonPayload: values,
   ```
   
   
   ###### Provide feedback to improve future suggestions
   [![Nice 
Catch](https://img.shields.io/badge/👍%20Nice%20Catch-71BC78)](https://app.korbit.ai/feedback/aa91ff46-6083-4491-9416-b83dd1994b51/48294880-1e8b-42c4-bead-fc998941d6c8/upvote)
 
[![Incorrect](https://img.shields.io/badge/👎%20Incorrect-white)](https://app.korbit.ai/feedback/aa91ff46-6083-4491-9416-b83dd1994b51/48294880-1e8b-42c4-bead-fc998941d6c8?what_not_true=true)
  [![Not in 
Scope](https://img.shields.io/badge/👎%20Out%20of%20PR%20scope-white)](https://app.korbit.ai/feedback/aa91ff46-6083-4491-9416-b83dd1994b51/48294880-1e8b-42c4-bead-fc998941d6c8?what_out_of_scope=true)
 [![Not in coding 
standard](https://img.shields.io/badge/👎%20Not%20in%20our%20standards-white)](https://app.korbit.ai/feedback/aa91ff46-6083-4491-9416-b83dd1994b51/48294880-1e8b-42c4-bead-fc998941d6c8?what_not_in_standard=true)
 
[![Other](https://img.shields.io/badge/👎%20Other-white)](https://app.korbit.ai/feedback/aa91ff46-6083-4491-9416-b83dd1994b51/48294880-1e8b-42c4-bead-fc998941d6c8)
   </details>
   
   <sub>
   
   💬 Looking for more details? Reply to this comment to chat with Korbit.
   </sub>
   
   <!--- korbi internal id:aedf1c18-2be8-476e-a2c5-97e26ec13ee6 -->
   
   
   [](aedf1c18-2be8-476e-a2c5-97e26ec13ee6)



##########
superset-frontend/src/features/users/utils.ts:
##########
@@ -0,0 +1,43 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *   http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied.  See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+import { SupersetClient } from '@superset-ui/core';
+import { FormValues } from './types';
+
+export const createUser = async (values: FormValues) => {
+  const { confirmPassword, ...payload } = values;
+  if (payload.active == null) {
+    payload.active = false;
+  }
+  await SupersetClient.post({
+    endpoint: '/api/v1/security/users/',
+    jsonPayload: { ...payload },

Review Comment:
   ### Unnecessary Object Spread <sub>![category 
Performance](https://img.shields.io/badge/Performance-4f46e5)</sub>
   
   <details>
     <summary>Tell me more</summary>
   
   ###### What is the issue?
   Unnecessary object spread operator used when passing the payload object 
directly would suffice.
   
   ###### Why this matters
   Creating a new object through spread when the original object could be used 
directly adds unnecessary object creation overhead.
   
   ###### Suggested change ∙ *Feature Preview*
   Remove the spread operator and pass the payload directly:
   ```typescript
   jsonPayload: payload,
   ```
   
   
   ###### Provide feedback to improve future suggestions
   [![Nice 
Catch](https://img.shields.io/badge/👍%20Nice%20Catch-71BC78)](https://app.korbit.ai/feedback/aa91ff46-6083-4491-9416-b83dd1994b51/66fa3585-7176-4433-be47-3359ac8e3841/upvote)
 
[![Incorrect](https://img.shields.io/badge/👎%20Incorrect-white)](https://app.korbit.ai/feedback/aa91ff46-6083-4491-9416-b83dd1994b51/66fa3585-7176-4433-be47-3359ac8e3841?what_not_true=true)
  [![Not in 
Scope](https://img.shields.io/badge/👎%20Out%20of%20PR%20scope-white)](https://app.korbit.ai/feedback/aa91ff46-6083-4491-9416-b83dd1994b51/66fa3585-7176-4433-be47-3359ac8e3841?what_out_of_scope=true)
 [![Not in coding 
standard](https://img.shields.io/badge/👎%20Not%20in%20our%20standards-white)](https://app.korbit.ai/feedback/aa91ff46-6083-4491-9416-b83dd1994b51/66fa3585-7176-4433-be47-3359ac8e3841?what_not_in_standard=true)
 
[![Other](https://img.shields.io/badge/👎%20Other-white)](https://app.korbit.ai/feedback/aa91ff46-6083-4491-9416-b83dd1994b51/66fa3585-7176-4433-be47-3359ac8e3841)
   </details>
   
   <sub>
   
   💬 Looking for more details? Reply to this comment to chat with Korbit.
   </sub>
   
   <!--- korbi internal id:0af09862-5efc-4720-a4a7-7715f31a8ada -->
   
   
   [](0af09862-5efc-4720-a4a7-7715f31a8ada)



##########
superset-frontend/src/views/routes.tsx:
##########
@@ -129,6 +134,10 @@ const RolesList = lazy(
   () => import(/* webpackChunkName: "RolesList" */ 'src/pages/RolesList'),
 );
 
+const UsersList: LazyExoticComponent<any> = lazy(
+  () => import(/* webpackChunkName: "UsersList" */ 'src/pages/UsersList'),
+);

Review Comment:
   ### Unsafe Type Annotation <sub>![category 
Performance](https://img.shields.io/badge/Performance-4f46e5)</sub>
   
   <details>
     <summary>Tell me more</summary>
   
   ###### What is the issue?
   Using <any> type annotation in LazyExoticComponent defeats TypeScript's type 
checking benefits and could mask potential runtime errors.
   
   ###### Why this matters
   Type safety issues may not be caught during development, potentially leading 
to runtime errors and decreased code reliability.
   
   ###### Suggested change ∙ *Feature Preview*
   Define proper type interface for the UsersList component and use it in the 
lazy loading:
   ```typescript
   interface UsersListProps {
     // define required props here
   }
   
   const UsersList: LazyExoticComponent<ComponentType<UsersListProps>> = lazy(
     () => import('src/pages/UsersList')
   );
   ```
   
   
   ###### Provide feedback to improve future suggestions
   [![Nice 
Catch](https://img.shields.io/badge/👍%20Nice%20Catch-71BC78)](https://app.korbit.ai/feedback/aa91ff46-6083-4491-9416-b83dd1994b51/386ed16f-26f5-4dbd-b3b3-f6a78540a394/upvote)
 
[![Incorrect](https://img.shields.io/badge/👎%20Incorrect-white)](https://app.korbit.ai/feedback/aa91ff46-6083-4491-9416-b83dd1994b51/386ed16f-26f5-4dbd-b3b3-f6a78540a394?what_not_true=true)
  [![Not in 
Scope](https://img.shields.io/badge/👎%20Out%20of%20PR%20scope-white)](https://app.korbit.ai/feedback/aa91ff46-6083-4491-9416-b83dd1994b51/386ed16f-26f5-4dbd-b3b3-f6a78540a394?what_out_of_scope=true)
 [![Not in coding 
standard](https://img.shields.io/badge/👎%20Not%20in%20our%20standards-white)](https://app.korbit.ai/feedback/aa91ff46-6083-4491-9416-b83dd1994b51/386ed16f-26f5-4dbd-b3b3-f6a78540a394?what_not_in_standard=true)
 
[![Other](https://img.shields.io/badge/👎%20Other-white)](https://app.korbit.ai/feedback/aa91ff46-6083-4491-9416-b83dd1994b51/386ed16f-26f5-4dbd-b3b3-f6a78540a394)
   </details>
   
   <sub>
   
   💬 Looking for more details? Reply to this comment to chat with Korbit.
   </sub>
   
   <!--- korbi internal id:cc310807-42b1-48cf-8cf8-e9256c4e518c -->
   
   
   [](cc310807-42b1-48cf-8cf8-e9256c4e518c)



##########
superset-frontend/src/features/users/utils.ts:
##########
@@ -0,0 +1,43 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *   http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied.  See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+import { SupersetClient } from '@superset-ui/core';
+import { FormValues } from './types';
+
+export const createUser = async (values: FormValues) => {
+  const { confirmPassword, ...payload } = values;
+  if (payload.active == null) {
+    payload.active = false;
+  }
+  await SupersetClient.post({
+    endpoint: '/api/v1/security/users/',
+    jsonPayload: { ...payload },
+  });
+};
+
+export const updateUser = async (user_Id: number, values: FormValues) => {

Review Comment:
   ### Inconsistent Parameter Naming Convention <sub>![category 
Readability](https://img.shields.io/badge/Readability-0284c7)</sub>
   
   <details>
     <summary>Tell me more</summary>
   
   ###### What is the issue?
   The parameter 'user_Id' uses inconsistent naming convention with snake_case 
instead of camelCase, which is the TypeScript standard.
   
   ###### Why this matters
   Inconsistent naming conventions make the code harder to read and violate 
TypeScript style guidelines, potentially confusing developers about parameter 
naming patterns.
   
   ###### Suggested change ∙ *Feature Preview*
   ```typescript
   export const updateUser = async (userId: number, values: FormValues)
   ```
   
   
   ###### Provide feedback to improve future suggestions
   [![Nice 
Catch](https://img.shields.io/badge/👍%20Nice%20Catch-71BC78)](https://app.korbit.ai/feedback/aa91ff46-6083-4491-9416-b83dd1994b51/38c17d59-c1c3-4888-a7e6-fa03003ae232/upvote)
 
[![Incorrect](https://img.shields.io/badge/👎%20Incorrect-white)](https://app.korbit.ai/feedback/aa91ff46-6083-4491-9416-b83dd1994b51/38c17d59-c1c3-4888-a7e6-fa03003ae232?what_not_true=true)
  [![Not in 
Scope](https://img.shields.io/badge/👎%20Out%20of%20PR%20scope-white)](https://app.korbit.ai/feedback/aa91ff46-6083-4491-9416-b83dd1994b51/38c17d59-c1c3-4888-a7e6-fa03003ae232?what_out_of_scope=true)
 [![Not in coding 
standard](https://img.shields.io/badge/👎%20Not%20in%20our%20standards-white)](https://app.korbit.ai/feedback/aa91ff46-6083-4491-9416-b83dd1994b51/38c17d59-c1c3-4888-a7e6-fa03003ae232?what_not_in_standard=true)
 
[![Other](https://img.shields.io/badge/👎%20Other-white)](https://app.korbit.ai/feedback/aa91ff46-6083-4491-9416-b83dd1994b51/38c17d59-c1c3-4888-a7e6-fa03003ae232)
   </details>
   
   <sub>
   
   💬 Looking for more details? Reply to this comment to chat with Korbit.
   </sub>
   
   <!--- korbi internal id:9936a61b-67cc-43b9-8ca9-221c905f09db -->
   
   
   [](9936a61b-67cc-43b9-8ca9-221c905f09db)



##########
superset-frontend/src/features/users/utils.ts:
##########
@@ -0,0 +1,43 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *   http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied.  See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+import { SupersetClient } from '@superset-ui/core';
+import { FormValues } from './types';
+
+export const createUser = async (values: FormValues) => {
+  const { confirmPassword, ...payload } = values;
+  if (payload.active == null) {
+    payload.active = false;
+  }
+  await SupersetClient.post({
+    endpoint: '/api/v1/security/users/',
+    jsonPayload: { ...payload },
+  });
+};
+
+export const updateUser = async (user_Id: number, values: FormValues) => {
+  await SupersetClient.put({
+    endpoint: `/api/v1/security/users/${user_Id}`,
+    jsonPayload: { ...values },
+  });
+};

Review Comment:
   ### Unnecessary Password Confirmation in Update Payload <sub>![category 
Functionality](https://img.shields.io/badge/Functionality-0284c7)</sub>
   
   <details>
     <summary>Tell me more</summary>
   
   ###### What is the issue?
   The updateUser function sends the entire form values including the 
confirmPassword field to the API, which is unnecessary and potentially 
problematic.
   
   ###### Why this matters
   Sending the confirmPassword field to the API is redundant and could cause 
issues if the API is not properly sanitizing the input. This also unnecessarily 
increases the payload size.
   
   ###### Suggested change ∙ *Feature Preview*
   Remove confirmPassword from the update payload similar to the createUser 
function:
   ```typescript
   export const updateUser = async (user_Id: number, values: FormValues) => {
     const { confirmPassword, ...payload } = values;
     await SupersetClient.put({
       endpoint: `/api/v1/security/users/${user_Id}`,
       jsonPayload: payload,
     });
   };
   ```
   
   
   ###### Provide feedback to improve future suggestions
   [![Nice 
Catch](https://img.shields.io/badge/👍%20Nice%20Catch-71BC78)](https://app.korbit.ai/feedback/aa91ff46-6083-4491-9416-b83dd1994b51/72b42265-894c-4406-aec1-7852f0d7ee84/upvote)
 
[![Incorrect](https://img.shields.io/badge/👎%20Incorrect-white)](https://app.korbit.ai/feedback/aa91ff46-6083-4491-9416-b83dd1994b51/72b42265-894c-4406-aec1-7852f0d7ee84?what_not_true=true)
  [![Not in 
Scope](https://img.shields.io/badge/👎%20Out%20of%20PR%20scope-white)](https://app.korbit.ai/feedback/aa91ff46-6083-4491-9416-b83dd1994b51/72b42265-894c-4406-aec1-7852f0d7ee84?what_out_of_scope=true)
 [![Not in coding 
standard](https://img.shields.io/badge/👎%20Not%20in%20our%20standards-white)](https://app.korbit.ai/feedback/aa91ff46-6083-4491-9416-b83dd1994b51/72b42265-894c-4406-aec1-7852f0d7ee84?what_not_in_standard=true)
 
[![Other](https://img.shields.io/badge/👎%20Other-white)](https://app.korbit.ai/feedback/aa91ff46-6083-4491-9416-b83dd1994b51/72b42265-894c-4406-aec1-7852f0d7ee84)
   </details>
   
   <sub>
   
   💬 Looking for more details? Reply to this comment to chat with Korbit.
   </sub>
   
   <!--- korbi internal id:5a53e2e0-9548-4236-8ed3-41ad907611bd -->
   
   
   [](5a53e2e0-9548-4236-8ed3-41ad907611bd)



##########
superset-frontend/src/features/users/utils.ts:
##########
@@ -0,0 +1,43 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *   http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied.  See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+import { SupersetClient } from '@superset-ui/core';
+import { FormValues } from './types';
+
+export const createUser = async (values: FormValues) => {
+  const { confirmPassword, ...payload } = values;
+  if (payload.active == null) {
+    payload.active = false;
+  }

Review Comment:
   ### Loose Equality Check <sub>![category 
Readability](https://img.shields.io/badge/Readability-0284c7)</sub>
   
   <details>
     <summary>Tell me more</summary>
   
   ###### What is the issue?
   The null check uses loose equality (==) instead of strict equality (===), 
which is less explicit and readable.
   
   ###### Why this matters
   Using loose equality makes the code's intent less clear as it doesn't 
explicitly show which values are being checked for, potentially leading to 
unexpected behavior.
   
   ###### Suggested change ∙ *Feature Preview*
   ```typescript
   if (payload.active === null || payload.active === undefined) {
     payload.active = false;
   }
   ```
   
   
   ###### Provide feedback to improve future suggestions
   [![Nice 
Catch](https://img.shields.io/badge/👍%20Nice%20Catch-71BC78)](https://app.korbit.ai/feedback/aa91ff46-6083-4491-9416-b83dd1994b51/8d775dcd-413c-455f-84ae-6a493dc283a8/upvote)
 
[![Incorrect](https://img.shields.io/badge/👎%20Incorrect-white)](https://app.korbit.ai/feedback/aa91ff46-6083-4491-9416-b83dd1994b51/8d775dcd-413c-455f-84ae-6a493dc283a8?what_not_true=true)
  [![Not in 
Scope](https://img.shields.io/badge/👎%20Out%20of%20PR%20scope-white)](https://app.korbit.ai/feedback/aa91ff46-6083-4491-9416-b83dd1994b51/8d775dcd-413c-455f-84ae-6a493dc283a8?what_out_of_scope=true)
 [![Not in coding 
standard](https://img.shields.io/badge/👎%20Not%20in%20our%20standards-white)](https://app.korbit.ai/feedback/aa91ff46-6083-4491-9416-b83dd1994b51/8d775dcd-413c-455f-84ae-6a493dc283a8?what_not_in_standard=true)
 
[![Other](https://img.shields.io/badge/👎%20Other-white)](https://app.korbit.ai/feedback/aa91ff46-6083-4491-9416-b83dd1994b51/8d775dcd-413c-455f-84ae-6a493dc283a8)
   </details>
   
   <sub>
   
   💬 Looking for more details? Reply to this comment to chat with Korbit.
   </sub>
   
   <!--- korbi internal id:da63c1cd-3e1d-4e66-8ede-45db7239b609 -->
   
   
   [](da63c1cd-3e1d-4e66-8ede-45db7239b609)



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]


---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]

Reply via email to