Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
29 changes: 29 additions & 0 deletions ui/src/permission/application/system-manage.ts
Original file line number Diff line number Diff line change
Expand Up @@ -102,5 +102,34 @@ const systemManage = {
],
'OR'
),
overview_read: () =>
hasPermission(
[
RoleConst.ADMIN,
PermissionConst.RESOURCE_APPLICATION_OVERVIEW_READ
],
'OR'
),
access_read: () =>
hasPermission(
[
RoleConst.ADMIN,
PermissionConst.RESOURCE_APPLICATION_ACCESS_READ
],'OR'
),
chat_user_read: () =>
hasPermission(
[
RoleConst.ADMIN,
PermissionConst.RESOURCE_APPLICATION_CHAT_USER_READ
],'OR'
),
chat_log_read: () =>
hasPermission(
[
RoleConst.ADMIN,
PermissionConst.RESOURCE_APPLICATION_CHAT_LOG_READ
],
'OR')
}
export default systemManage
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 is a simplified representation of defining permission management logic within a system interface using Role-based Access Control (RBAC). You've highlighted several potential improvements:

Potential Issues/Improvements:

  1. Code Duplicity: The methods overview_read, access_read, chat_user_read, and chat_log_read essentially repeat similar logic with only slight variations. This can lead to redundancy and maintenance challenges.

  2. Repetition of Role Check: There appears to be repetition in hardcoding roles like RoleConst.ADMIN. Consider storing these constants elsewhere to prevent duplication and make changes easily if needed.

  3. Variable Initialization: Although it's minimal in this case, initializing variables at the top can sometimes clean up the function body and make it more readable.

  4. Method Naming: While descriptive, some method names (overview_read, access_read) may not clearly convey their functionality. Consider renaming them to better reflect the operations they perform.

  5. Function Arguments: Currently, all functions take an array of permissions as their third argument, which could benefit from being documented or made consistent across all implementations.

  6. Return Type Expectation: Ensure that each function returns a boolean indicating whether the user has read access based on the role(s) specified.

Here’s an optimized version of the code with some suggestions applied:

const { RoleConst, PermissionConst } = require('path-to-your-permissions-module'); // Adjust path accordingly

// Helper function to determine permission status
function hasPermissions(userRoles, requiredPermissions, type = 'OR') {
  return userRoles.some(role => {
    const matchingPermissions = requiredPermissions.filter(permission =>
      typeof permission === 'string' ? permission.includes(role) : Array.isArray(permission) && permission.includes(role)
    );
    switch (type.toUpperCase()) {
      case 'AND':
        return matchingPermissions.length === requiredPermissions.length;
      case 'OR':
        return matchingPermissions.length > 0;
      default:
        throw new Error(`Unsupported type: ${type}`);
    }
  });
}

/**
 * Manages different types of application data read accesses.
 */
const systemManage = {
  overview_read: () => hasPermissions(
    [ RoleConst.ADMIN ],
    [ PermissionConst.RESOURCE_APPLICATION_OVERVIEW_READ ]
  ),
  
  access_read: () => hasPermissions(
    [ RoleConst.ADMIN ],
    [ PermissionConst.RESOURCE_APPLICATION_ACCESS_READ ]
  ),

  chat_user_read: () => hasPermissions(
    [ RoleConst.ADMIN ],
    [ PermissionConst.RESOURCE_APPLICATION_CHAT_USER_READ ]
  ),  
  
  chat_log_read: () => hasPermissions(
    [ RoleConst.ADMIN ],
    [ PermissionConst.RESOURCE_APPLICATION_CHAT_LOG_READ ]
  ) 
};

export default systemManage;

Key Improvements:

  • Helper Function: A reusable helper function hasPermissions encapsulates the role-checking logic, making it easier to modify the logic for checking multiple conditions in one place.
  • DRY Principle: By reusing the same logic across multiple methods, we reduce code duplication.
  • Comments/documentation: Added comments to describe the purpose of each function.
  • Consistent Structure: Ensured that the structure remains clear and understandable.

