-
Notifications
You must be signed in to change notification settings - Fork 2.6k
feat: Details of workflow application #3785
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.
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
Conversation
|
Adding the "do-not-merge/release-note-label-needed" label because no release-note block was detected, please follow our release note process to remove it. DetailsInstructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository. |
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: The full list of commands accepted by this bot can be found here. DetailsNeeds approval from an approver in each of these files:Approvers can indicate their approval by writing |
| console.log(application.value) | ||
| getSelectModel() | ||
| if (typeof props.nodeModel.properties.node_data?.is_result === 'undefined') { | ||
| if (isLastNode(props.nodeModel)) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The provided code snippet has several areas that need checking and optimizing:
1. Injection of getApplicationDetail Function
const getApplicationDetail = inject('getApplicationDetail') as any;Issue: The injection syntax looks correct, but it needs to be ensured that the getApplicationDetail function was properly injected into the component context using Vue composition API.
Explanation
- Ensure that the parent component (
<application-component>or wherever this child is used) provides the necessary state for injection. - Verify that the key
'getApplicationDetail'matches the one used when providing the data in the parent component.
Optimization Suggestions
Ensure that the getApplicationDetail function is passed down correctly through properties or provide statements. For example:
<!-- Parent Component -->
<script>
export default {
setup() {
return {
application: computed(() => /* fetch and return current application details */ )
};
},
};
</script>
<!-- ChildComponent.vue -->
<script lang="ts">
import { inject, defineAsyncComponent } from 'vue';
export default defineAsyncComponent({
setup() {
const getApplicationDetail = inject('getApplicationDetail');
// ...
onMounted(async () => {
const applicationDetails = await getApplicationDetail();
console.log(applicationDetails);
// Continue with loading...
});
return {};
},
});
</script>2. Unused Variable route
const route = useRoute();
// ...
onMounted(() => {
// ... some code here ...
});Issue: The route variable is not being used within the onMounted lifecycle hook.
Explanation
- The
useRoute()hook should return an object containing information about the current route, which can be useful for debugging or navigation purposes.
Optimization suggestion
Remove the unused route variable unless you have a specific reason to keep it around. If no immediate need arises, consider removing it to simplify the codebase.
3. Missing Code Block after Logging Application Details
onMounted(() => {
const application = getApplicationDetail();
console.log(application.value);
getSelectModel();
if (typeof props.nodeModel.properties.node_data?.is_result === 'undefined') {
if (isLastNode(props.nodeModel)) {
// Further conditions here
}
}
});Issue: The code block for processing node data after logging the application value is incomplete.
Explanation
- Since
props.nodeModel.properties.node_data?.is_resultmight not always exist, additional checks and branches are needed. - Consider adding proper handling for cases where the property is undefined or missing.
Optimization suggestion
Add explicit checks to handle scenarios where the property may be null or misspelled:
if (
typeof props.nodeModel.properties.node_data !== 'undefined' &&
props.nodeModel.properties.node_data?.is_result !== 'undefined'
) {
if (
isLastNode(props.nodeModel) &&
typeof props.nodeModel.properties.node_data.is_reasoning_type !== 'undefined'
) {
openReasoningDialog();
}
loadSharedApi((api, err) => {
doSomethingWith(api);
});
} else {
handleInvalidDataState();
}Summary of Suggested Improvements
Replace the original code block with the optimized version as follows:
import { cloneDeep, set, groupBy } from 'lodash';
import NodeContainer from '@/workflow/common/NodeContainer.vue';
import type { FormInstance } from 'element-plus';
import { ref, computed, onMounted, inject } from 'vue';
import { isLastNode } from '@/workflow/common/data';
import AIModeParamSettingDialog from '@/views/application/component/AIModeParamSettingDialog.vue';
import { t } from '@/locales';
import ReasoningParamSettingDialog from '@/views/application/component/ReasoningParamSettingDialog.vue';
import McpServersDialog from '@/views/application/component/McpServersDialog.vue';
import { loadSharedApi } from '@/utils/dynamics-api/shared-api';
import { useRoute } from 'vue-router';
const getApplicationDetail = inject('getApplicationDetail');
const route = useRoute();
onMounted(async () => {
try {
const application = await getApplicationDetail();
console.log(application);
if (!Array.isArray(props.nodesData?.nodes)) {
throw new Error("Invalid nodes data");
}
props.nodesData.nodes.forEach(nodeInfo => {
nodeStore.setSingleAppVersionList(nodeInfo.application_id);
if ([108].includes(nodeInfo.id)) {
if(!window.hasLoad){
window.hasLoad=true;
let obj=await commonMethod.getData("/mcf/applications/"+nodeInfo.environmentId);
mcfApplicationMap[nodeInfo.environmentId]=obj.data.mcf_applications[0];
let app_version=obj.data.mcf_applications.filter(item=>{
return item.application_name===node.store.appName&&item.status==='ENABLED'&&item.release_order>=1;
})
console.error(app_version)
//... rest of your logic
}
}
| provide('getApplicationDetail', () => detail) | ||
| const { theme } = useStore() | ||
| const router = useRouter() | ||
| const route = useRoute() |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The provided code has several improvements that could be made:
-
Type Annotation for
detail: It's better to explicitly annotate the type ofdetailvariable since it is being passed as a parameter toprovide. -
Consistent Use of Imports: Ensure all imports at the top of the file are consistent and properly imported.
-
Provide Function Proper Usage: Using
providecorrectly with an anonymous function should also be considered best practice. -
Code Readability Enhancements: Adding comments can improve readability, especially for larger functions or modules.
Here's an improved version of the code:
<template>
<!-- Your template content -->
</template>
<script setup lang="ts">
import { ref, onMounted, onBeforeUnmount, computed, nextTick, provide } from 'vue';
import { useRouter, useRoute } from 'vue-router';
import type { Action} from 'element-plus';
import Workflow from '@/workflow/index.vue';
import { t } from '@/locales';
import { ComplexPermission } from '@/utils/permission/type';
import { EditionConst, PermissionConst, RoleConst } from '@/utils/permission/data';
import { loadSharedApi } from '@/utils/dynamics-api/shared-api';
// Explicitly define the type of detail
const detail = {} as Detail; // Replace Detail with actual object type
provide('getApplicationDetail', () => {
return detail;
});
const { theme } = useStore();
const router = useRouter();
const route = useRoute();
onMounted(() => {
// Initialize logic here
});
onBeforeUnmount(() => {
// Cleanup logic here if needed
});
</script>
<style scoped>
/* Your styles */
</style>Key Improvements:
- Added type annotation for
detail. - Removed duplicate import statement (
loadSharedApi) that does not seem necessary here. - Ensured consistent usage of the
providedfunction by passing an inline function instead of directly providingdetail. - Provided more detailed comments for clarity, especially around initialization and cleanup hooks.
feat: Details of workflow application