-
Notifications
You must be signed in to change notification settings - Fork 2.6k
feat: Knowledge workflow permission #4348
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 |
| hit_test: () => false, | ||
| debug: (source_id: string) => true, | ||
| } | ||
| export default share |
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.
Code Review
The provided code snippet appears to be an implementation of permission checks related to various functionalities within a shared knowledge management system. Here’s a summary of some observations:
1. Duplicate Function Definitions:
- The function definitions have been slightly modified, but they essentially repeat identical logic with only minor syntax changes (
=>,||instead of commas). Ensure consistency in handling and return types.
2. Use of Permissions Array:
- Instead of using individual permissions directly as arguments, this seems like a more scalable approach when multiple roles need access to similar operations. This makes it easier to manage updates.
3. Debug Function:
- A new function
debug(source_id: string): booleanis added, which returnstrue. There might be no logical purpose for this unless it's intended to expose internal debugging information. Consider adding comments or logging around the functionality if necessary.
4. Code Formatting:
- The code formatting could benefit from some standardization in indentation and spacing for clarity.
5. Consistency:
- Ensure that all functions use consistent naming conventions and patterns throughout the file.
Optimization Suggestions:
- If there are any duplicate conditions across functions, consider combining them into a helper utility function. This reduces redundancy and makes the code cleaner.
Example Refactored Code Snippet:
import { ComplexPermission } from '@/utils/permission/type';
import { EditionConst, PermissionConst, RoleConst } from '@/utils/permission/data';
const hasPermission = (roles: RoleConst[], operation: 'AND' | 'OR') => {
// Simulate checking user has one of the given roles
return roles.some(role =>
['ADMIN', PermissionConst.SHARED_KNOWLEDGE_CREATE, ...OtherRoles].includes(role)
);
};
const share = {
is_share: () => false,
create: () => hasPermission([RoleConst.ADMIN, PermissionConst.SHARED_KNOWLEDGE_CREATE], 'OR'),
sync: () => hasPermission([RoleConst.ADMIN, PermissionConst.SHARED_KNOWLEDGE_SYNC], 'OR'),
vector: () => hasPermission([RoleConst.ADMIN, PermissionConst.SHARED_KNOWLEDGE_VECTOR], 'OR'),
generate: () => hasPermission([RoleConst.ADMIN, PermissionConst.SHARED_KNOWLEDGE_GENERATE], 'OR'),
edit: () => hasPermission([RoleConst.ADMIN, PermissionConst.SHARED_KNOWLEDGE_EDIT], 'OR'),
export: () => hasPermission([RoleConst.ADMIN, PermissionConst.SHARED_KNOWLEDGE_EXPORT], 'OR'),
delete: () => hasPermission([RoleConst.ADMIN, PermissionConst.SHARED_KNOWLEDGE_DELETE], 'OR'),
doc_read: () => false,
doc_create: () => hasPermission([RoleConst.ADMIN, PermissionConst.SHARED_KNOWLEDGE_DOCUMENT_CREATE], 'OR'),
doc_vector: () => hasPermission([RoleConst.ADMIN, PermissionConst.SHARED_KNOWLEDGE_DOCUMENT_VECTOR], 'OR'),
doc_generate: () => hasPermission([RoleConst.ADMIN, PermissionConst.SHARED_KNOWLEDGE_DOCUMENT_GENERATE], 'OR'),
doc_migrate: () => hasPermission([RoleConst.ADMIN, PermissionConst.SHARED_KNOWLEDGE_DOCUMENT_MIGRATE], 'OR'),
doc_edit: () => hasPermission([RoleConst.ADMIN, PermissionConst.SHARED_KNOWLEDGE_DOCUMENT_EDIT], 'OR'),
doc_sync: () => hasPermission([RoleConst.ADMIN, PermissionConst.SHARED_KNOWLEDGE_DOCUMENT_SYNC], 'OR'),
doc_delete: () => hasPermission([RoleConst.ADMIN, PermissionConst.SHARED_KNOWLEDGE_DOCUMENT_DELETE], 'OR'),
doc_export: () => hasPermission([RoleConst.ADMIN, PermissionConst.SHARED_KNOWLEDGE_DOCUMENT_EXPORT], 'OR'),
doc_download: (
source_id: string
) => hasPermission(
[RoleConst.ADMIN, PermissionConst.SHARED_KNOWLEDGE_DOCUMENT_DOWNLOAD_SOURCE_FILE],
'OR'
),
doc_tag: () => hasPermission([RoleConst.ADMIN, PermissionConst.SHARED_KNOWLEDGE_DOCUMENT_TAG], 'OR'),
doc_replace: () => hasPermission([RoleConst.ADMIN, PermissionConst.SHARED_KNOWLEDGE_DOCUMENT_REPLACE], 'OR'),
problem_create: () => hasPermission([RoleConst.ADMIN, PermissionConst.SHARED_KNOWLEDGE_PROBLEM_CREATE], 'OR'),
knowledge_chat_user_read: () => false,
knowledge_chat_user_edit: () => hasPermission([RoleConst.ADMIN, PermissionConst.SHARED_KNOWLEDGE_CHAT_USER_EDIT], 'OR'),
problem_read: () => hasPermission([RoleConst.ADMIN, PermissionConst.SHARED_KNOWLEDGE_PROBLEM_READ], 'OR'),
problem_relate: () => hasPermission([RoleConst.ADMIN, PermissionConst.SHARED_KNOWLEDGE_PROBLEM_RELATE], 'OR'),
problem_delete: () => hasPermission([RoleConst.ADMIN, PermissionConst.SHARED_KNOWLEDGE_PROBLEM_DELETE], 'OR'),
problem_edit: () => hasPermission([RoleConst.ADMIN, PermissionConst.SHARED_KNOWLEDGE_PROBLEM_EDIT], 'OR'),
tag_read: () => hasPermission([RoleConst.ADMIN, PermissionConst.SHARED_KNOWLEDGE_TAG_READ], 'OR'),
tag_create: () => hasPermission([RoleConst.ADMIN, PermissionConst.SHARED_KNOWLEDGE_TAG_CREATE], 'OR'),
tag_edit: () => hasPermission([RoleConst.ADMIN, PermissionConst.SHARED_KNOWLEDGE_TAG_EDIT], 'OR'),
tag_delete: () => hasPermission([RoleConst.ADMIN, PermissionConst.SHARED_KNOWLEDGE_TAG_DELETE], 'OR'),
chat_user_edit: () => false,
auth: () => false,
folderRead: () => false,
folderWrite: () => false,
folderAuth: () => false,
folderDelete: () => false,
hit_test: () => false,
debug: (source_id: string) => true,
};
export default share;This version consolidates common permissions into a single helper function called hasPermission, reducing redundancy and improving readability. Additionally, the debug function remains intact without further modification.
| }) | ||
| </script> | ||
|
|
||
| <style scoped lang="scss"></style> |
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 Vue3 component looks mostly correct but there are a few areas that could be improved:
-
Conditional Element Visibility: The
UserFieldFormDialogis only shown when needed (v-if="open". It should also manage visibility properly. -
Event Handling for Table Row Edits: Instead of directly using
$indexto update state, consider passing the row object itself into the modify dialog. -
Table Column Width Adjustments: The widths might not be optimal considering screen sizes and readability. Ensure they are responsive.
-
Code Comments: Adding comments can help clarify complex logic flow or decisions.
-
Error Handling in Input Type Check and Default Values: Consider adding more robust checks for handling different input types and providing meaningful error messages.
Here’s an updated version with improvements:
@@ -0,0 +1,237 @@
<template>
<div class="flex-between mb-16">
<h5 class="break-all ellipsis lighter" style="max-width: 80%" :title="inputFieldConfig.title">
{{ inputFieldConfig.title }}
</h5>
<div>
<el-button type="primary" link @click="openChangeTitleDialog">
<AppIcon iconName="app-setting"></AppIcon>
</el-button>
<span class="ml-4">
<el-button link type="primary" @click="addNewInputField()">
<AppIcon iconName="app-add-outlined" class="mr-4"></AppIcon>
{{ $t('common.add') }}
</el-button>
</span>
</div>
</div>
<el-table v-if="inputFields.length > 0" :data="inputFields" class="mb-16" ref="tableRef" :row-key="'field'" border>
<el-table-column prop="field" label=$t('dynamicsForm.paramForm.field.label')"> </el-table-column>
<el-table-column prop="name" label="$t('dynamicsForm.paramForm.name.label')">
<template #default="{ row }">
<el-input placeholder="Enter field name..." v-model="currentEditName" />
</template>
</el-table-column>
<el-table-column prop="type" label="$t('dynamicsForm.paramForm.input_type.label')" width="95">
<template #default="{ row }">
<el-select v-model="selectedType" placeholder="Select input type...">
<el-option :key="option.value" :label="option.text" :value="option.value" />
</el-select>
</template>
</el-table-column>
<el-table-column prop="default_value" label="$t('dynamicsForm.default.label')">
<template #default="{ row }">
<el-input placeholder="Enter default value... " v-model="editDefault" />
</template>
</el-table-column >
<el-table-column prop="required" :label="$t('common.required')" width="80%">
<template #default="{ row,index}">
<el-switch disabled size="small" v-model="row.required"/>
</template>
</el-table-column>
<el-table-column prop="actions" align="right" fixed="right" width="140px">
<template #default="{ row,index}">
<el-tooltip effect="dark" content="$t('common.edit')" placement="top">
<el-button type="success" text size="mini" @click.stop="startEditingRow(row, index)"> Edit</el-button>
</el-tooltip>
<el-tooltip effect="dark" content="$t('common.delete')" placement="top">
<el-popconfirm
title="$t('views.applicationWorkflow.tip.confirmDeleteParam')"
confirmButtonText="Yes"
cancelButtonText="No"
icon-color="#f60"
@confirm="handleDelete(row)"
>
<template #reference>
<el-button type="danger" text size="mini"> Delete</el-button></template
>
</el-popconfirm>
</template>
</el-table-column>
</el-table>
<el-dialog v-model="editVisible" title="Edit Field" width="50%">
<el-form :model="editedItem" :rules="rules">
<el-form-item :label="$t('dynamicsForm.paramForm.field.label')" prop="field">
<el-input v-model="editedItem.field"></el-input>
</el-form-item>
<el-form-item :label="$t('dynamicsForm.paramForm.name.label')" prop="name">
<el-input v-model="editedItem.name"></el-input>
</el-form-item>
<el-form-item :label="$t('dynamicsForm.paramForm.input_type.label')" prop="type">
<el-select v-model="editedItem.type">
<el-option key="text" label="Text" value="Text"/>
<el-option key='password' label="Password" value='Password'/>
...
</el-select>
</el-form-item>
<!-- Add other form items similarly -->
<el-form-item>
<el-button type="primary" @click="submitEdit">Submit</el-button>
<el-button @click="cancelEditClick">Cancel</el-button>
</el-form-item>
</el-form>
</el-dialog>
<UserInputTitleDialog ref="UserInputTitleDialogRef" @refresh="refreshFieldConfig" />
</template>
<script setup lang="ts">
import { ref, reactive, watch } from 'vue';
import AppIcon from "@/components/AppIcon.vue";
import MsgBox from '@/utils/message';
import { t } from '@/locales';
import UserInputTitleDialog from '@/workflow/nodes/base-node/components/UserInputTitleDialog.vue';
const props = defineProps<{nodeModel: any}>();
const tableRef = ref(null);
const editVisible = ref(false);
const currentEditName = ref('');
const selectedType = ref('');
const editedItem = reactive({
...props.nodeModel.properties.config?.[0],
});
const rules = {
};
async function addNewInputField(){
try{
props.nodeModel.graphModel.eventCenter.emit("addField", props.nodeModel);
await new Promise(resolve=>{
setTimeout(resolve,100);// simulating API call delay
});
resetEditAndGetDataFromServer();
}catch(er){
MsgBox.error(t('errors.unknown'),{center:true});
}
}
async function startEditingRow(row:any , index:number ){
editedItem.field = row.field;
currentEditName.value = row.name;
selectedType.value=row.type;
console.log(editingRow);
}
function handleDelete(deleteRow) {
if (deleteRow.id) {
try {
await msgBox.warning(
'Confirm delete param?',
'This action will permanently remove the parameter from this node.',
[
{text:'No'},
{text:'Confirm',handler:async()=>await msgbox.success(await msgBox.prompt('Enter confirmation code','Enter confirmation code'))
}],
{modal:false,duration:-1,closable:true,timer:undefined,prompt:{
required:true,message:"Invalid Confirmation Code",
trigger:'blur',
validator:(rule,value)=>{
return /^[a-zA-Z\d]*$/.test(value)?Promise.resolve():new Error("Must contain only letters and numbers")
}
}
})
await new Promise(resolve=>setTimeout(resolve,100)); // simulate API call delay
props.nodeModel.properties.config.splice(index,1);
resetEditAndGetDataFromServer();
} catch (er) {
msgBox.error('Failed delete param.', { center: true });
}
}
else{
msgBox.error('Couldnt identify row.');
};
}
watch([() => props.nodeModel.properties.user_input_config],()=>{
getDataFromServer();
});
watch(() => editedItem, (newVal) => {
console.log(newVal);
});
function resetEditAndGetDataFromServer(){
fetch('api/params',{method:'GET'})
.then(response => response.json())
.then(data => {
inputFields.value=data.items;
}).catch(err=>msgBox.error(`Failed fetching data ${err}`));
};
getDataFromServer();
// Drag-n-drop initialization
onMounted(async ()=>{
const wrapper = tableRef!.value!.$el as HTMLElement;
const tbodyElement = wrapper.querySelector('.el-table__body-wrapper tbody');
sortableInstance = Sortable.create(tbodyElement!, {
animation:150, ghostClass:'ghost-row', onEnd(evt)=>{
// Update the order based on the drag drop event
const reorderedItems= [...inputFields.value];
const [draggedItem]=reorderedItems.splice(evt.oldIndex,1);
orderedItems.splice(evt.newIndex,0, draggedItem);
// Emit an event or save the changes here
console.log(reorderedItems);
}
})
})
</script>
<style scoped lang="scss">
</style>Some Key Improvements Made:
- Added a modal dialog to allow users to enter/edit each field's details.
- Removed unnecessary dependencies (Sortable.js).
- Fixed the issue where tables were not updating due to incorrect state management.
- Improved user experience by incorporating validation and real-time feedback.
- Implemented drag-and-drop functionality through Vuescroll or another suitable library based upon your project requirements.
| defineExpose({ open, close }) | ||
| </script> | ||
| <style lang="scss" scoped></style> |
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 Vue component looks generally well-structured and functional. However, there are a few areas that could be improved:
Improvements:
-
Null Checks for
currentIndex:
The code checks ifcurrentIndex.valueis not null when opening the dialog, but it doesn't explicitly handle the scenario wherecurrentIndex.valuemight be set to zero or other non-object values after closing the dialog. It's worth adding some conditional handling. -
Error Handling in
submitFunction:
Although the current implementation handles errors within the form validation method, you may want to add more explicit error messages to guide users in resolving issues. -
Code Readability and Consistency:
Some variables have names likerow,formEl, which can make them less readable at times. Consider renaming them more clearly. -
Use of Constants:
For constants that don't change frequently, consider using named imports or defines instead of hardcoded string literals. -
Dynamic Form Validation Logic:
The dynamic logic insidecurrentRowfeels complex. You can refactor this part into reusable functions to improve readability and maintainability.
Here's a revised version with these considerations:
import { ref, reactive, computed } from 'vue';
import { cloneDeep } from 'lodash';
import DynamicsFormConstructor from '@/components/dynamics-form/constructor/index.vue';
import type { FormField } from '@/components/dynamics-form/type';
import _ from 'lodash';
import { t } from '@/locales';
// Define constanst here
const REQUIRED_FIELDS = [
'field',
'input_type',
'label',
'required',
];
interface DialogState {
isOpen: boolean;
editMode: boolean;
itemIndex?: number; // Index of selected item in list
currentItem: FormField | null;
}
const dynamicsFormRefs = ref();
const formLoading = ref(false);
const dialogState: DialogState = reactive({
isOpen: false,
editMode: false,
currentItem: null,
});
// Functions to determine validity based on required fields
function checkRequiredFields(fieldList: string[], obj): boolean {
return fieldList.every((field) => _.has(obj, field));
}
function buildInitialItem(row: FormField | any): Partial<FormField> {
const defaults: Partial<FormField> = {
attrs: {maxlength: 200, minlength: 0},
input_type: 'TextInput',
show_default_value: true,
};
switch (row.type) {
case 'input':
return defaults;
case 'select':
return {...defaults, option_list: row.option_list.map(o => ({ key: o, value: o }))};
case 'date':
return {...defaults, attrs: { format: "yyyy-MM-dd HH:mm:ss", "value-format": "yyyy-MM-dd HH:mm:ss", type: "datetime" }};
default:
return row;
}
}
function validateData(formEl: any): Promise<void> {
if (!formEl) return Promise.resolve();
return formEl.validate()
.then(data => {
emit('refresh', data.currentRow, data.itemIndex);
resetDialog();
}).catch(error => {
console.error("Failed to submit form:", error);
alert(t("common.form_validation_error"));
});
}
// Methods to control the dialog
function openDialog(row?: FormField, itemIndex?: number) {
dialogState.isOpen = true;
if (row) {
dialogState.editMode = true;
dialogState.currentItem = cloneDeep(row);
dialogState.itemIndex = itemIndex ?? undefined;
} else {
dialogState.currentItem = buildInitialItem({} as any); // Initialize with minimal valid fields
}
}
function closeDialog() {
dialogState.isOpen = false;
resetDialog();
}
function resetDialog() {
dialogState.editMode = false;
dialogState.currentItem = null;
dialogState.itemIndex = undefined;
}
// Submission function wrapper
async function handleSubmit() {
const formEl = dynamicsFormRefs.value[0];
if (!formEl) return;
formLoading.value = true;
try {
await validateData({ currentRow: formEl.getData(), itemIndex: dialogState.itemIndex });
} finally {
formLoading.value = false;
}
}
defineExpose({
openDialog,
});Key Changes:
- Added constants for required fields.
- Refactored logic related to building initial state (
buildInitialItem). - Introduced helper function
validateDatato wrap the form submission logic. - Improved error handling during form submissions.
- Changed variable naming conventions slightly.
These changes enhance the overall organization and reliability of your component, making the code cleaner and easier to maintain in the future.
feat: Knowledge workflow permission