From 253ea6523556e72c77b33e94efdd9fa7d1d4952b Mon Sep 17 00:00:00 2001 From: Ulises Gascon Date: Tue, 24 Dec 2024 17:04:32 +0100 Subject: [PATCH] feat: remove util `isCheckApplicableToProjectCategory` and levels ref --- .../add-a-new-compliance-check.md | 1 - .../workflows/review-compliance-checks.yml | 2 +- CONTRIBUTING.md | 1 - __tests__/checks/validators.test.js | 29 ------------------- __tests__/utils.test.js | 27 +---------------- src/checks/validators/githubOrgMFA.js | 8 +---- .../validators/softwareDesignTraining.js | 9 +----- src/utils/index.js | 14 --------- 8 files changed, 4 insertions(+), 87 deletions(-) diff --git a/.github/ISSUE_TEMPLATE/add-a-new-compliance-check.md b/.github/ISSUE_TEMPLATE/add-a-new-compliance-check.md index 05ab506..841f2b6 100644 --- a/.github/ISSUE_TEMPLATE/add-a-new-compliance-check.md +++ b/.github/ISSUE_TEMPLATE/add-a-new-compliance-check.md @@ -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. diff --git a/.github/workflows/review-compliance-checks.yml b/.github/workflows/review-compliance-checks.yml index 65b3196..8b5d43c 100644 --- a/.github/workflows/review-compliance-checks.yml +++ b/.github/workflows/review-compliance-checks.yml @@ -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" + diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index 8c082a3..9912157 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -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`). diff --git a/__tests__/checks/validators.test.js b/__tests__/checks/validators.test.js index 3f286d5..43a101c 100644 --- a/__tests__/checks/validators.test.js +++ b/__tests__/checks/validators.test.js @@ -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 = [ @@ -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', () => { @@ -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 = [ @@ -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: [] - }) - }) }) diff --git a/__tests__/utils.test.js b/__tests__/utils.test.js index 68ef298..d2b704a 100644 --- a/__tests__/utils.test.js +++ b/__tests__/utils.test.js @@ -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 @@ -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') diff --git a/src/checks/validators/githubOrgMFA.js b/src/checks/validators/githubOrgMFA.js index 2f8daf4..80b7f98 100644 --- a/src/checks/validators/githubOrgMFA.js +++ b/src/checks/validators/githubOrgMFA.js @@ -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') @@ -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, diff --git a/src/checks/validators/softwareDesignTraining.js b/src/checks/validators/softwareDesignTraining.js index 872fcca..6ef3049 100644 --- a/src/checks/validators/softwareDesignTraining.js +++ b/src/checks/validators/softwareDesignTraining.js @@ -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' @@ -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, diff --git a/src/utils/index.js b/src/utils/index.js index 6133994..ff16076 100644 --- a/src/utils/index.js +++ b/src/utils/index.js @@ -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]]) { @@ -124,7 +111,6 @@ module.exports = { validateGithubUrl, ensureGithubToken, getSeverityFromPriorityGroup, - isCheckApplicableToProjectCategory, groupArrayItemsByCriteria, redactSensitiveData, logger