4 changes: 4 additions & 0 deletions ui/src/permission/application/workspace.ts
Original file line number Diff line number Diff line change
Expand Up @@ -164,6 +164,10 @@ const workspace = {
],
'OR'
),
overview_read: () => false,
access_read: () => false,
chat_user_read: () => false,
chat_log_read: () => false
}


Expand Down
49 changes: 45 additions & 4 deletions ui/src/router/modules/application-detail.ts
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,16 @@ const ApplicationDetailRouter = {
return PermissionConst.APPLICATION_OVERVIEW_READ.getApplicationWorkspaceResourcePermission(
to ? to.params.id : '',
)
}
},
() => {
const to: any = get_next_route()
if (to.path.includes('resource-management')) {return RoleConst.ADMIN}
},
() => {
const to: any = get_next_route()
console.log('ss',to)
if (to.path.includes('resource-management')) { return PermissionConst.RESOURCE_APPLICATION_OVERVIEW_READ}
},
]
},
component: () => import('@/views/application-overview/index.vue'),
Expand All @@ -59,7 +68,15 @@ const ApplicationDetailRouter = {
return PermissionConst.APPLICATION_EDIT.getApplicationWorkspaceResourcePermission(
to ? to.params.id : '',
)
}
},
() => {
const to: any = get_next_route()
if (to.path.includes('resource-management')) {return RoleConst.ADMIN}
},
()=>{
const to: any = get_next_route()
if (to.path.includes('resource-management')) { return PermissionConst.RESOURCE_APPLICATION_EDIT}
},
]
},
component: () => import('@/views/application/ApplicationSetting.vue'),
Expand All @@ -84,6 +101,14 @@ const ApplicationDetailRouter = {
return PermissionConst.APPLICATION_ACCESS_READ.getApplicationWorkspaceResourcePermission(
to ? to.params.id : '',)
}],[EditionConst.IS_EE, EditionConst.IS_PE],'OR'),
() => {
const to: any = get_next_route()
if (to.path.includes('resource-management')) {return RoleConst.ADMIN}
},
()=>{
const to: any = get_next_route()
if (to.path.includes('resource-management')) { return PermissionConst.RESOURCE_APPLICATION_ACCESS_READ}
},
]
},
component: () => import('@/views/application/ApplicationAccess.vue'),
Expand All @@ -109,7 +134,15 @@ const ApplicationDetailRouter = {
const to: any = get_next_route()
return new ComplexPermission([],[PermissionConst.APPLICATION_CHAT_USER_READ.getApplicationWorkspaceResourcePermission(
to ? to.params.id : '',)],[EditionConst.IS_EE, EditionConst.IS_PE],'OR')
}
},
() => {
const to: any = get_next_route()
if (to.path.includes('resource-management')) {return RoleConst.ADMIN}
},
()=>{
const to: any = get_next_route()
if (to.path.includes('resource-management')) { return PermissionConst.RESOURCE_APPLICATION_CHAT_USER_READ}
},
]
},
component: () => import('@/views/chat-user/index.vue'),
Expand All @@ -135,7 +168,15 @@ const ApplicationDetailRouter = {
return PermissionConst.APPLICATION_CHAT_LOG_READ.getApplicationWorkspaceResourcePermission(
to ? to.params.id : '',
)
}
},
() => {
const to: any = get_next_route()
if (to.path.includes('resource-management')) {return RoleConst.ADMIN}
},
()=>{
const to: any = get_next_route()
if (to.path.includes('resource-management')) { return PermissionConst.RESOURCE_APPLICATION_CHAT_LOG_READ}
},
]
},
component: () => import('@/views/chat-log/index.vue'),
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 is an implementation of Vue Router routes with permission checks on resource-based access. Here's a breakdown of some suggestions and potential improvements:

  1. Avoid Redundant Calls: The code calls get_next_route() multiple times without storing the result. Consider caching this value or reusing it.

  2. Optimize Permissions Check:

    • Consolidate the logic for determining who gets what permissions based on route path.
    • Use a more dynamic approach to determine which permissions should be allowed when accessing certain sections of the application.
  3. Use Caching Mechanisms: Cache the route parameters to avoid recalculating them unnecessarily during navigation changes.

  4. Code Duplication: Notice that similar logic appears several times (ADMIN, read-write, etc.). Refactor common parts into helper functions.

  5. Logging Improvements: Move the logging statement inside where its meaningful (in the case where permissions are granted).

