Skip to content

Commit

Permalink
feat: remove util isCheckApplicableToProjectCategory and levels ref
Browse files Browse the repository at this point in the history
  • Loading branch information
UlisesGascon committed Dec 24, 2024
1 parent 41de39f commit 253ea65
Show file tree
Hide file tree
Showing 8 changed files with 4 additions and 87 deletions.
1 change: 0 additions & 1 deletion .github/ISSUE_TEMPLATE/add-a-new-compliance-check.md
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,6 @@ You can find more details in [the contributing guide](/CONTRIBUTING.md#current-i
- [ ] **3. Implement the Business Logic [Validator Example](https://github.com/OpenPathfinder/visionBoard/commit/44c41d119f0daefb7b2e496ba35d5ab65bcc319b) and [Check Example](https://github.com/OpenPathfinder/visionBoard/commit/6f1e16129ee0d01a1b9b536cd2dc6090b048b71f)**
- [ ] Add the specific validator in `src/checks/validators/index.js`
- [ ] Add the check logic in `src/checks/complianceChecks`
- [ ] Ensure that the check is in scope for the organization (use `isCheckApplicableToProjectCategory`)
- [ ] Ensure that the `severity` value is well calculated (use `getSeverityFromPriorityGroup`)
- [ ] Add the alert row in the `compliance_checks_alerts` table when is needed.
- [ ] Add the task row in the `compliance_checks_tasks` table when is needed.
Expand Down
2 changes: 1 addition & 1 deletion .github/workflows/review-compliance-checks.yml
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,7 @@ jobs:
"- [ ] Have you updated the compliance check in the `compliance_checks` table?\n" +
"- [ ] Have you included a specific validator (`src/checks/validators/`) for this check with unit tests (`__tests__/checks/`)?\n" +
"- [ ] Have you included a specific file in `src/checks/complianceChecks` with the integration tests (`__tests__/checks/`)?\n" +
"- [ ] Have you included severity validation (`getSeverityFromPriorityGroup`) and checked applicability (`isCheckApplicableToProjectCategory`)?\n" +
"- [ ] Have you included severity validation (`getSeverityFromPriorityGroup`)?\n" +
"- [ ] Have you included the tasks, alerts, and results in the database tables?\n" +
"- [ ] Have you tested the check with `check run --name {check_code_name}` using the seeded database (`npm run db:seed`)?\n" +
"- [ ] Have you created a PR in [the website](https://github.com/OpenPathfinder/website) with the calculation details?\n" +
Expand Down
1 change: 0 additions & 1 deletion CONTRIBUTING.md
Original file line number Diff line number Diff line change
Expand Up @@ -143,7 +143,6 @@ We are looking for contributors to implement compliance checks in the Dashboard.
- **3. Implement the Business Logic ([Validator Example](https://github.com/OpenPathfinder/visionBoard/commit/44c41d119f0daefb7b2e496ba35d5ab65bcc319b) and [Check Example](https://github.com/OpenPathfinder/visionBoard/commit/6f1e16129ee0d01a1b9b536cd2dc6090b048b71f)):**
- Add the specific validator in `src/checks/validators/index.js`.
- Add the check logic in `src/checks/complianceChecks`.
- Ensure the check is applicable to the organization (`isCheckApplicableToProjectCategory`).
- Calculate `severity` accurately (`getSeverityFromPriorityGroup`).
- Update relevant database tables (`compliance_checks_alerts`, `compliance_checks_tasks`, `compliance_checks_results`).

Expand Down
29 changes: 0 additions & 29 deletions __tests__/checks/validators.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -25,9 +25,6 @@ describe('githubOrgMFA', () => {
id: 1,
priority_group: 'P1',
details_url: 'https://example.com',
level_incubating_status: 'expected',
level_active_status: 'expected',
level_retiring_status: 'expected'
}

projects = [
Expand Down Expand Up @@ -133,18 +130,6 @@ describe('githubOrgMFA', () => {
tasks: []
})
})

it('Should skip the check if it is not in scope for the project category', () => {
check.level_active_status = 'n/a'
check.level_incubating_status = 'n/a'
check.level_retiring_status = 'n/a'
const analysis = githubOrgMFA({ organizations, check, projects })
expect(analysis).toEqual({
alerts: [],
results: [],
tasks: []
})
})
})

describe('softwareDesignTraining', () => {
Expand All @@ -165,9 +150,6 @@ describe('softwareDesignTraining', () => {
id: 1,
priority_group: 'P1',
details_url: 'https://example.com',
level_incubating_status: 'expected',
level_active_status: 'expected',
level_retiring_status: 'expected'
}

projects = [
Expand Down Expand Up @@ -299,15 +281,4 @@ describe('softwareDesignTraining', () => {
]
})
})
it('Should skip the check if it is not in scope for the project category', () => {
check.level_active_status = 'n/a'
check.level_incubating_status = 'n/a'
check.level_retiring_status = 'n/a'
const analysis = softwareDesignTraining({ trainings, check, projects })
expect(analysis).toEqual({
alerts: [],
results: [],
tasks: []
})
})
})
27 changes: 1 addition & 26 deletions __tests__/utils.test.js
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
const { validateGithubUrl, ensureGithubToken, groupArrayItemsByCriteria, isCheckApplicableToProjectCategory, getSeverityFromPriorityGroup, isDateWithinPolicy, redactSensitiveData } = require('../src/utils/index')
const { validateGithubUrl, ensureGithubToken, groupArrayItemsByCriteria, getSeverityFromPriorityGroup, isDateWithinPolicy, redactSensitiveData } = require('../src/utils/index')

describe('ensureGithubToken', () => {
let originalGithubToken
Expand Down Expand Up @@ -66,31 +66,6 @@ describe('groupArrayItemsByCriteria', () => {
})
})

describe('isCheckApplicableToProjectCategory', () => {
const disabledCheck = {
level_active_status: 'n/a',
level_incubating_status: 'n/a',
level_retiring_status: 'n/a'
}

it('should return false if the check is not applicable to the project category', () => {
let project = { category: 'impact' }
expect(isCheckApplicableToProjectCategory(disabledCheck, project)).toBe(false)
project = { category: 'incubation' }
expect(isCheckApplicableToProjectCategory(disabledCheck, project)).toBe(false)
project = { category: 'emeritus' }
expect(isCheckApplicableToProjectCategory(disabledCheck, project)).toBe(false)
project = { category: 'at-large' }
expect(isCheckApplicableToProjectCategory(disabledCheck, project)).toBe(false)
})

it('should return true if the check is applicable to the project category', () => {
const project = { category: 'impact' }
const check = { ...disabledCheck, level_active_status: 'recommended' }
expect(isCheckApplicableToProjectCategory(check, project)).toBe(true)
})
})

describe('getSeverityFromPriorityGroup', () => {
it('should return the correct severity based on the priority group', () => {
expect(getSeverityFromPriorityGroup('P0')).toBe('critical')
Expand Down
8 changes: 1 addition & 7 deletions src/checks/validators/githubOrgMFA.js
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
const debug = require('debug')('checks:validator:githubOrgMFA')
const { getSeverityFromPriorityGroup, isCheckApplicableToProjectCategory, groupArrayItemsByCriteria } = require('../../utils')
const { getSeverityFromPriorityGroup, groupArrayItemsByCriteria } = require('../../utils')

const groupByProject = groupArrayItemsByCriteria('project_id')

Expand All @@ -17,12 +17,6 @@ module.exports = ({ organizations = [], check, projects = [] }) => {
organizationsGroupedByProject.forEach((projectOrgs) => {
debug(`Processing project (${projectOrgs[0].project_id})`)
const project = projects.find(p => p.id === projectOrgs[0].project_id)
const isInScope = isCheckApplicableToProjectCategory(check, project)
// If the check is not in scope, skip it.
if (!isInScope) {
debug(`This check is not in scope for project (${project.id})`)
return
}

const baseData = {
project_id: projectOrgs[0].project_id,
Expand Down
9 changes: 1 addition & 8 deletions src/checks/validators/softwareDesignTraining.js
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
const debug = require('debug')('checks:validator:softwareDesignTraining')
const { isCheckApplicableToProjectCategory, getSeverityFromPriorityGroup, isDateWithinPolicy } = require('../../utils')
const { getSeverityFromPriorityGroup, isDateWithinPolicy } = require('../../utils')

const expirationPolicy = '6m'

Expand All @@ -11,13 +11,6 @@ module.exports = ({ trainings = [], check, projects = [] }) => {

debug('Processing Projects...')
projects.forEach(project => {
const isInScope = isCheckApplicableToProjectCategory(check, project)
// If the check is not in scope, skip it.
if (!isInScope) {
debug(`This check is not in scope for project (${project.id})`)
return
}

const baseData = {
project_id: project.id,
compliance_check_id: check.id,
Expand Down
14 changes: 0 additions & 14 deletions src/utils/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -61,19 +61,6 @@ const getSeverityFromPriorityGroup = (priorityGroup) => {
}
}

const isCheckApplicableToProjectCategory = (check, project) => {
if (['impact', 'at-large'].includes(project.category) && check.level_active_status === 'n/a') {
return false
}
if (project.category === 'incubation' && check.level_incubating_status === 'n/a') {
return false
}
if (project.category === 'emeritus' && check.level_retiring_status === 'n/a') {
return false
}
return true
}

const groupArrayItemsByCriteria = criteria => items => Object.values(
items.reduce((acc, item) => {
if (!acc[item[criteria]]) {
Expand Down Expand Up @@ -124,7 +111,6 @@ module.exports = {
validateGithubUrl,
ensureGithubToken,
getSeverityFromPriorityGroup,
isCheckApplicableToProjectCategory,
groupArrayItemsByCriteria,
redactSensitiveData,
logger
Expand Down

0 comments on commit 253ea65

Please sign in to comment.