Skip to content

fix: fix github#1988 #1995

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Jan 8, 2025
Merged

fix: fix github#1988 #1995

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
31 changes: 26 additions & 5 deletions apps/application/serializers/application_serializers.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,7 @@
from common.constants.authentication_type import AuthenticationType
from common.db.search import get_dynamics_model, native_search, native_page_search
from common.db.sql_execute import select_list
from common.exception.app_exception import AppApiException, NotFound404, AppUnauthorizedFailed
from common.exception.app_exception import AppApiException, NotFound404, AppUnauthorizedFailed, ChatException
from common.field.common import UploadedImageField, UploadedFileField
from common.models.db_model_manage import DBModelManage
from common.response import result
Expand Down Expand Up @@ -268,7 +268,8 @@ def get_embed(self, with_valid=True, params=None):
float_location = application_setting.float_location
if application_setting.custom_theme is not None and len(
application_setting.custom_theme.get('header_font_color', 'rgb(100, 106, 115)')) > 0:
header_font_color = application_setting.custom_theme.get('header_font_color', 'rgb(100, 106, 115)')
header_font_color = application_setting.custom_theme.get('header_font_color',
'rgb(100, 106, 115)')

is_auth = 'true' if application_access_token is not None and application_access_token.is_active else 'false'
t = Template(content)
Expand Down Expand Up @@ -916,6 +917,12 @@ def profile(self, with_valid=True):
if application_access_token is None:
raise AppUnauthorizedFailed(500, "非法用户")
application_setting_model = DBModelManage.get_model('application_setting')
if application.type == ApplicationTypeChoices.WORK_FLOW:
work_flow_version = QuerySet(WorkFlowVersion).filter(application_id=application.id).order_by(
'-create_time')[0:1].first()
if work_flow_version is not None:
application.work_flow = work_flow_version.work_flow

xpack_cache = DBModelManage.get_model('xpack_cache')
X_PACK_LICENSE_IS_VALID = False if xpack_cache is None else xpack_cache.get('XPACK_LICENSE_IS_VALID', False)
application_setting_dict = {}
Expand Down Expand Up @@ -1150,9 +1157,23 @@ def application_list(self, with_valid=True):
def get_application(self, app_id, with_valid=True):
if with_valid:
self.is_valid(raise_exception=True)
application = QuerySet(Application).filter(id=self.data.get("application_id")).first()
return ApplicationSerializer.Operate(data={'user_id': application.user_id, 'application_id': app_id}).one(
with_valid=True)
if with_valid:
self.is_valid()
embed_application = QuerySet(Application).get(id=app_id)
if embed_application.type == ApplicationTypeChoices.WORK_FLOW:
work_flow_version = QuerySet(WorkFlowVersion).filter(application_id=embed_application.id).order_by(
'-create_time')[0:1].first()
if work_flow_version is not None:
embed_application.work_flow = work_flow_version.work_flow
dataset_list = self.list_dataset(with_valid=False)
mapping_dataset_id_list = [adm.dataset_id for adm in
QuerySet(ApplicationDatasetMapping).filter(application_id=app_id)]
dataset_id_list = [d.get('id') for d in
list(filter(lambda row: mapping_dataset_id_list.__contains__(row.get('id')),
dataset_list))]
self.update_search_node(embed_application.work_flow, [str(dataset.get('id')) for dataset in dataset_list])
return {**ApplicationSerializer.Query.reset_application(ApplicationSerializerModel(embed_application).data),
'dataset_id_list': dataset_id_list}

class ApplicationKeySerializerModel(serializers.ModelSerializer):
class Meta:
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The code provided appears to have several potential issues:

  1. Imports and Class Definitions: The imports at the top appear correct, but there might be unused import statements which could cause confusion.

  2. get_application Method:

    • If with_valid is set to True, it checks if the current user (self) is valid. However, there is no method named is_valid() defined within the ProfileAPI class.
    • For non-embedded applications (i.e., when with_valid is false), the code retrieves an application using its ID and then returns an updated version of it using a serializer. This is done correctly.

  3. Custom Theme Handling: There is unnecessary complexity with default font colors handled directly rather than using a function or template context.

  4. Work Flow Version Selection: The selection of the latest Work Flow version in embedded applications seems redundant since the method would only return the most recent version regardless of whether embedded or standalone.

  5. Database Queries: While the logic looks efficient, some queries can be optimized, particularly ensuring that related data like datasets is fetched concurrently where possible and avoiding nested calls like list_dataset.

Here is a brief refactored snippet with improvements mentioned above:

def update_search_node(work_flow_id, dataset_ids):
    # Assuming update_search_node handles this operation efficiently
    pass

class ProfileAPI(APIViewBase):

    @staticmethod
    def get_embed(with_valid=True, params=None):
        # Ensure proper error handling
        if params is None:
            params = {}
        
        # Get other necessary data here...

    @staticmethod
    def get_application(app_id, with_valid=True):
        if with_valid:
            raise AppUnauthorizedFailed(500, "非法用户")  # Correct exception raising
        
        try:
            embed_application = QuerySet(Application).get(id=app_id)
            
            if app_instance.type == ApplicationTypeChoices.WORK_FLOW:
                versions_query_set = query_work_flow_versions(embed_application.id).latest()
                embed_application.work_flow = versions_query_set.workflow
            
            dataset_list = self.list_dataset(with_valid=False)
            mapping_dataset_id_list = QuerySet(ApplicationDatasetMapping).filter(application_id=app_id)
            dataset_id_list = [d['id'] for d in filter(lambda x: x in map(str, mapping_dataset_id_list), dataset_list)]

            embed_app_data = ApplicationSerializer.Operate(data={'user_id': embed_application.user_id, 'application_id': app_id})
            updated_app_data = {**embed_app_data.one(), 'dataset_id_list': dataset_id_list}

            return updated_app_data
        except DoesNotExist:
            raise NotFound404()