Here's an improved version incorporating these suggestions:

const ApplicationDetailRouter = [
  {
    path: '/application/:id/overview',
    meta: {
      permissionCheckers: (to) => {
        const hasResourceManagementPath =
          // Implement logic here to determine if there is a specific path related to Resource Management
          false; // Replace with actual logic

        return [
          () =>
            !hasResourceManagementPath ||
            PermissionConst.APPLICATION_OVERVIEW_READ.getApplicationWorkspaceResourcePermission(to?.params.id),
          () => (!hasResourceManagementPath || RoleConst.ADMIN),
          () =>
            !hasResourceManagementPath ||
            PermissionConst.RESOURCE_APPLICATION_OVERVIEW_READ,
        ];
      },
    },
    component: () => import('@/views/application-overview/index.vue'),
  },
  {
    path: '/application/:id/edit',
    meta: {
      permissionCheckers: (to) => {
        const hasResourceManagementPath =
          // Implement logic here to determine if there is a specific path related to Resource Management
          false;

        return [
          () => !hasResourceManagementPath ||
                  PermissionConst.APPLICATION_EDIT.getApplicationWorkspaceResourcePermission(to?.params.id),
          () => (!hasResourceManagementPath || RoleConst.ADMIN),
          () =>
            !hasResourceManagementPath ||
            PermissionConst.RESOURCE_APPLICATION_EDIT,
        ];
      },
    },
    component: () => import('@/views/application/ApplicationSetting.vue'),
  },
  {
    // Rest of the routes remain unchanged but apply the same logic through their respective meta.permissionsChecker
  }
];

Note: The above suggestion assumes you have a specific way to identify paths associated with "Resource Management." Adjust the logic accordingly based on your application structure.

By addressing redundant function calls and consolidating duplicate logic, we can make the code cleaner and potentially enhance performance, given the complexity involved in routing and security checks within applications.

Expand Down
2 changes: 1 addition & 1 deletion ui/src/views/chat-user/index.vue
Original file line number Diff line number Diff line change
Expand Up @@ -259,7 +259,7 @@ const permissionObj = ref<any>({
[],
'OR',
),
APPLICATION_KNOWLEDGE: [RoleConst.ADMIN, PermissionConst.RESOURCE_APPLICATION_CHAT_USER_EDIT],
RESOURCE_APPLICATION: [RoleConst.ADMIN, PermissionConst.RESOURCE_APPLICATION_CHAT_USER_EDIT],
RESOURCE_KNOWLEDGE: [RoleConst.ADMIN, PermissionConst.RESOURCE_KNOWLEDGE_CHAT_USER_EDIT],
SHAREDKNOWLEDGE: new ComplexPermission(
[RoleConst.ADMIN],
Expand Down
152 changes: 22 additions & 130 deletions ui/src/views/system-resource-management/ApplicationResourceIndex.vue
Original file line number Diff line number Diff line change
Expand Up @@ -241,19 +241,26 @@
effect="dark"
:content="$t('views.system.resource_management.management')"
placement="top"
v-if="managePermission()"
>
<span class="mr-8">
<el-button
type="primary"
text
:title="$t('views.system.resource_management.management')"
@click="goApp(row)"
@click="
router.push({
path: `/application/resource-management/${row.id}/${row.type}/overview`,
})
"
>
<AppIcon iconName="app-admin-operation"></AppIcon>
</el-button>
</span>
</el-tooltip>
<el-dropdown trigger="click">
<el-dropdown trigger="click"
v-if="MoreFilledPermission()"
>
<el-button text @click.stop>
<el-icon>
<MoreFilled />
Expand Down Expand Up @@ -295,9 +302,6 @@ import { datetimeFormat } from '@/utils/time'
import { loadPermissionApi } from '@/utils/dynamics-api/permission-api.ts'
import { isWorkFlow } from '@/utils/application.ts'
import UserApi from '@/api/user/user.ts'
import { hasPermission } from '@/utils/permission'
import { ComplexPermission } from '@/utils/permission/type'
import { EditionConst, PermissionConst, RoleConst } from '@/utils/permission/data'
import permissionMap from '@/permission'
import { MsgSuccess, MsgConfirm, MsgError } from '@/utils/message'

Expand All @@ -309,6 +313,19 @@ const permissionPrecise = computed(() => {
return permissionMap['application']['systemManage']
})

const managePermission = () => {
return permissionPrecise.value.overview_read() ||
permissionPrecise.value.access_read() ||
permissionPrecise.value.edit() ||
permissionPrecise.value.chat_log_read() ||
permissionPrecise.value.chat_user_read()
}

const MoreFilledPermission = () => {
return permissionPrecise.value.export() ||
permissionPrecise.value.delete()
}

const apiInputParams = ref([])
function toChat(row: any) {
row?.work_flow?.nodes
Expand Down Expand Up @@ -396,131 +413,6 @@ const paginationConfig = reactive({
total: 0,
})

const goApp = (item: any) => {
router.push({ path: get_route(item) })
}

const get_route = (item: any) => {
if (
hasPermission(
[
new ComplexPermission(
[RoleConst.USER],
[PermissionConst.APPLICATION.getApplicationWorkspaceResourcePermission(item.id)],
[],
'AND',
),
RoleConst.WORKSPACE_MANAGE.getWorkspaceRole,
PermissionConst.APPLICATION_OVERVIEW_READ.getWorkspacePermissionWorkspaceManageRole,
PermissionConst.APPLICATION_OVERVIEW_READ.getApplicationWorkspaceResourcePermission(
item.id,
),
],
'OR',
)
) {
return `/application/resource-management/${item.id}/${item.type}/overview`
} else if (
hasPermission(
[
new ComplexPermission(
[RoleConst.USER],
[PermissionConst.APPLICATION.getApplicationWorkspaceResourcePermission(item.id)],
[],
'AND',
),
RoleConst.WORKSPACE_MANAGE.getWorkspaceRole,
PermissionConst.APPLICATION_EDIT.getWorkspacePermissionWorkspaceManageRole,
PermissionConst.APPLICATION_EDIT.getApplicationWorkspaceResourcePermission(item.id),
],
'OR',
)
) {
if (item.type == 'WORK_FLOW') {
return `/application/resource-management/${item.id}/workflow`
} else {
return `/application/resource-management/${item.id}/${item.type}/setting`
}
} else if (
hasPermission(
[
new ComplexPermission(
[RoleConst.USER],
[PermissionConst.APPLICATION.getApplicationWorkspaceResourcePermission(item.id)],
[EditionConst.IS_EE, EditionConst.IS_PE],
'AND',
),
new ComplexPermission(
[RoleConst.WORKSPACE_MANAGE.getWorkspaceRole],
[PermissionConst.APPLICATION_ACCESS_READ.getWorkspacePermissionWorkspaceManageRole],
[EditionConst.IS_EE, EditionConst.IS_PE],
'OR',
),
new ComplexPermission(
[],
[
PermissionConst.APPLICATION_ACCESS_READ.getApplicationWorkspaceResourcePermission(
item.id,
),
],
[EditionConst.IS_EE, EditionConst.IS_PE],
'OR',
),
],
'OR',
)
) {
return `/application/resource-management/${item.id}/${item.type}/access`
} else if (
hasPermission(
[
new ComplexPermission(
[RoleConst.USER],
[PermissionConst.APPLICATION.getApplicationWorkspaceResourcePermission(item.id)],
[EditionConst.IS_EE, EditionConst.IS_PE],
'AND',
),
new ComplexPermission(
[RoleConst.WORKSPACE_MANAGE.getWorkspaceRole],
[PermissionConst.APPLICATION_CHAT_USER_READ.getWorkspacePermissionWorkspaceManageRole],
[EditionConst.IS_EE, EditionConst.IS_PE],
'OR',
),
new ComplexPermission(
[],
[
PermissionConst.APPLICATION_CHAT_USER_READ.getApplicationWorkspaceResourcePermission(
item.id,
),
],
[EditionConst.IS_EE, EditionConst.IS_PE],
'OR',
),
],
'OR',
)
) {
return `/application/resource-management/${item.id}/${item.type}/chat-user`
} else if (
hasPermission(
[
new ComplexPermission(
[RoleConst.USER],
[PermissionConst.APPLICATION.getApplicationWorkspaceResourcePermission(item.id)],
[],
'AND',
),
PermissionConst.APPLICATION_CHAT_LOG_READ.getWorkspacePermissionWorkspaceManageRole,
PermissionConst.APPLICATION_CHAT_LOG_READ.getApplicationWorkspaceResourcePermission(
item.id,
),
],
'OR',
)
) {
return `/application/resource-management/${item.id}/${item.type}/chat-log`
} else return `/system/resource-management/application/`
}

const workspaceOptions = ref<any[]>([])
const workspaceVisible = ref(false)
Copy link
Contributor Author

Choose a reason for hiding this comment

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

In this code snippet, there are several improvements can be made:

  1. Use v-show Instead of v-if:

    <el-tooltip
      v-show="... condition ... "
      :effect="..."
      
      ...
    
      <!-- Use v-show instead of v-if -->
      <el-button text v-show="morefilledper()"  
        @click="
          router.push({
            path: `/application/resource-management/${row.id}/${row.type}/overview`,
          })
        "
      >
        
        ...
        
      </el-button>
    
      ...
     
     
      
    
    
      
      
      
      
      <div v-show="otherCondition">
           ...
      </div>   
    
  2. Code Optimization:

    • The use of conditional rendering in Vue components might make the code longer than strictly necessary. For example, you could extract parts of logic into functions rather than writing it inline where possible.
    <template>
      <!-- Code using v-show with multiple conditions can be condensed -->
      <Tooltip
        v-for="(cond, key) in conditionsObj" 
         v-show="!cond.condition" 
        :key="key"
        :effect="tooltipEffect"
        placement="top"   
        > 
        
        ......
        
        
      </Tooltip>
    
    </template>
    
    <script lang="ts">
    
      export default {
       setup(){
         const conditonsObj = {
            condblockA:{
             condition:true,
                // Other data...
             },
            condblockB:{
                condition:false,
               
               
                 
              
                //Other data
              },
            
            // other similar objects
        
           
         }
    
         const showComp = (obj)=>{
              return !data.some(el=> el === obj)
             
          }
    
    
         
         
         const tooltipEffect='dark';
    
       
       
      
    
    
         return{conditionsObj,showComp}
        }
      }
    </script>
    

3. **Consistent Code Styling:**
    - It's important to ensure consistent styling across your HTML and JavaScript code for readability.
    ```
    <!-- CSS style guide examples -->

    .element-class-name {
        color: blue;
        font-size: 14px;
        line-height: 24px; /* Optional but recommended */
        background-color: lightgray;
        padding: 5px;
        border-radius: 4px;
     }

     div.container{
        max-width: 80%;
        margin: auto;
     }
    ```

**Note:** I've provided general suggestions that will help improve the efficiency and maintainability of your Vue.js application without changing any existing functionality. Adjust them according to specific project requirements or additional context. Let me know if more details on specific aspects of your implementation are needed!

Expand Down
Loading