Skip to content

feat(CLA): add CLA signing functionality#2020

Open
juanlou1217 wants to merge 3 commits intoweb3infra-foundation:mainfrom
juanlou1217:feat/cla
Open

feat(CLA): add CLA signing functionality#2020
juanlou1217 wants to merge 3 commits intoweb3infra-foundation:mainfrom
juanlou1217:feat/cla

Conversation

@juanlou1217
Copy link
Contributor

  • 在设置页面集成 CLA 状态展示入口
  • 在 MergeBox 中集成 CLA 签署检查逻辑
  • MergeSection 新增 claCheck 参数,未签署时显示"Sign CLA"按钮
  • 修复NewCodeView下创建文件路径解析问题

- 在设置页面集成 CLA 状态展示入口
- 在 MergeBox 中集成 CLA 签署检查逻辑
- MergeSection 新增 claCheck 参数,未签署时显示"Sign CLA"按钮
- 修复NewCodeView下创建文件路径解析问题
Copy link

@chatgpt-codex-connector chatgpt-codex-connector bot left a comment

Choose a reason for hiding this comment

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

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: e8e6dbccb1

ℹ️ About Codex in GitHub

Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".

const additionalChecks = mergeBoxData?.merge_requirements?.conditions ?? []

const claCondition = additionalChecks.find((c) => c.type === CheckType.ClaSign)
const claCheck = claCondition ? claCondition.result === ConditionResult.PASSED : true

Choose a reason for hiding this comment

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

P1 Badge Block queue action until CLA check is loaded

This defaults claCheck to true whenever the ClaSign condition is absent, which also happens before merge-box data has finished loading. In that window MergeSection treats the CL as mergeable and enables “Add to Queue”, so unsigned contributors can enqueue a CL before the CLA requirement arrives from the API. This is risky because queue admission currently validates open status but does not enforce merge requirements in add_to_merge_queue (ceres/src/api_service/mono_api_service.rs).

Useful? React with 👍 / 👎.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

正常是需要MergeBox全部校验通过,才显示add按钮,但是因为一些校验没做完整,要测试add,就把校验逻辑注释了,后续统一开启校验逻辑

const { data: claData, isLoading: isContentLoading } = useGetClaContent()
const { mutate: signCla, isPending: isSigning } = usePostClaChangeSignStatus()
const { mutate: saveClaContent, isPending: isSaving } = usePostClaContent()
const viewerIsAdmin = useViewerIsAdmin()

Choose a reason for hiding this comment

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

P2 Badge Use an admin check that is stable on /me settings

useViewerIsAdmin() depends on organization scope, but this page is under /me/settings/... where no org route param is present on direct load. On a hard refresh, the hook resolves false and hides “Edit CLA” even for authorized admins, so access to CLA editing becomes dependent on prior navigation state instead of actual permissions.

Useful? React with 👍 / 👎.

Copy link

@chatgpt-codex-connector chatgpt-codex-connector bot left a comment

Choose a reason for hiding this comment

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

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 8cdb1fb964

ℹ️ About Codex in GitHub

Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".

return null
}

