fix: [code-1085]: fix multiple bugs from bugbash (#786)

web/src/components/BranchProtection/BranchProtectionForm/BranchProtectionForm.module.scss

@@ -71,6 +71,16 @@
   width: 100% !important;
 }
 
+.codeClose {
+  cursor: pointer !important;
+  padding: 2px !important;
+  &:hover {
+    background: var(--grey-100) !important;
+    box-shadow: var(--elevation-2) !important;
+    padding: 2px !important;
+  }
+}
+
 .targetContainer {
   :global(.bp3-form-group) {
     margin-bottom: unset !important;

web/src/components/BranchProtection/BranchProtectionForm/BranchProtectionForm.module.scss.d.ts

@@ -21,6 +21,7 @@ export declare const bypassContainer: string
 export declare const checkboxLabel: string
 export declare const checkboxText: string
 export declare const checkContainer: string
+export declare const codeClose: string
 export declare const dividerContainer: string
 export declare const generalContainer: string
 export declare const greyButton: string

web/src/components/BranchProtection/BranchProtectionForm/BranchProtectionForm.tsx

@@ -15,6 +15,7 @@
  */
 import React, { useMemo, useState } from 'react'
 import cx from 'classnames'
+import * as yup from 'yup'
 import {
   Button,
   ButtonVariation,
@@ -36,7 +37,7 @@ import { useHistory } from 'react-router-dom'
 import { useGet, useMutate } from 'restful-react'
 import { BranchTargetType, SettingsTab, branchTargetOptions } from 'utils/GitUtils'
 import { useStrings } from 'framework/strings'
-import { getErrorMessage, rulesFormInitialPayload } from 'utils/Utils'
+import { REGEX_VALID_REPO_NAME, getErrorMessage, rulesFormInitialPayload } from 'utils/Utils'
 import type {
   TypesRepository,
   OpenapiRule,
@@ -192,6 +193,10 @@ const BranchProtectionForm = (props: {
       formName="branchProtectionRulesNewEditForm"
       initialValues={initialValues}
       enableReinitialize
+      validationSchema={yup.object().shape({
+        name: yup.string().trim().required().matches(REGEX_VALID_REPO_NAME, getString('validation.nameLogic')),
+        minReviewers: yup.number().typeError(getString('enterANumber'))
+      })}
       onSubmit={async formData => {
         const stratArray = [
           formData.squashMerge && 'squash',
@@ -243,6 +248,9 @@ const BranchProtectionForm = (props: {
             }
           }
         }
+        if (!formData.limitMergeStrategies) {
+          delete (payload?.definition as ProtectionBranch)?.pullreq?.merge?.strategies_allowed
+        }
         if (!formData.requireMinReviewers) {
           delete (payload?.definition as ProtectionBranch)?.pullreq?.approvals?.require_minimum_count
         }
@@ -393,6 +401,7 @@ const BranchProtectionForm = (props: {
                             )
                             formik.setFieldValue('targetList', filteredData)
                           }}
+                          className={css.codeClose}
                         />
                       </Container>
                     )
@@ -408,6 +417,8 @@ const BranchProtectionForm = (props: {
                   items={filteredUserOptions}
                   onQueryChange={setSearchTerm}
                   className={css.widthContainer}
+                  value={{ label: '', value: '' }}
+                  placeholder={getString('selectUsers')}
                   onChange={item => {
                     const id = item.value?.toString().split(' ')[0]
                     const displayName = item.label
@@ -432,7 +443,10 @@ const BranchProtectionForm = (props: {
               <Container padding={{ top: 'large' }}>
                 <Layout.Horizontal spacing="small">
                   <Button
-                    type="submit"
+                    onClick={() => {
+                      formik.submitForm()
+                    }}
+                    type="button"
                     text={editMode ? getString('branchProtection.saveRule') : getString('branchProtection.createRule')}
                     variation={ButtonVariation.PRIMARY}
                     disabled={false}

web/src/components/BranchProtection/BranchProtectionForm/BypassList.tsx

@@ -27,6 +27,7 @@ const BypassList = (props: {
                   const filteredData = bypassList.filter(item => !(item[0] === owner[0] && item[1] === owner[1]))
                   setFieldValue('bypassList', filteredData)
                 }}
+                className={css.codeClose}
               />
             </Layout.Horizontal>
           )

web/src/components/BranchProtection/BranchProtectionForm/ProtectionRulesForm/ProtectionRulesForm.tsx

@@ -83,6 +83,11 @@ const ProtectionRulesForm = (props: {
         className={css.checkboxLabel}
         label={getString('branchProtection.requireMinReviewersTitle')}
         name={'requireMinReviewers'}
+        onChange={e => {
+          if ((e.target as HTMLInputElement).checked) {
+            setFieldValue('minReviewers', 1)
+          }
+        }}
       />
       <Text padding={{ left: 'xlarge' }} className={css.checkboxText}>
         {getString('branchProtection.requireMinReviewersContent')}
@@ -141,6 +146,8 @@ const ProtectionRulesForm = (props: {
           <FormInput.Select
             onQueryChange={setSearchStatusTerm}
             items={filteredStatusOptions}
+            value={{ label: '', value: '' }}
+            placeholder={getString('selectStatuses')}
             onChange={item => {
               statusChecks?.push(item.value as string)
               const uniqueArr = Array.from(new Set(statusChecks))
@@ -162,6 +169,7 @@ const ProtectionRulesForm = (props: {
                   </Text>
                   <FlexExpander />
                   <Icon
+                    className={css.codeClose}
                     name="code-close"
                     onClick={() => {
                       const filteredData = statusChecks.filter(item => !(item === status))

web/src/components/BranchProtection/BranchProtectionListing.tsx

@@ -26,7 +26,8 @@ import {
   ButtonVariation,
   Toggle,
   useToaster,
-  FlexExpander
+  FlexExpander,
+  StringSubstitute
 } from '@harnessio/uicore'
 import cx from 'classnames'
 
@@ -196,15 +197,19 @@ const BranchProtectionListing = (props: { activeTab: string }) => {
                         <Text
                           padding={{ top: 'medium', bottom: 'medium' }}
                           font={{ variation: FontVariation.BODY2_SEMI }}>
-                          {checked ? getString('disableWebhookContent') : getString('enableWebhookContent')}
+                          <StringSubstitute
-                          <strong>{row.original?.uid}</strong>
+                            str={checked ? getString('disableWebhookContent') : getString('enableWebhookContent')}
+                            vars={{
+                              name: <strong>{row.original?.uid}</strong>
+                            }}
+                          />
                         </Text>
                         <Layout.Horizontal>
                           <Button
                             variation={ButtonVariation.PRIMARY}
                             text={getString('confirm')}
                             onClick={() => {
-                              const data = { enabled: !checked }
+                              const data = { state: checked ? 'disabled' : 'active' }
                               mutate(data)
                                 .then(() => {
                                   showSuccess(getString('branchProtection.ruleUpdated'))

web/src/components/RuleViolationAlertModal/RuleViolationAlertModal.module.scss

@@ -1,5 +1,5 @@
 .dialog {
-  max-width: 500px;
+  max-width: 700px;
 }
 
 .warningIcon {
@@ -15,6 +15,6 @@
 .ruleContainer {
   border-radius: 4px !important;
   background: var(--primary-bg) !important;
-  width: 380px !important;
+  width: fit-content !important;
   height: 36px !important;
 }

web/src/framework/strings/stringTypes.ts

@@ -229,6 +229,7 @@ export interface StringsMap {
   enableWebhookContent: string
   enableWebhookTitle: string
   enabled: string
+  enterANumber: string
   enterAddress: string
   enterBitbucketPlaceholder: string
   enterBranchPlaceholder: string
@@ -708,7 +709,9 @@ export interface StringsMap {
   selectRange: string
   selectSpace: string
   selectSpaceText: string
+  selectStatuses: string
   selectToViewMore: string
+  selectUsers: string
   setAsAdmin: string
   setting: string
   settings: string
@@ -822,7 +825,6 @@ export interface StringsMap {
   viewRaw: string
   viewRepo: string
   viewed: string
-  waitForApproval: string
   webhook: string
   webhookAllEventsSelected: string
   webhookBranchCreated: string

web/src/i18n/strings.en.yaml

@@ -368,7 +368,7 @@ webhookPRCommentCreated: PR comment created
 nameYourWebhook: Name your webhook
 submitReview: Submit Review
 approve: Approve
-requestChanges: Request changes
+requestChanges: Changes Requested
 repoEmptyMarkdown: |
   ## This repository is empty
 
@@ -439,8 +439,8 @@ reviewers: Reviewers
 refresh: Refresh
 enableWebhookTitle: Enable the Webhook
 disableWebhookTitle: Disable the Webhook
-enableWebhookContent: Please confirm that you wanted to turn on
+enableWebhookContent: Please confirm that you wanted to turn on {name}
-disableWebhookContent: Please confirm that you wanted to turn off
+disableWebhookContent: Please confirm that you wanted to turn off {name}
 ascending: Ascending
 descending: Descending
 active: Active
@@ -919,5 +919,7 @@ codeOwner:
   waitToApprove: '{count} pending {count|1:approval,approvals} '
   approvalCompleted: '{count}/{total} approvals completed'
 approved: Approved
-waitForApproval: Wait for Approval
 comingSoon: Coming soon...
+enterANumber: Enter a number
+selectUsers: Select Users
+selectStatuses: Select Statuses

web/src/pages/PullRequest/CodeOwners/CodeOwnersOverview.module.scss

@@ -15,6 +15,7 @@
  */
 
 .main {
+  //   --layout-spacing: var(--layout-medium) !important;
   border-radius: 4px;
   border: 1px solid var(--border-color);
   background: var(--white);

web/src/pages/PullRequest/CodeOwners/CodeOwnersOverview.tsx

@@ -101,7 +101,7 @@ export function CodeOwnersOverview({
   return codeOwners?.evaluation_entries?.length ? (
     <Container
       className={css.main}
-      margin={{ bottom: pullRequestMetadata.description ? undefined : 'large' }}
+      margin={{ top: 'medium', bottom: pullRequestMetadata.description ? undefined : 'large' }}
       style={{ '--border-color': Utils.getRealCSSColor(borderColor) } as React.CSSProperties}>
       <Match expr={isExpanded}>
         <Truthy>
@@ -269,7 +269,7 @@ const CodeOwnerSection: React.FC<CodeOwnerSectionsProps> = ({ data }) => {
             return (
               <Text flex className={cx(css.approvalText, css.waitingContainer)} color={Color.ORANGE_700}>
                 <Container className={css.circle}></Container>
-                {getString('waitForApproval')}
+                {getString('pending')}
               </Text>
             )
           }

web/src/pages/PullRequest/Conversation/PullRequestActionsBox/PullRequestActionsBox.tsx

@@ -48,7 +48,6 @@ import { useAppContext } from 'AppContext'
 import { Images } from 'images'
 import {
   extractInfoFromRuleViolationArr,
-  FieldCheck,
   getErrorMessage,
   MergeCheckStatus,
   permissionProps,
@@ -62,7 +61,7 @@ import ReviewSplitButton from 'components/Changes/ReviewSplitButton/ReviewSplitB
 import RuleViolationAlertModal from 'components/RuleViolationAlertModal/RuleViolationAlertModal'
 import css from './PullRequestActionsBox.module.scss'
 
-const POLLING_INTERVAL = 20000
+const POLLING_INTERVAL = 60000
 export const PullRequestActionsBox: React.FC<PullRequestActionsBoxProps> = ({
   repoMetadata,
   pullRequestMetadata,
@@ -100,11 +99,11 @@ export const PullRequestActionsBox: React.FC<PullRequestActionsBoxProps> = ({
     () => pullRequestMetadata.merge_check_status === MergeCheckStatus.UNCHECKED && !isClosed,
     [pullRequestMetadata, isClosed]
   )
+
   useEffect(() => {
     if (ruleViolationArr && !isDraft && ruleViolationArr.data.rule_violations) {
       const { checkIfBypassAllowed, violationArr, uniqueViolations } = extractInfoFromRuleViolationArr(
-        ruleViolationArr.data.rule_violations,
+        ruleViolationArr.data.rule_violations
-        fieldsToCheck
       )
       setNotBypassable(checkIfBypassAllowed)
       setFinalRulesArr(violationArr)
@@ -112,25 +111,35 @@ export const PullRequestActionsBox: React.FC<PullRequestActionsBoxProps> = ({
     }
     // eslint-disable-next-line react-hooks/exhaustive-deps
   }, [ruleViolationArr])
-  useEffect(() => {
+  const dryMerge = () => {
-    const dryMerge = () => {
+    if (!isClosed && pullRequestMetadata.state !== PullRequestState.MERGED) {
-      if (!isClosed) {
+      mergePR({ bypass_rules: true, dry_run: true, source_sha: pullRequestMetadata?.source_sha })
-        mergePR({ bypass_rules: true, dry_run: true, method: 'merge', source_sha: pullRequestMetadata?.source_sha })
+        .then(res => {
-          .then(res => {
+          if (res?.rule_violations?.length > 0) {
-            if (res?.rule_violations?.length > 0) {
+            setRuleViolation(true)
-              setRuleViolation(true)
+            setRuleViolationArr({ data: { rule_violations: res?.rule_violations } })
-              setRuleViolationArr({ data: { rule_violations: res?.rule_violations } })
+            setAllowedStrats(res.allowed_methods)
-            } else {
+          } else {
-              setRuleViolation(false)
+            setRuleViolation(false)
-            }
+            setAllowedStrats(res.allowed_methods)
-          })
+          }
-          .catch(err => {
+        })
+        .catch(err => {
+          if (err.status === 422) {
             setRuleViolation(true)
             setRuleViolationArr(err)
-          })
+            setAllowedStrats(err.allowed_methods)
-      }
+          } else {
+            showError(getErrorMessage(err))
+          }
+        })
     }
-    dryMerge()
+  }
+  useEffect(() => {
+    dryMerge() // eslint-disable-next-line react-hooks/exhaustive-deps
+  }, [])
+  useEffect(() => {
+    // dryMerge()
     const intervalId = setInterval(async () => {
       dryMerge()
     }, POLLING_INTERVAL) // Poll every 20 seconds
@@ -165,6 +174,12 @@ export const PullRequestActionsBox: React.FC<PullRequestActionsBoxProps> = ({
       desc: getString('pr.mergeOptions.closeDesc')
     }
   ]
+  const [allowedStrats, setAllowedStrats] = useState<string[]>([
+    mergeOptions[0].method,
+    mergeOptions[1].method,
+    mergeOptions[2].method,
+    mergeOptions[3].method
+  ])
   const draftOptions: PRDraftOption[] = [
     {
       method: 'open',
@@ -178,25 +193,21 @@ export const PullRequestActionsBox: React.FC<PullRequestActionsBoxProps> = ({
     }
   ]
 
-  const fieldsToCheck: FieldCheck = {
-    'pullreq.approvals.require_minimum_count': getString('branchProtection.requireMinReviewersTitle'),
-    'pullreq.approvals.require_code_owners': getString('branchProtection.reqReviewFromCodeOwnerTitle'),
-    'pullreq.approvals.require_latest_commit': getString('branchProtection.reqNewChangesTitle'),
-    'pullreq.comments.require_resolve_all': getString('branchProtection.reqCommentResolutionTitle'),
-    'pullreq.status_checks.all_must_succeed': getString('branchProtection.reqStatusChecksTitle'),
-    'pullreq.status_checks.require_uids': getString('branchProtection.reqStatusChecksTitle'),
-    'pullreq.merge.strategies_allowed': getString('branchProtection.limitMergeStrategies'),
-    'pullreq.merge.delete_branch': getString('branchProtection.autoDeleteTitle'),
-    'lifecycle.create_forbidden': getString('branchProtection.blockBranchCreation'),
-    'lifecycle.delete_forbidden': getString('branchProtection.blockBranchDeletion'),
-    'lifecycle.update_forbidden': getString('branchProtection.requirePr')
-  }
-
   const [mergeOption, setMergeOption, resetMergeOption] = useUserPreference<PRMergeOption>(
     UserPreference.PULL_REQUEST_MERGE_STRATEGY,
     mergeOptions[0],
     option => option.method !== 'close'
   )
+  useEffect(() => {
+    if (allowedStrats) {
+      const matchingMethods = mergeOptions.filter(option => allowedStrats.includes(option.method))
+      if (matchingMethods.length > 0) {
+        setMergeOption(matchingMethods[0])
+      }
+    } else {
+      setMergeOption(mergeOptions[3])
+    } // eslint-disable-next-line react-hooks/exhaustive-deps
+  }, [allowedStrats])
   const [draftOption, setDraftOption] = useState<PRDraftOption>(draftOptions[0])
   const permPushResult = hooks?.usePermissionTranslate?.(
     {
@@ -216,7 +227,6 @@ export const PullRequestActionsBox: React.FC<PullRequestActionsBoxProps> = ({
   if (pullRequestMetadata.state === PullRequestFilterOption.MERGED) {
     return <MergeInfo pullRequestMetadata={pullRequestMetadata} />
   }
-
   return (
     <Container
       className={cx(css.main, {
@@ -276,7 +286,7 @@ export const PullRequestActionsBox: React.FC<PullRequestActionsBoxProps> = ({
                   : ruleViolation
                   ? 'branchProtection.prFailedText'
                   : 'pr.branchHasNoConflicts',
-                ruleViolation ? { ruleCount: length } : {}
+                ruleViolation ? { ruleCount: length } : { ruleCount: 0 }
               )}
               {ruleViolation && mergeable && !isDraft ? (
                 <Button
@@ -439,11 +449,12 @@ export const PullRequestActionsBox: React.FC<PullRequestActionsBoxProps> = ({
                               }
                             }}>
                             {mergeOptions.map(option => {
+                              const mergeCheck = allowedStrats !== undefined && allowedStrats.includes(option.method)
                               return (
                                 <Menu.Item
                                   key={option.method}
                                   className={css.menuItem}
-                                  disabled={option.disabled}
+                                  disabled={option.method !== 'close' ? !mergeCheck : option.disabled}
                                   text={
                                     <>
                                       <BIcon icon={mergeOption.method === option.method ? 'tick' : 'blank'} />

web/src/utils/Utils.ts

@@ -76,15 +76,13 @@ export interface PageBrowserProps {
   page: string
 }
 
-export const extractInfoFromRuleViolationArr = (ruleViolationArr: TypesRuleViolations[], fieldsToCheck: FieldCheck) => {
+export const extractInfoFromRuleViolationArr = (ruleViolationArr: TypesRuleViolations[]) => {
   // eslint-disable-next-line @typescript-eslint/no-explicit-any
   const tempArray: any[] = ruleViolationArr?.flatMap(
-    (item: { violations?: TypesViolation[] | null }) => item?.violations?.map(violation => violation.code) ?? []
+    (item: { violations?: TypesViolation[] | null }) => item?.violations?.map(violation => violation.message) ?? []
   )
   const uniqueViolations = new Set(tempArray)
-  const violationArr = [...uniqueViolations]
+  const violationArr = [...uniqueViolations].map(violation => ({ violation: violation }))
-    .filter(violation => violation in fieldsToCheck)
-    .map(violation => ({ violation: fieldsToCheck[violation] }))
 
   const checkIfBypassAllowed = ruleViolationArr.some(ruleViolation => ruleViolation.bypassed === false)