-
Notifications
You must be signed in to change notification settings - Fork 2.6k
feat: Tag Permission #4224
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
feat: Tag Permission #4224
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -133,6 +133,14 @@ const share = { | |
| ], | ||
| 'OR' | ||
| ), | ||
| doc_tag: () => | ||
| hasPermission ( | ||
| [ | ||
| RoleConst.ADMIN, | ||
| PermissionConst.SHARED_KNOWLEDGE_DOCUMENT_TAG | ||
| ], | ||
| 'OR' | ||
| ), | ||
| problem_create: () => | ||
| hasPermission ( | ||
| [ | ||
|
|
@@ -182,6 +190,39 @@ const share = { | |
| ], | ||
| '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, | ||
|
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The provided code snippet seems to define several functions within an object named There are no explicit issues or optimizations present in the code. However, consider the following improvements:
Here's an enhanced version of the code with some of these suggestions applied: interface PermissionsConstants {
ADMIN: string;
SHARED_KNOWLEDGE_DOCUMENT_TAG: string;
SHARE_TAG_READ: string;
// Add other shared knowledge tag-related constants here
}
const { ADMIN, SHARED_KNOWLEDGE_DOCUMENT_TAG } = Object.freeze(
permissionsConstants || {}
);
function hasPermission(rolesOrPermissions: string[], operator: 'AND' | 'OR') {
// Implementation of hasPermission function goes here
}
const share = {
doc_tag: (): boolean =>
hasPermission([ADMIN, SHARED_KNOWLEDGE_DOCUMENT_TAG], 'OR'),
problem_create: () => false,
view_document_details: () => {},
modify_question_answers: () => {},
manage_course_sections: () => {},
grant_user_permissions: () => {}, // Placeholder name
chat_user_edit: (): boolean => true,
auth: () => false,
};
// Helper function for repeated code extraction
function createTagManagementFunctions(role: RoleConst) {
const tags = ['read', 'create', 'edit', 'delete'];
tags.forEach(tag => {
share[`${tag}_tag_${role as keyof typeof role}`] = () =>
hasPermission([role, `${PermissionConst.SHARED_KNOWLEDGE}__${tag.toUpperCase()}`], 'OR');
})
}
// Example usage of the helper function
if (typeof process !== 'undefined' && process.env.NODE_ENV === 'development') {
createTagManagementFunctions(Role.Admin);
}This version maintains the original functionality while improving upon clarity, reusability, and adherence to coding standards. |
||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -232,6 +232,16 @@ const workspace = { | |
| ], | ||
| 'OR', | ||
| ), | ||
| doc_tag: (source_id:string) => | ||
| hasPermission( | ||
| [ | ||
| new ComplexPermission([RoleConst.USER],[PermissionConst.KNOWLEDGE.getKnowledgeWorkspaceResourcePermission(source_id)],[],'AND'), | ||
| RoleConst.WORKSPACE_MANAGE.getWorkspaceRole, | ||
| PermissionConst.KNOWLEDGE_DOCUMENT_TAG.getKnowledgeWorkspaceResourcePermission(source_id), | ||
| PermissionConst.KNOWLEDGE_DOCUMENT_TAG.getWorkspacePermissionWorkspaceManageRole, | ||
| ], | ||
| 'OR', | ||
| ), | ||
| knowledge_chat_user_read: (source_id:string) => false, | ||
| knowledge_chat_user_edit: (source_id:string) => | ||
| hasPermission( | ||
|
|
@@ -293,6 +303,46 @@ const workspace = { | |
| ], | ||
| 'OR', | ||
| ), | ||
| tag_read: (source_id:string) => | ||
| hasPermission( | ||
| [ | ||
| new ComplexPermission([RoleConst.USER],[PermissionConst.KNOWLEDGE.getKnowledgeWorkspaceResourcePermission(source_id)],[],'AND'), | ||
| RoleConst.WORKSPACE_MANAGE.getWorkspaceRole, | ||
| PermissionConst.KNOWLEDGE_TAG_READ.getKnowledgeWorkspaceResourcePermission(source_id), | ||
| PermissionConst.KNOWLEDGE_TAG_READ.getWorkspacePermissionWorkspaceManageRole, | ||
| ], | ||
| 'OR', | ||
| ), | ||
| tag_create: (source_id:string) => | ||
| hasPermission( | ||
| [ | ||
| new ComplexPermission([RoleConst.USER],[PermissionConst.KNOWLEDGE.getKnowledgeWorkspaceResourcePermission(source_id)],[],'AND'), | ||
| RoleConst.WORKSPACE_MANAGE.getWorkspaceRole, | ||
| PermissionConst.KNOWLEDGE_TAG_CREATE.getKnowledgeWorkspaceResourcePermission(source_id), | ||
| PermissionConst.KNOWLEDGE_TAG_CREATE.getWorkspacePermissionWorkspaceManageRole, | ||
| ], | ||
| 'OR', | ||
| ), | ||
| tag_edit: (source_id:string) => | ||
| hasPermission( | ||
| [ | ||
| new ComplexPermission([RoleConst.USER],[PermissionConst.KNOWLEDGE.getKnowledgeWorkspaceResourcePermission(source_id)],[],'AND'), | ||
| RoleConst.WORKSPACE_MANAGE.getWorkspaceRole, | ||
| PermissionConst.KNOWLEDGE_TAG_EDIT.getKnowledgeWorkspaceResourcePermission(source_id), | ||
| PermissionConst.KNOWLEDGE_TAG_EDIT.getWorkspacePermissionWorkspaceManageRole, | ||
| ], | ||
| 'OR', | ||
| ), | ||
| tag_delete: (source_id:string) => | ||
| hasPermission( | ||
| [ | ||
| new ComplexPermission([RoleConst.USER],[PermissionConst.KNOWLEDGE.getKnowledgeWorkspaceResourcePermission(source_id)],[],'AND'), | ||
| RoleConst.WORKSPACE_MANAGE.getWorkspaceRole, | ||
| PermissionConst.KNOWLEDGE_TAG_DELETE.getKnowledgeWorkspaceResourcePermission(source_id), | ||
| PermissionConst.KNOWLEDGE_TAG_DELETE.getWorkspacePermissionWorkspaceManageRole, | ||
| ], | ||
| 'OR', | ||
| ), | ||
| chat_user_edit: (source_id:string) => | ||
| hasPermission( | ||
| [ | ||
|
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The provided code appears to be configuring access permissions for various actions related to knowledge and tags within a workspace system, using a combination of role-based checks and conditional logic. Here are some points to consider:
Overall, the code structure looks comprehensive but could benefit from cleanups and optimizations for better readability and maintenance efficiency. |
||
|
|
||
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.
There are several issues and improvements that can be made in the provided JavaScript code snippet:
Duplicate
chat_user_editMethod: The methodchat_user_editis defined twice, but it should only be present once since both methods are intended to represent the same functionality whenhasPermission returns true.Code Redundancy: There is redundancy in defining methods like
doc_tag,knowledge_chat_user_read,tag_create, etc. instead of using arrays to collect common permission checks.Simplified Permission Check Logic: Instead of manually adding each role and permission pair multiple times, you could use a utility function or array to store these pairs, which would make your code cleaner and more maintainable.
Boolean Negation: In some cases (e.g.,
chat_user_edit), boolean negations are unnecessary unless there's specific context for them.Here’s an optimized version of the code with these improvements:
Explanation:
hasPermission): A placeholder for where actual user permission validation logic would go. You need to implement this function based on your application's architecture and authentication mechanism.This refactored approach makes the code more modular, easier to understand, and scalable.