# Helper functions omitted for brevity

This should improve readability and reduce redundancy while maintaining core functionality.

Expand Down
18 changes: 14 additions & 4 deletions ui/src/workflow/nodes/application-node/index.vue
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -202,7 +202,6 @@ const update_field = () => {
.then((ok) => {
const old_api_input_field_list = props.nodeModel.properties.node_data.api_input_field_list
const old_user_input_field_list = props.nodeModel.properties.node_data.user_input_field_list

if (isWorkFlow(ok.data.type)) {
const nodeData = ok.data.work_flow.nodes[0].properties.node_data
const new_api_input_field_list = ok.data.work_flow.nodes[0].properties.api_input_field_list
Expand All @@ -213,7 +212,13 @@ const update_field = () => {
(old_item: any) => old_item.variable == item.variable
)
if (find_field) {
return { ...item, default_value: JSON.parse(JSON.stringify(find_field.default_value)) }
return {
...item,
default_value: JSON.parse(JSON.stringify(find_field.default_value)),
value: JSON.parse(JSON.stringify(find_field.value)),
label:
typeof item.label === 'object' && item.label != null ? item.label.label : item.label
}
} else {
return item
}
Expand All @@ -228,12 +233,17 @@ const update_field = () => {
(old_item: any) => old_item.field == item.field
)
if (find_field) {
return { ...item, default_value: JSON.parse(JSON.stringify(find_field.default_value)) }
return {
...item,
default_value: JSON.parse(JSON.stringify(find_field.default_value)),
value: JSON.parse(JSON.stringify(find_field.value)),
label:
typeof item.label === 'object' && item.label != null ? item.label.label : item.label
}
} else {
return item
}
})
console.log(merge_user_input_field_list)
set(
props.nodeModel.properties.node_data,
'user_input_field_list',
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The provided code snippet seems to be part of a React component handling updates for node data fields, specifically focusing on input variables. Here are some observations and suggestions:

  1. Variable Initialization: The function update_field is defined within a context where it might not have access to all required dependencies. Ensure that props.nodeModel, isWorkFlow, ok.data.work_flow.nodes, etc., are properly initialized.

  2. JSON Parsing: When dealing with stringified JSON objects (e.g., default_value, value), ensure that you handle any potential errors or exceptions during deserialization. Consider using .catch() or type casting to manage these cases.

  3. Type Checking: In the logic that handles different field types (api_input_field_list and user_input_field_list), ensure that the variable names (variable) used in comparisons match the actual keys in the JSON structure. This could lead to bugs if they do not align.

  4. Null Value Handling: For labels, which can also be stringified json, consider adding checks to avoid undefined values when extracting the label property.

  5. Optimization: If the list operations involve many items, consider using more efficient methods like array spreads instead of creating new arrays at each iteration.

Here's an updated version of the code incorporating these considerations:

const update_field = () => {
  return Ok.ok({
    properties: [
      {
        // Update your payload according to your application requirements here
        node_data: {
          api_input_field_list: [...],
          user_input_field_list: [...]
        }
      }
    ]
  });
};

// Example call:
Ok.then(update_field).then((response) => {
  const { node_model } = response.payload;
  
  if (node_model) {
    let merged_user_input = [...node_model.properties[node_model.name_of_node]];
    
    // Assuming you want to merge based on certain conditions
    merged_user_input.forEach(item => {
      const findFieldApiList = old_api_input_field_list.find(oldItem => 
        newItem.variable === item.variable
      );

      if (findFieldApiList) {
        item.default_value = JSON.parse(JSON.stringify(findFieldApiList.default_value));
        
        if ('value' in findFieldApiList && typeof findFieldApiList.value !== 'string') {
          try {
            item.value = JSON.parse(JSON.stringify(findFieldApiList.value)); // Add error handling
          } catch (error) {
            console.error('Error parsing value:', error);
          }
        }

        if (
          (typeof item.label === 'object' || typeof item.label === 'array') &&
          item.label?.label !== undefined
        ) {
          item.label = item.label.label; // Extract label from object or array
        }
      } else {
        item.default_value = '';
        item.value = ''; // Initialize or set appropriate defaults
      }
      
      // Merge similarly for other lists...
    });

    node_model.properties[node_model.name_of_node] = merged_user_input;

    console.log(node_model); // Log the updated model
  } else {
    console.error("Node model was not found.");
  }
}).catch(error => {
  console.error('There was a problem updating fields!', error.message);
});

Key Changes:

  • Added initial checks for old_api_input_field_list and node_model.
  • Ensured proper handling of JSON.stringify to prevent circular references.
  • Applied similar checks and treatments to both api_input_field_list and user_input_field_list.
  • Included basic error handling for default_value parsing.
  • Used logical operators correctly and ensured consistent naming conventions.

Expand Down
Loading