return (

Choose a reason for hiding this comment

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

P1 Badge Block menu edits until sidebar data has loaded

Rendering the editor unconditionally here allows interaction before useGetSidebarList() has returned real data (hard refresh / slow network), so sortedData is treated as empty and admins can click New + Apply against an incomplete items list. That can overwrite the saved menu with partial data and drop existing entries, and the late fetch will not repopulate once items.length > 0 because initialization only runs for an empty local list.

Useful? React with 👍 / 👎.

Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

This PR adds Contributor License Agreement (CLA) support to the Moon web app, exposing user CLA status in settings, enabling CLA signing/editing, and enforcing CLA status as part of CL merge requirements. It also includes a NewCodeView path parsing fix and corresponding type updates.

Changes:

  • Add CLA status section in account settings plus a dedicated CLA signing page (with admin-only CLA content editing).
  • Integrate CLA status into MergeBox/MergeSection merge gating and UI (including “Sign CLA” CTA).
  • Fix NewCodeView create-entry path/name parsing to properly split parent path vs filename.

Reviewed changes

Copilot reviewed 16 out of 16 changed files in this pull request and generated 4 comments.

Show a summary per file
File Description
moon/packages/ui/src/Logo/Logo.tsx Updates the primary logo SVG assets/sizing.
moon/packages/types/generated.ts Regenerates API types to include CLA endpoints/check type and additional schema changes.
moon/apps/web/pages/me/settings/index.tsx Adds CLA status section to settings; gates MenuPicker to admins.
moon/apps/web/pages/me/settings/cla/sign/index.tsx New CLA signing page with optional admin edit dialog for CLA content.
moon/apps/web/hooks/Cla/usePostClaContent.ts Mutation hook to update CLA content and invalidate CLA content query.
moon/apps/web/hooks/Cla/usePostClaChangeSignStatus.ts Mutation hook to sign CLA and invalidate CLA status query.
moon/apps/web/hooks/Cla/useGetClaContent.ts Query hook to fetch current CLA content.
moon/apps/web/hooks/Cla/useClaStatus.ts Query hook to fetch current user’s CLA signing status.
moon/apps/web/components/Sidebar/SidebarMenu/MenuPicker.tsx Removes internal admin gating (now gated by settings page).
moon/apps/web/components/Setting/ClaStatusSection.tsx New settings section showing CLA status and linking to signing flow.
moon/apps/web/components/CodeView/NewCodeView/PathInput.tsx Refactors base path handling to use a ref and adjusts params parsing/deps.
moon/apps/web/components/CodeView/NewCodeView/NewCodeView.tsx Fixes create-entry submission by splitting parent path and filename.
moon/apps/web/components/ClBox/types/mergeCheck.config.ts Adds display label for the new ClaSign merge requirement.
moon/apps/web/components/ClBox/hooks/useGetMergeBox.ts Forces merge-box refetch on mount to keep requirements current.
moon/apps/web/components/ClBox/MergeSection.tsx Adds claCheck prop and shows “Sign CLA” CTA; gates mergeability by CLA.
moon/apps/web/components/ClBox/MergeBox.tsx Computes CLA condition result from merge requirements and passes it to MergeSection.
Comments suppressed due to low confidence (1)

moon/apps/web/components/ClBox/MergeSection.tsx:105

  • When claCheck is false but all reviewers are approved, statusNode still shows "Ready to merge - All reviewers approved" even though the merge/queue action is disabled. This is misleading; add an explicit branch for claCheck === false (ideally before the isAllReviewerApproved check) so the UI communicates that merging is blocked until the CLA is signed.
    const isMergeable = isAllReviewerApproved && claCheck !== false
    const isDraft = clStatus?.toLowerCase() === 'draft'

    if (isDraft) {
      statusNode = (
        <div className='flex items-center text-yellow-700'>
          <WarningTriangleIcon className='mr-3 h-5 w-5' />
          <span className='font-semibold'>CL has not yet prepared for the review</span>
        </div>
      )
    } else if (!isAllReviewerApproved) {
      statusNode = (
        <div className='flex items-center text-yellow-700'>
          <WarningTriangleIcon className='mr-3 h-5 w-5' />
          <span className='font-semibold'>Merging is blocked - Waiting for reviewers approval</span>
        </div>
      )
    } else {
      statusNode = (
        <div className='flex items-center text-green-700'>
          <CheckCircleIcon className='mr-3 h-5 w-5' />
          <span className='font-semibold'>Ready to merge - All reviewers approved</span>
        </div>
      )
    }

Comment on lines 30 to +44
@@ -37,10 +38,10 @@ export default function PathInput({ pathState, nameState }: PathInputProps) {
}
newBase.unshift('')

setBasePath(newBase)
fixedBasePathRef.current = newBase
updatePath(newBase, userSegments, current)
// eslint-disable-next-line react-hooks/exhaustive-deps
}, [JSON.stringify((params as any)?.path)])
}, [params?.path])
Comment on lines +3586 to +3606
export type CommonResultCommonPageWebhookResponse = {
data?: {
items: {
active: boolean
created_at: string
event_types: string[]
/** @format int64 */
id: number
path_filter?: string | null
target_url: string
updated_at: string
}[]
/**
* @format int64
* @min 0
*/
total: number
}
err_message: string
req_result: boolean
}
Comment on lines +7302 to +7305
export type GetBuildsLogsV2Data = LogLinesResponse

export type GetBuildsLogsV2Error = LogErrorResponse

Comment on lines 65 to +68
const additionalChecks = mergeBoxData?.merge_requirements?.conditions ?? []

const claCondition = additionalChecks.find((c) => c.type === CheckType.ClaSign)
const claCheck = claCondition ? claCondition.result === ConditionResult.PASSED : true
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants