[shaohuzhang1]

fix: workspace trigger permission

[shaohuzhang1]

Here is the review of the provided code snippet, highlighting irregularities, potential issues, and optimization suggestions.

1. Irregularities & Code Style Issues

• Missing Spaces: There are missing spaces after keywords and operators, which violates standard JavaScript/TypeScript formatting (e.g., workspaceId=getWorkspaceId(), (new Role(...))).
• Poor Indentation: The code inside the read function has inconsistent and insufficient indentation. The .addPermission line does not align visually with .addRole.
• Long Lines: The return statement is extremely long and hard to read. It chains multiple method calls on one line.

2. Potential Logic Issues

• Empty workspaceId Handling:

  const workspaceId = getWorkspaceId() || ''

  If getWorkspaceId() returns an empty string or undefined, it defaults to ''. Passing an empty string as a key in {'workspaceId': ''} might cause unexpected permission behavior or bugs in downstream logic that expects a valid ID. Consider explicitly handling the case where the workspace ID is missing (e.g., returning false or throwing an error if context is required).

• Implicit Async Behavior: If hasPermission or can were asynchronous (though likely not here based on typical permission libraries), this code would fail. Assuming they are synchronous, it's fine, but verify the signature of hasPermission.

• Redundant Permission Check?:
  You are adding:

  1. A Role-based permission: WORKSPACE_MANAGE role for the specific workspace.
  2. A Direct permission: P.TRIGGER_READ.getWorkspacePermission()....

  Ensure this is intentional. Usually, having the WORKSPACE_MANAGE role implies read access to triggers. If hasPermission uses an OR-logic (allow if any pass), ensure the direct permission isn't redundant. If it uses AND-logic, both conditions must be met. Clarify the semantics of AggregatePermission.builder().build() vs hasPermission() evaluation logic.

3. Optimization & Best Practice Suggestions

• Use Modern JS Syntax: Prefer const and arrow functions consistently (already done, but spacing matters).
• Break Down Complex Expressions: Split the builder pattern into multiple lines for readability.
• Avoid Magic Strings/Keys: The object {'workspaceId':workspaceId} should use shorthand if possible or ensure it matches the expected API contract strictly.
• Consistency with Other Methods: The comment shows other methods like create. Ensure consistency in how they fetch workspace context and check permissions (e.g., do they also use hasPermission now, or still can?). Mixed approaches lead to maintenance nightmares.

---

✅ Suggested Refactored Code

/** 工作空间触发器操作权限。 */
import { can } from '../policy'
import { hasPermission, PermissionConstants as P } from '../core'
import { AggregatePermission, Role, RoleConstants } from '../core/common'
import { getWorkspaceId } from '@/utils/resource-context'

export default {
  /**
   * 工作空间「触发器」只读权限
   */
  read: () => {
    const workspaceId = getWorkspaceId()
    
    // Optional: Handle missing workspace context explicitly
    if (!workspaceId) {
      return false 
    }

    const aggregatePerm = AggregatePermission.builder()
      .addRole(new Role(RoleConstants.WORKSPACE_MANAGE.name, workspaceId))
      .addPermission(
        P.TRIGGER_READ.getWorkspacePermission()({ workspaceId })
      )
      .build()

    return hasPermission(aggregatePerm)
  },

  /**
   * 工作空间「触发器」创建权限
   */
  // ... rest of the file
}

Key Improvements in Refactor:

1. Readability: Broken down into multiple steps.
2. Early Return: Handles missing workspaceId early to prevent passing empty strings.
3. Shorthand Property: { workspaceId } instead of { workspaceId: workspaceId }.
4. Formatting: Proper spacing and indentation applied.