Convert EU to a framework #1240
Conversation
|
""" WalkthroughThis update introduces a comprehensive migration of the existing EU compliance and assessment system to a modular, framework-based architecture. It adds new database tables and models for frameworks and project-framework relationships, migrates legacy EU assessment and compliance data into structured, framework-aware schemas, and refactors controllers and utilities to use transactional operations and the new data structures. The server now exposes new endpoints for EU framework operations and ensures atomicity and data integrity across all create, update, and delete operations. Changes
Sequence Diagram(s)sequenceDiagram
participant Client
participant ExpressServer
participant EUController
participant EUUtils
participant SequelizeDB
Client->>ExpressServer: PATCH /eu-ai-act/controls/:id (with files)
ExpressServer->>EUController: saveControls(req, res)
EUController->>EUUtils: updateControlEUByIdQuery(...)
EUUtils->>SequelizeDB: Begin transaction
EUUtils->>SequelizeDB: Update control and subcontrols, upload/delete files
EUUtils->>SequelizeDB: Commit transaction
EUUtils->>EUController: Updated control data
EUController->>ExpressServer: 200 OK with status message
ExpressServer->>Client: Response
Client->>ExpressServer: GET /eu-ai-act/assessment-progress/:projectId
ExpressServer->>EUController: getProjectAssessmentProgress(req, res)
EUController->>EUUtils: countAnswersEUByProjectId(...)
EUUtils->>SequelizeDB: Query assessment/answer counts
EUUtils->>EUController: Progress metrics
EUController->>ExpressServer: 200 OK with progress data
ExpressServer->>Client: Response
Assessment against linked issues
Poem
📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
✨ Finishing Touches
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
HarshP4585
changed the title
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 27
🔭 Outside diff range comments (2)
Servers/controllers/project.ctrl.ts (2)
82-90: 🛠️ Refactor suggestion
⚠️ Potential issueRemove console logs & honour caller-supplied frameworks
Debug
console.logstatements leak to production logs and can expose PII.
Secondly, theframeworkarray sent by the client is discarded later – defeating the purpose of making it part of the API.- console.log("req.body : ", req.body); ... - framework: number[]; + framework: number[]; ... - console.log(newProject); + // validate input instead of logging itFollow up: ensure the value in
newProject.frameworkis validated & forwarded tocreateNewProjectQuery.
196-203:⚠️ Potential issueMissing
await– functions will return unresolved Promises
getUserByIdQueryis asynchronous. BothownerUseranduserWhoUpdatedwill be Promises, so the response will serialise as{}.-const ownerUser: any = getUserByIdQuery(project_owner); +const ownerUser = await getUserByIdQuery(project_owner); ... -const userWhoUpdated: any = getUserByIdQuery(project_last_updated_by); +const userWhoUpdated = await getUserByIdQuery(project_last_updated_by);
🧹 Nitpick comments (34)
Servers/database/migrations/20250422180112-create-table-for-frameworks.js (4)
1-1: Consider removing redundant 'use strict' directive.JavaScript modules are automatically in strict mode, without requiring an explicit directive.
-'use strict';🧰 Tools
🪛 Biome (1.9.4)
[error] 1-1: Redundant use strict directive.
The entire contents of JavaScript modules are automatically in strict mode, with no statement needed to initiate it.
Safe fix: Remove the redundant use strict directive.(lint/suspicious/noRedundantUseStrict)
16-23: Review the primary key structure for better clarity.The table definition includes both a SERIAL UNIQUE column 'id' and a composite PRIMARY KEY. This might lead to confusion about which identifier should be used in relationships.
Consider either:
- Using the composite key only (if that's the intended unique identifier)
- Making the 'id' column the primary key and adding a unique constraint on (project_id, framework_id)
Choose based on how you plan to reference this table from other tables.
43-45: Fix typo in variable name 'prjectsFrameworksInsert'.There's a typo in the variable name that should be corrected.
- const prjectsFrameworksInsert = projectsQuery[0].map((project) => { + const projectsFrameworksInsert = projectsQuery[0].map((project) => { return `(${project.id}, ${framework_id})` }).join(', ');
46-48: Use the corrected variable name in the query.Update the query to use the fixed variable name for consistency.
await queryInterface.sequelize.query( - `INSERT INTO projects_frameworks (project_id, framework_id) VALUES ${prjectsFrameworksInsert};`, { transaction } + `INSERT INTO projects_frameworks (project_id, framework_id) VALUES ${projectsFrameworksInsert};`, { transaction } );Servers/routes/eu.route.ts (2)
9-14: Consider cleaning up commented codeThere are several commented routes that appear to be placeholders or remnants of development. Consider removing these comments if they're no longer needed, or add TODO comments explaining why they're being preserved.
-// router.get("/:id", authenticateJWT, getAssessmentById); -// router.get("/:id", authenticateJWT, getControlById); - -// router.get("/getAnswers/:id", authenticateJWT, getAnswers); -// router.get("/compliance/:id", authenticateJWT, getComplianceById);
31-36: Consider cleaning up additional commented routesMore commented routes are present and should be either removed or documented with TODOs for clarity.
-// router.get("/byassessmentid/:id", authenticateJWT, getTopicByAssessmentId); -// router.get("/bytopic/:id", authenticateJWT, getSubtopicByTopicId); - -// router.get("/bysubtopic/:id", authenticateJWT, getQuestionsBySubtopicId); -// router.get("/bytopic/:id", authenticateJWT, getQuestionsByTopicId);Servers/models/EU/topicStructEU.model.ts (2)
3-8: Unclear comment about model replacementThe comment states "This is the new Topic model(Schema) and will be replaced with the new one." This is confusing - if this is the new model being introduced, it's unclear why it would need to be replaced.
Consider clarifying the comment to better explain the model's purpose and lifecycle, or remove it if it's no longer relevant.
16-18: Consider more specific table namingThe model's tableName is set to "topics" which is generic. Since this is specifically for the EU framework, a more specific name might prevent conflicts with existing or future tables.
Consider using a more specific table name like "topics_eu" or "eu_topics":
@Table({ - tableName: "topics" + tableName: "topics_struct_eu" })Servers/models/EU/subTopicStructEU.model.ts (2)
4-9: Unclear comment about model replacementThe comment states "This is the new Subtopic model(Schema) and will be replaced with the new one." This is confusing - if this is the new model being introduced, it's unclear why it would need to be replaced.
Consider clarifying the comment to better explain the model's purpose and lifecycle, or remove it if it's no longer relevant.
18-20: Consider more specific table namingThe model's tableName is set to "subtopics" which is generic. Since this is specifically for the EU framework, a more specific name might prevent conflicts with existing or future tables.
Consider using a more specific table name like "subtopics_eu" or "eu_subtopics":
@Table({ - tableName: "subtopics" + tableName: "subtopics_struct_eu" })Servers/models/projectFrameworks.model.ts (1)
14-26: Consider adding an auto-incrementing ID fieldCurrently, the model uses a composite primary key of framework_id and project_id. While this works for a many-to-many relationship, having an auto-generated ID field would make it easier to reference specific project-framework relationships, especially from other tables.
Consider adding an auto-incrementing ID field:
export class ProjectFrameworksModel extends Model<ProjectFrameworks> { + @Column({ + type: DataType.INTEGER, + autoIncrement: true, + primaryKey: true, + }) + id!: number; @ForeignKey(() => FrameworkModel) @Column({ type: DataType.INTEGER, - primaryKey: true + allowNull: false }) framework_id!: number; @ForeignKey(() => ProjectModel) @Column({ type: DataType.INTEGER, - primaryKey: true + allowNull: false }) project_id!: number;Servers/models/EU/assessmentEU.model.ts (1)
9-10: Fix typo in commentThere's a typo in the comment: "In fact nothing specific has changedn but we're only" should be "In fact nothing specific has changed but we're only".
-In fact nothing specific has changedn but we're only +In fact nothing specific has changed but we're onlyServers/models/frameworks.model.ts (3)
8-8: Remove trailing semicolon for consistencyThe semicolon after the interface declaration is unnecessary and inconsistent with TypeScript style conventions.
-}; +}
21-29: Consider adding validation constraints for name and descriptionAdd validation constraints such as length limits for the name and description fields to ensure data integrity.
@Column({ - type: DataType.STRING + type: DataType.STRING, + validate: { + len: [1, 100] + } }) name!: string; @Column({ - type: DataType.STRING + type: DataType.STRING, + validate: { + len: [0, 500] + } }) description!: string;
35-35: Remove trailing semicolon for consistencyThe semicolon after the class declaration is unnecessary and inconsistent with TypeScript style conventions.
-}; +}Servers/models/EU/subControlStructEU.model.ts (3)
1-4: Unused import detected.The file imports
UserModelfrom "../user.model" but it's not being used anywhere in this model.import { Column, DataType, ForeignKey, Model, Table } from "sequelize-typescript"; import { ControlEUModel } from "./controlEU.model"; -import { UserModel } from "../user.model";
5-10: Replace placeholder comments with proper documentation.These placeholder comments should be replaced with actual documentation that explains the purpose of this model and how it fits into the EU framework architecture.
-/* - -This is the new Subcontrol model(Schema) and will be replaced with the new one. -Please align other files with this - -*/ +/** + * Model representing the subcontrol structure for EU compliance tracking. + * Contains metadata for subcontrols that are part of a control in the EU framework. + */
11-17: Looks good, but consider adding JSDoc comments.The type definition is clear, but adding JSDoc comments for each property would improve code documentation and provide better IDE tooltips for developers.
Servers/models/EU/controlCategoryStructEU.model.ts (1)
5-10: Replace placeholder comments with proper documentation.These placeholder comments should be replaced with actual documentation that explains the purpose of this model and how it fits into the EU framework architecture.
-/* - -This is the new ControlCategory model(Schema) and will be replaced with the new one. -Please align other files with this - -*/ +/** + * Model representing control categories for EU compliance tracking. + * A control category contains multiple controls and is part of the EU framework structure. + */Servers/models/EU/controlStructEU.model.ts (2)
1-3: Unused import detected.The file imports
Validatefrom "sequelize-typescript" but it's not being used anywhere in this model.-import { Column, DataType, ForeignKey, Model, Table, Validate } from "sequelize-typescript"; +import { Column, DataType, ForeignKey, Model, Table } from "sequelize-typescript"; import { ControlCategoryStructEUModel } from "./controlCategoryStructEU.model";
4-9: Replace placeholder comments with proper documentation.These placeholder comments should be replaced with actual documentation that explains the purpose of this model and how it fits into the EU framework architecture.
-/* - -This is the new Control model(Schema) and will be replaced with the new one. -Please align other files with this - -*/ +/** + * Model representing the control structure for EU compliance tracking. + * Controls are part of a control category and contain implementation details for compliance. + */Servers/models/EU/questionStructEU.model.ts (1)
4-9: Replace placeholder comments with proper documentation.These placeholder comments should be replaced with actual documentation that explains the purpose of this model and how it fits into the EU framework architecture.
-/* - -This is the new Question model(Schema) and will be replaced with the new one. -Please align other files with this - -*/ +/** + * Model representing questions for EU compliance assessment. + * Questions are part of a subtopic and require answers as part of the EU assessment process. + */Servers/controllers/framework.ctrl.ts (1)
32-37: Prefer 204 over 404 for a missing single resource?Currently the code returns
404when the framework isn’t found and includes a JSON payload.
Ensure this is aligned with the API contract. Ifnullis returned, consider204 No Contentwithout a body instead, keeping404for an invalid route / id.Servers/models/EU/controlEU.model.ts (2)
1-2:Validateimport is unusedThe
Validatesymbol is imported but never referenced. Remove it to avoid dead-code & keep build size lean.-import { Column, DataType, ForeignKey, Model, Table, Validate } from "sequelize-typescript"; +import { Column, DataType, ForeignKey, Model, Table } from "sequelize-typescript";
24-29: DTO contains computed-only properties – document this
numberOfSubcontrols,numberOfDoneSubcontrols, andsubControlsexist in the TS type but have no column definitions.
That’s fine (they’re denormalised/calculated) but please add a JSDoc comment stating they are virtual fields so future maintainers don’t try to add migrations for them.Servers/controllers/project.ctrl.ts (2)
105-106: AvoidObjectas a type – use a precise Record-const frameworks: { [key: string]: Object } = {} +const frameworks: Record<string, unknown> = {};This satisfies Biome’s
noBannedTypesrule and improves type-safety.🧰 Tools
🪛 Biome (1.9.4)
[error] 105-105: Don't use 'Object' as a type.
Prefer explicitly define the object shape. This type means "any non-nullable value", which is slightly better than 'unknown', but it's still a broad type.
(lint/complexity/noBannedTypes)
135-141: Prefer reassignment overdeletefor performance & type-safety
delete updatedProject.members;anddelete updatedProject.framework;trigger hidden-class de-opts in V8 and violate Biome’snoDeleterule.-updatedProject.members && (updatedProject.members = undefined); -updatedProject.framework && (updatedProject.framework = undefined); +delete updatedProject.members; // if you really must remove – consider eslint-disable next-lineOr keep the properties and ignore them in the SQL layer.
🧰 Tools
🪛 Biome (1.9.4)
[error] 139-139: Avoid the delete operator which can impact performance.
Unsafe fix: Use an undefined assignment instead.
(lint/performance/noDelete)
[error] 140-140: Avoid the delete operator which can impact performance.
Unsafe fix: Use an undefined assignment instead.
(lint/performance/noDelete)
[error] 141-141: Avoid the delete operator which can impact performance.
Unsafe fix: Use an undefined assignment instead.
(lint/performance/noDelete)
Servers/database/migrations/20250424175531-create-structure-for-assessment-tracker.js (2)
1-1:'use strict'is redundant in ESM/CLI modulesNode executes ES modules & CommonJS modules in strict mode by default.
Safe to delete.🧰 Tools
🪛 Biome (1.9.4)
[error] 1-1: Redundant use strict directive.
The entire contents of JavaScript modules are automatically in strict mode, with no statement needed to initiate it.
Safe fix: Remove the redundant use strict directive.(lint/suspicious/noRedundantUseStrict)
52-56: Inline string interpolation may break on large datasets & poses a riskBuilding raw SQL with template literals inside a loop is brittle and makes SQL-injection audits harder (even if values are internal).
Consider parameterised bulk updates instead:
await Promise.all( assessments[0].map(a => queryInterface.sequelize.query( "UPDATE assessments SET projects_frameworks_id = :pfId WHERE id = :aId", { replacements: { pfId: a.projects_frameworks_id, aId: a.assessment_id }, transaction } ) ) );Servers/models/EU/subControlEU.model.ts (1)
5-10: Out-of-date TODO comment clutters the modelThe block still says “will be replaced with the new one”.
Once the file is merged this will no longer be true and the comment will only confuse future readers.-/* - -This is the new Subcontrol model(Schema) and will be replaced with the new one. -Please align other files with this - -*/ +/* Sub-control EU schema */Servers/database/migrations/20250424175556-create-structure-for-compliance-tracker.js (1)
1-2: Redundant"use strict"in an ES moduleNode treats every file executed by
sequelize-cliin strict mode
already. Keeping the directive is harmless but noisy.-'use strict';🧰 Tools
🪛 Biome (1.9.4)
[error] 1-1: Redundant use strict directive.
The entire contents of JavaScript modules are automatically in strict mode, with no statement needed to initiate it.
Safe fix: Remove the redundant use strict directive.(lint/suspicious/noRedundantUseStrict)
Servers/utils/project.utils.ts (1)
417-500: PreferbooleanoverBooleanin promise signatureUpper-case
Booleanis a wrapper object and trips static-analysis
rules.-export const deleteProjectByIdQuery = async ( - id: number -): Promise<Boolean> => { +export const deleteProjectByIdQuery = async ( + id: number +): Promise<boolean> => {🧰 Tools
🪛 Biome (1.9.4)
[error] 419-419: Don't use 'Boolean' as a type.
Use lowercase primitives for consistency.
Safe fix: Use 'boolean' instead(lint/complexity/noBannedTypes)
Servers/controllers/eu.ctrl.ts (1)
345-354: Minor: variable shadowing/race-free but misleadingUsing
Promise.allwith outer-scoped mutating variables (totalNumberOfSubcontrols,totalNumberOfDoneSubcontrols) is safe in Node’s single thread, yet hard to reason about. Prefer local accumulation and a final merge to avoid accidental misuse.Servers/utils/eu.utils.ts (1)
18-36: Replace bannedString/Objecttypes with precise definitionsStatic analysis rightly flags the broad aliases.
Example fix:-const getDemoAnswers = (): String[] => { +const getDemoAnswers = (): string[] => {Define explicit interfaces for the
controlsarray instead ofObject[]to improve type-safety.🧰 Tools
🪛 Biome (1.9.4)
[error] 18-18: Don't use 'String' as a type.
Use lowercase primitives for consistency.
Safe fix: Use 'string' instead(lint/complexity/noBannedTypes)
[error] 30-30: Don't use 'Object' as a type.
Prefer explicitly define the object shape. This type means "any non-nullable value", which is slightly better than 'unknown', but it's still a broad type.
(lint/complexity/noBannedTypes)
[error] 31-31: Don't use 'Object' as a type.
Prefer explicitly define the object shape. This type means "any non-nullable value", which is slightly better than 'unknown', but it's still a broad type.
(lint/complexity/noBannedTypes)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (29)
Servers/controllers/eu.ctrl.ts(1 hunks)Servers/controllers/file.ctrl.ts(4 hunks)Servers/controllers/framework.ctrl.ts(1 hunks)Servers/controllers/project.ctrl.ts(5 hunks)Servers/database/db.ts(2 hunks)Servers/database/migrations/20250418170914-add-status-for-questions.js(1 hunks)Servers/database/migrations/20250422180112-create-table-for-frameworks.js(1 hunks)Servers/database/migrations/20250424175531-create-structure-for-assessment-tracker.js(1 hunks)Servers/database/migrations/20250424175556-create-structure-for-compliance-tracker.js(1 hunks)Servers/index.ts(2 hunks)Servers/models/EU/answerEU.model.ts(1 hunks)Servers/models/EU/assessmentEU.model.ts(1 hunks)Servers/models/EU/controlCategoryStructEU.model.ts(1 hunks)Servers/models/EU/controlEU.model.ts(1 hunks)Servers/models/EU/controlStructEU.model.ts(1 hunks)Servers/models/EU/questionStructEU.model.ts(1 hunks)Servers/models/EU/subControlEU.model.ts(1 hunks)Servers/models/EU/subControlStructEU.model.ts(1 hunks)Servers/models/EU/subTopicStructEU.model.ts(1 hunks)Servers/models/EU/topicStructEU.model.ts(1 hunks)Servers/models/frameworks.model.ts(1 hunks)Servers/models/projectFrameworks.model.ts(1 hunks)Servers/routes/eu.route.ts(1 hunks)Servers/structures/assessment-tracker/subtopics/03-data-governance.subtopic.ts(1 hunks)Servers/structures/assessment-tracker/subtopics/04-technical-documentation.subtopic.ts(1 hunks)Servers/utils/eu.utils.ts(1 hunks)Servers/utils/fileUpload.utils.ts(2 hunks)Servers/utils/framework.utils.ts(1 hunks)Servers/utils/project.utils.ts(9 hunks)
🧰 Additional context used
🧬 Code Graph Analysis (10)
Servers/controllers/file.ctrl.ts (2)
Servers/utils/fileUpload.utils.ts (1)
uploadFile(7-44)Servers/utils/eu.utils.ts (1)
addFileToAnswerEU(609-665)
Servers/models/EU/topicStructEU.model.ts (2)
Servers/models/EU/controlCategoryStructEU.model.ts (1)
Table(19-51)Servers/models/EU/subTopicStructEU.model.ts (1)
Table(18-56)
Servers/routes/eu.route.ts (1)
Servers/controllers/eu.ctrl.ts (12)
getAssessmentsByProjectId(14-27)getCompliancesByProjectId(29-42)getProjectComplianceProgress(285-304)getProjectAssessmentProgress(264-283)getAllProjectsComplianceProgress(338-371)getAllProjectsAssessmentProgress(306-335)getTopicById(44-62)getControlById(64-82)saveControls(84-200)updateQuestionById(202-226)deleteAssessmentsByProjectId(228-244)deleteCompliancesByProjectId(246-262)
Servers/database/migrations/20250422180112-create-table-for-frameworks.js (2)
Servers/database/migrations/20250424175531-create-structure-for-assessment-tracker.js (2)
queries(64-103)queries(212-218)Servers/database/migrations/20250424175556-create-structure-for-compliance-tracker.js (1)
queries(238-244)
Servers/models/EU/questionStructEU.model.ts (1)
Servers/models/EU/subTopicStructEU.model.ts (1)
Table(18-56)
Servers/database/migrations/20250424175531-create-structure-for-assessment-tracker.js (1)
Servers/mocks/question.mock.data.ts (1)
questions(3-1977)
Servers/controllers/framework.ctrl.ts (1)
Servers/utils/framework.utils.ts (2)
getAllFrameworksQuery(5-26)getAllFrameworkByIdQuery(28-50)
Servers/models/projectFrameworks.model.ts (1)
Servers/models/frameworks.model.ts (1)
Table(10-35)
Servers/models/EU/subControlEU.model.ts (2)
Servers/models/EU/controlStructEU.model.ts (1)
Table(19-63)Servers/models/EU/controlEU.model.ts (1)
Table(31-104)
Servers/models/EU/controlEU.model.ts (2)
Servers/models/EU/subControlEU.model.ts (2)
SubcontrolEU(11-27)Table(29-116)Servers/models/EU/controlCategoryStructEU.model.ts (1)
Table(19-51)
🪛 Biome (1.9.4)
Servers/database/migrations/20250422180112-create-table-for-frameworks.js
[error] 1-1: Redundant use strict directive.
The entire contents of JavaScript modules are automatically in strict mode, with no statement needed to initiate it.
Safe fix: Remove the redundant use strict directive.
(lint/suspicious/noRedundantUseStrict)
Servers/models/EU/answerEU.model.ts
[error] 12-12: Don't use 'Object' as a type.
Prefer explicitly define the object shape. This type means "any non-nullable value", which is slightly better than 'unknown', but it's still a broad type.
(lint/complexity/noBannedTypes)
Servers/database/migrations/20250424175531-create-structure-for-assessment-tracker.js
[error] 1-1: Redundant use strict directive.
The entire contents of JavaScript modules are automatically in strict mode, with no statement needed to initiate it.
Safe fix: Remove the redundant use strict directive.
(lint/suspicious/noRedundantUseStrict)
Servers/models/EU/subControlEU.model.ts
[error] 22-22: Don't use 'Object' as a type.
Prefer explicitly define the object shape. This type means "any non-nullable value", which is slightly better than 'unknown', but it's still a broad type.
(lint/complexity/noBannedTypes)
[error] 23-23: Don't use 'Object' as a type.
Prefer explicitly define the object shape. This type means "any non-nullable value", which is slightly better than 'unknown', but it's still a broad type.
(lint/complexity/noBannedTypes)
Servers/controllers/project.ctrl.ts
[error] 105-105: Don't use 'Object' as a type.
Prefer explicitly define the object shape. This type means "any non-nullable value", which is slightly better than 'unknown', but it's still a broad type.
(lint/complexity/noBannedTypes)
[error] 139-139: Avoid the delete operator which can impact performance.
Unsafe fix: Use an undefined assignment instead.
(lint/performance/noDelete)
[error] 140-140: Avoid the delete operator which can impact performance.
Unsafe fix: Use an undefined assignment instead.
(lint/performance/noDelete)
Servers/database/migrations/20250424175556-create-structure-for-compliance-tracker.js
[error] 1-1: Redundant use strict directive.
The entire contents of JavaScript modules are automatically in strict mode, with no statement needed to initiate it.
Safe fix: Remove the redundant use strict directive.
(lint/suspicious/noRedundantUseStrict)
Servers/utils/project.utils.ts
[error] 419-419: Don't use 'Boolean' as a type.
Use lowercase primitives for consistency.
Safe fix: Use 'boolean' instead
(lint/complexity/noBannedTypes)
Servers/utils/eu.utils.ts
[error] 18-18: Don't use 'String' as a type.
Use lowercase primitives for consistency.
Safe fix: Use 'string' instead
(lint/complexity/noBannedTypes)
[error] 30-30: Don't use 'Object' as a type.
Prefer explicitly define the object shape. This type means "any non-nullable value", which is slightly better than 'unknown', but it's still a broad type.
(lint/complexity/noBannedTypes)
[error] 31-31: Don't use 'Object' as a type.
Prefer explicitly define the object shape. This type means "any non-nullable value", which is slightly better than 'unknown', but it's still a broad type.
(lint/complexity/noBannedTypes)
[error] 334-334: Don't use 'Object' as a type.
Prefer explicitly define the object shape. This type means "any non-nullable value", which is slightly better than 'unknown', but it's still a broad type.
(lint/complexity/noBannedTypes)
[error] 361-361: Don't use 'String' as a type.
Use lowercase primitives for consistency.
Safe fix: Use 'string' instead
(lint/complexity/noBannedTypes)
[error] 485-485: Don't use 'Object' as a type.
Prefer explicitly define the object shape. This type means "any non-nullable value", which is slightly better than 'unknown', but it's still a broad type.
(lint/complexity/noBannedTypes)
🔇 Additional comments (28)
Servers/database/migrations/20250422180112-create-table-for-frameworks.js (2)
5-54: LGTM! Transaction handling is properly implemented.The migration correctly uses transactions for atomicity, with appropriate error handling and rollback in case of failures. The structure with try/catch ensures that any errors during the migration will trigger a rollback.
56-71: LGTM! Down migration correctly reverts changes.The down migration properly drops tables in the correct order (child table before parent table) to avoid foreign key constraint violations.
Servers/index.ts (2)
23-23: LGTM! New EU router import added.The import for the EU router has been correctly added.
89-89: LGTM! EU router registered properly.The EU router is correctly registered with the Express app under the "/eu" path.
Servers/structures/assessment-tracker/subtopics/04-technical-documentation.subtopic.ts (1)
24-24: LGTM! Properly escaped apostrophe in string.The apostrophe in "organization's" has been correctly escaped as "organization''s" to prevent SQL string issues.
Servers/structures/assessment-tracker/subtopics/03-data-governance.subtopic.ts (1)
155-155: LGTM! Corrected punctuation from period to question mark.Properly changed the ending punctuation from a period to a question mark to correctly format the text as a question.
Servers/database/db.ts (2)
22-33: Well-structured imports for the new EU framework modelsThe imports for the new Framework models and related EU-specific models follow a clear and consistent naming convention, which will make the codebase more maintainable.
65-77: Proper registration of new models in SequelizeAll new models are correctly registered in the Sequelize configuration, allowing them to be used throughout the application. The models are added in a logical order, with framework models first followed by the EU-specific models.
Servers/controllers/file.ctrl.ts (4)
6-6: Correct import of the new EU utility functionThe import of
addFileToAnswerEUfrom EU utilities aligns with the framework conversion effort.
58-58: Updated parameter naming for framework architectureThe parameter renaming from
project_idtoproject_framework_idcorrectly aligns with the new framework-based structure.
71-71: Adapted file upload to use project_framework_idThe file upload now correctly uses the
project_framework_idparameter instead ofproject_id, which is consistent with the framework architecture changes.
82-82: Migrated to EU-specific answer file handlingThe function now uses
addFileToAnswerEUinstead of the previousaddFileToQuestionfunction, properly implementing the framework-based approach for file attachments.Servers/database/migrations/20250418170914-add-status-for-questions.js (1)
25-31: Improved migration rollback with proper enum cleanupThe
downmethod has been enhanced to properly drop both thestatuscolumn and its associated enum type, ensuring clean rollbacks. This approach follows best practices for PostgreSQL enum management in migrations.Servers/routes/eu.route.ts (4)
1-8: Well-structured router setup with proper dependenciesThe router is correctly set up with the necessary imports, middleware, and controller functions. The use of multer for file uploads is appropriate for handling form data with file attachments.
16-19: Clear route naming for project-related endpointsThe routes for retrieving assessments and compliances by project ID are well-named and follow RESTful conventions. The comments suggesting previous route paths provide helpful context.
20-24: Well-structured progress tracking endpointsThe progress tracking endpoints are logically organized for both individual projects and aggregated data across all projects. This organization will make the API intuitive to use.
39-48: Well-implemented update and delete operationsThe routes for saving controls, updating answers, and deleting assessments/compliances by project ID are properly configured with appropriate HTTP methods. The file upload middleware is correctly applied to the control update endpoint.
Servers/models/EU/assessmentEU.model.ts (1)
19-47: Model implementation looks goodThe model is well-defined with proper column types, primary key, foreign key relationship, and appropriate nullable/non-nullable flags.
Servers/models/EU/answerEU.model.ts (1)
49-52: Model implementation is more specific than type definitionThe model definition provides a detailed structure for
evidence_files, but the type definition uses the genericObject[]. Ensure both use the same specific type.The JSONB implementation with the detailed structure is good, just make sure to update the type definition as suggested earlier to match this structure.
Servers/models/EU/subControlStructEU.model.ts (1)
19-57: Model implementation looks good.The Sequelize model implementation is correct with appropriate column definitions and foreign key relationships.
Servers/models/EU/controlCategoryStructEU.model.ts (2)
11-17: The type definition looks good.The type definition correctly includes an optional array of related controls, which matches the database relationship.
19-51: Model implementation looks good.The Sequelize model implementation is correct with appropriate column definitions for all fields.
Servers/models/EU/controlStructEU.model.ts (2)
10-17: Type definition looks good.The type definition is clear and includes appropriate comments explaining each field's purpose.
19-63: Model implementation looks good.The Sequelize model implementation is correct with appropriate column definitions and foreign key relationships.
Servers/models/EU/questionStructEU.model.ts (2)
10-22: Type definition looks good.The type definition is comprehensive and includes appropriate comments explaining each field's purpose.
24-74: Model implementation looks good with the exception of the subtopic_id data type.The rest of the Sequelize model implementation is correct with appropriate column definitions for all fields.
Also applies to: 80-92
Servers/database/migrations/20250424175556-create-structure-for-compliance-tracker.js (1)
61-69: Enum types must exist before the tables reference them
enum_controls_status,enum_controls_risk_review,
enum_subcontrols_status, andenum_subcontrols_risk_revieware used
without being created in this migration. Unless they were added in an
earlier migration, Postgres will error out.Please double-check the migration history or prepend
CREATE TYPE ... AS ENUM (...)statements here.Servers/utils/eu.utils.ts (1)
308-314: Possible runtime crash when no assessment exists for a frameworkAccessing
assessmentId[0][0].idassumes at least one row; an empty set throwsTypeError: Cannot read property 'id' of undefined.Add a guard:
if (assessmentId[0].length === 0) { return []; }
| export type TopicStructEU = { | ||
| id?: number; // auto generated by database | ||
| title: string; // gets assigned from the structure | ||
| order_no?: number; // gets assigned from the structure | ||
| created_at?: Date; | ||
| }; |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Type definition missing is_demo field
The TopicStructEU type definition doesn't include the is_demo field, but it's implemented in the model class. This creates an inconsistency between the type and the model implementation.
Update the type definition to include the missing field:
export type TopicStructEU = {
id?: number; // auto generated by database
title: string; // gets assigned from the structure
order_no?: number; // gets assigned from the structure
+ is_demo?: boolean; // flag to indicate if this is a demo topic
created_at?: Date;
};Also applies to: 37-42
Servers/utils/fileUpload.utils.ts
Outdated
| const project = await sequelize.query( | ||
| "SELECT project_id FROM projects_frameworks WHERE id = :id", | ||
| { replacements: { id: project_framework_id } }, | ||
| ) as [{ project_id: number }[], number] |
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
Table name inconsistency
The SQL query uses "projects_frameworks" but the model in projectFrameworks.model.ts specifies "project_frameworks" (without the 's' after project).
Also applies to: 33-33
🏁 Script executed:
#!/bin/bash
# Check if the table name is consistent across the codebase
echo "Checking table name in database migrations..."
rg -A 5 -B 5 "CREATE TABLE.*projects?_frameworks" --glob="*.js"
echo "Checking table name references in code..."
rg "projects?_frameworks" --glob="*.ts" | grep -v "projectFrameworks.model.ts"Length of output: 6441
🏁 Script executed:
#!/bin/bash
echo "Locating projectFrameworks.model.ts..."
fd -t f projectFrameworks.model.ts
echo "Displaying first 200 lines of the model file(s):"
for file in $(fd -t f projectFrameworks.model.ts); do
echo "---- $file ----"
sed -n '1,200p' "$file"
doneLength of output: 1285
Align Sequelize model table name with migration and raw SQL queries
The migration creates the table as projects_frameworks and all raw SQL references use that name, but the model is annotated with project_frameworks. Update the model to match:
• File: Servers/models/projectFrameworks.model.ts
@Table({
- tableName: "project_frameworks"
+ tableName: "projects_frameworks"
})
export class ProjectFrameworksModel extends Model<ProjectFrameworks> {
…
}This change ensures the ORM model matches the actual database table and existing queries.
Servers/utils/fileUpload.utils.ts
Outdated
| project_framework_id: number, | ||
| source: "Assessment tracker group" | "Compliance tracker group" | ||
| ) => { | ||
| const project = await sequelize.query( | ||
| "SELECT project_id FROM projects_frameworks WHERE id = :id", | ||
| { replacements: { id: project_framework_id } }, | ||
| ) as [{ project_id: number }[], number] |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Add error handling for missing project_framework_id
The function retrieves the project_id from project_framework_id, but there's no error handling if the project_framework_id doesn't exist or doesn't return a result.
Add error handling to prevent potential runtime errors:
const project = await sequelize.query(
"SELECT project_id FROM projects_frameworks WHERE id = :id",
{ replacements: { id: project_framework_id } },
) as [{ project_id: number }[], number]
+
+ if (!project[0] || project[0].length === 0) {
+ throw new Error(`No project found for project_framework_id: ${project_framework_id}`);
+ }📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| project_framework_id: number, | |
| source: "Assessment tracker group" | "Compliance tracker group" | |
| ) => { | |
| const project = await sequelize.query( | |
| "SELECT project_id FROM projects_frameworks WHERE id = :id", | |
| { replacements: { id: project_framework_id } }, | |
| ) as [{ project_id: number }[], number] | |
| const project = await sequelize.query( | |
| "SELECT project_id FROM projects_frameworks WHERE id = :id", | |
| { replacements: { id: project_framework_id } }, | |
| ) as [{ project_id: number }[], number] | |
| if (!project[0] || project[0].length === 0) { | |
| throw new Error(`No project found for project_framework_id: ${project_framework_id}`); | |
| } |
| export type SubtopicStructEU = { | ||
| id?: number; // auto generated by database | ||
| title: string; // gets assigned from the structure | ||
| order_no?: number; // gets assigned from the structure | ||
| topic_id: number; // when topic is created, its id will be stored and assign here as FK | ||
| created_at?: Date; | ||
| }; |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Type definition missing is_demo field
The SubtopicStructEU type definition doesn't include the is_demo field, but it's implemented in the model class. This creates an inconsistency between the type and the model implementation.
Update the type definition to include the missing field:
export type SubtopicStructEU = {
id?: number; // auto generated by database
title: string; // gets assigned from the structure
order_no?: number; // gets assigned from the structure
topic_id: number; // when topic is created, its id will be stored and assign here as FK
+ is_demo?: boolean; // flag to indicate if this is a demo subtopic
created_at?: Date;
};Also applies to: 45-50
| export type ProjectFrameworks = { | ||
| framework_id: number; | ||
| project_id: number; | ||
| }; |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Type definition missing is_demo field
The ProjectFrameworks type definition doesn't include the is_demo field, but it's implemented in the model class. This creates an inconsistency between the type and the model implementation.
Update the type definition to include the missing field:
export type ProjectFrameworks = {
framework_id: number;
project_id: number;
+ is_demo?: boolean;
};Also applies to: 28-33
Servers/controllers/eu.ctrl.ts
Outdated
| const response = { | ||
| ...{ control, subControls: subControlResp }, | ||
| }; | ||
| // Update the project's last updated date | ||
| await updateProjectUpdatedByIdQuery(controlId, "controls"); | ||
|
|
||
| return res.status(200).json(STATUS_CODE[200]({ response })); | ||
| } catch (error) { |
There was a problem hiding this comment.
🛠️ Refactor suggestion
updateProjectUpdatedByIdQuery is invoked with a control id, not a project id
updateProjectUpdatedByIdQuery(controlId, "controls") most likely expects the owning project (or project_framework) id to bump the project’s updated_at timestamp. Passing a control id results in the wrong row being patched or – if FK constraints exist – in a runtime error.
👍 Pass Control.project_framework_id (or the parent project id) instead.
| let totalNumberOfQuestions = 0; | ||
| let totalNumberOfAnsweredQuestions = 0; | ||
| try { | ||
| const projects = await getAllProjectsQuery(); | ||
| if (projects && projects.length > 0) { | ||
| await Promise.all( | ||
| projects.map(async (project) => { | ||
| // // calculating assessments | ||
| const { totalAssessments, answeredAssessments } = await countAnswersEUByProjectId(project.id!); | ||
| totalNumberOfQuestions = parseInt(totalAssessments); | ||
| totalNumberOfAnsweredQuestions = parseInt(answeredAssessments); | ||
| }) | ||
| ); | ||
| return res.status(200).json( |
There was a problem hiding this comment.
Incorrect aggregation and wrong identifier supplied in project-wide assessment progress calculation
countAnswersEUByProjectIdexpects a projectFrameworkId, butproject.id(the project’s PK) is passed – this returns incorrect counts or SQL errors.totalNumberOfQuestions/totalNumberOfAnsweredQuestionsare over-written on every iteration (=) instead of accumulated (+=).
- const { totalAssessments, answeredAssessments } = await countAnswersEUByProjectId(project.id!);
- totalNumberOfQuestions = parseInt(totalAssessments);
- totalNumberOfAnsweredQuestions = parseInt(answeredAssessments);
+ // pick the EU framework for the project (id = 1)
+ const projectFrameworkId = (project as ProjectModel & { frameworks: any[] })
+ .frameworks.find(f => f.framework_id === 1)?.id;
+ if (!projectFrameworkId) return;
+
+ const { totalAssessments, answeredAssessments } =
+ await countAnswersEUByProjectId(projectFrameworkId);
+
+ totalNumberOfQuestions += parseInt(totalAssessments);
+ totalNumberOfAnsweredQuestions += parseInt(answeredAssessments);Failing to fix this will surface wrong global progress figures in the dashboard.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| let totalNumberOfQuestions = 0; | |
| let totalNumberOfAnsweredQuestions = 0; | |
| try { | |
| const projects = await getAllProjectsQuery(); | |
| if (projects && projects.length > 0) { | |
| await Promise.all( | |
| projects.map(async (project) => { | |
| // // calculating assessments | |
| const { totalAssessments, answeredAssessments } = await countAnswersEUByProjectId(project.id!); | |
| totalNumberOfQuestions = parseInt(totalAssessments); | |
| totalNumberOfAnsweredQuestions = parseInt(answeredAssessments); | |
| }) | |
| ); | |
| return res.status(200).json( | |
| let totalNumberOfQuestions = 0; | |
| let totalNumberOfAnsweredQuestions = 0; | |
| try { | |
| const projects = await getAllProjectsQuery(); | |
| if (projects && projects.length > 0) { | |
| await Promise.all( | |
| projects.map(async (project) => { | |
| // // calculating assessments | |
| // pick the EU framework for the project (id = 1) | |
| const projectFrameworkId = (project as ProjectModel & { frameworks: any[] }) | |
| .frameworks.find(f => f.framework_id === 1)?.id; | |
| if (!projectFrameworkId) return; | |
| const { totalAssessments, answeredAssessments } = | |
| await countAnswersEUByProjectId(projectFrameworkId); | |
| totalNumberOfQuestions += parseInt(totalAssessments); | |
| totalNumberOfAnsweredQuestions += parseInt(answeredAssessments); | |
| }) | |
| ); | |
| return res.status(200).json( |
Servers/controllers/eu.ctrl.ts
Outdated
| // Update the project's last updated date | ||
| await updateProjectUpdatedByIdQuery(questionId, "questions"); | ||
|
|
||
| return res.status(202).json(STATUS_CODE[202](question)); | ||
| } catch (error) { |
There was a problem hiding this comment.
Same identifier mix-up for question updates
Here questionId (answer row PK) is forwarded to updateProjectUpdatedByIdQuery, causing incorrect bookkeeping of “last updated” timestamps.
- await updateProjectUpdatedByIdQuery(questionId, "questions");
+ await updateProjectUpdatedByIdQuery(body.project_framework_id, "questions");📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| // Update the project's last updated date | |
| await updateProjectUpdatedByIdQuery(questionId, "questions"); | |
| return res.status(202).json(STATUS_CODE[202](question)); | |
| } catch (error) { | |
| // Update the project's last updated date | |
| await updateProjectUpdatedByIdQuery(body.project_framework_id, "questions"); | |
| return res.status(202).json(STATUS_CODE[202](question)); |
Servers/utils/eu.utils.ts
Outdated
| export const updateControlEUByIdQuery = async ( | ||
| id: number, | ||
| control: Partial<ControlEU> | ||
| ): Promise<ControlEU> => { | ||
| const updateControl: Partial<Record<keyof ControlEU, any>> = {}; | ||
| const setClause = [ | ||
| "status", | ||
| "approver", | ||
| "risk_review", | ||
| "owner", | ||
| "reviewer", | ||
| "due_date", | ||
| "implementation_details" | ||
| ].filter(f => { | ||
| if (control[f as keyof ControlEU] !== undefined && control[f as keyof ControlEU]) { | ||
| updateControl[f as keyof ControlEU] = control[f as keyof ControlEU] | ||
| return true | ||
| } | ||
| }).map(f => `${f} = :${f}`).join(", "); | ||
|
|
||
| const query = `UPDATE controls_eu SET ${setClause} WHERE id = :id RETURNING *;`; | ||
|
|
||
| updateControl.id = id; | ||
|
|
||
| const result = await sequelize.query(query, { | ||
| replacements: updateControl, | ||
| mapToModel: true, | ||
| model: ControlEUModel, | ||
| // type: QueryTypes.UPDATE, | ||
| }); | ||
| return result[0]; | ||
| }; |
There was a problem hiding this comment.
updateControlEUByIdQuery can generate invalid SQL when no updatable fields are supplied
If the caller only sends files or leaves all scalar fields empty, setClause becomes the empty string resulting in:
UPDATE controls_eu SET WHERE id = …which throws a syntax error.
- const query = `UPDATE controls_eu SET ${setClause} WHERE id = :id RETURNING *;`;
+ if (!setClause) {
+ return sequelize.query(
+ "SELECT * FROM controls_eu WHERE id = :id",
+ { replacements: { id }, mapToModel: true, model: ControlEUModel }
+ ).then(r => r[0]);
+ }
+ const query = `UPDATE controls_eu SET ${setClause} WHERE id = :id RETURNING *;`;| ].filter(f => { | ||
| if (f == 'evidence_files' && currentEvidenceFiles.length > 0) { | ||
| updateSubControl['evidence_files'] = JSON.stringify(currentEvidenceFiles); | ||
| return true; | ||
| } | ||
| if (f == 'feedback_files' && currentFeedbackFiles.length > 0) { | ||
| updateSubControl['feedback_files'] = JSON.stringify(currentFeedbackFiles); | ||
| return true; | ||
| } | ||
| if (subcontrol[f as keyof SubcontrolEU] !== undefined && subcontrol[f as keyof SubcontrolEU]) { |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Cannot remove all evidence/feedback files
evidence_files / feedback_files are only updated when the resulting array is non-empty. Therefore a user cannot delete the last remaining file – the DB retains stale links.
- if (f == 'evidence_files' && currentEvidenceFiles.length > 0) {
+ if (f === 'evidence_files') {
updateSubControl['evidence_files'] = JSON.stringify(currentEvidenceFiles);
return true;
}
- if (f == 'feedback_files' && currentFeedbackFiles.length > 0) {
+ if (f === 'feedback_files') {
updateSubControl['feedback_files'] = JSON.stringify(currentFeedbackFiles);
return true;
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| ].filter(f => { | |
| if (f == 'evidence_files' && currentEvidenceFiles.length > 0) { | |
| updateSubControl['evidence_files'] = JSON.stringify(currentEvidenceFiles); | |
| return true; | |
| } | |
| if (f == 'feedback_files' && currentFeedbackFiles.length > 0) { | |
| updateSubControl['feedback_files'] = JSON.stringify(currentFeedbackFiles); | |
| return true; | |
| } | |
| if (subcontrol[f as keyof SubcontrolEU] !== undefined && subcontrol[f as keyof SubcontrolEU]) { | |
| ].filter(f => { | |
| if (f === 'evidence_files') { | |
| updateSubControl['evidence_files'] = JSON.stringify(currentEvidenceFiles); | |
| return true; | |
| } | |
| if (f === 'feedback_files') { | |
| updateSubControl['feedback_files'] = JSON.stringify(currentFeedbackFiles); | |
| return true; | |
| } | |
| if ( | |
| subcontrol[f as keyof SubcontrolEU] !== undefined && | |
| subcontrol[f as keyof SubcontrolEU] | |
| ) { | |
| // … |
…into hp-apr-24-convert-eu-to-a-framework
…com/bluewave-labs/verifywise into hp-may-3-transactionalize-queries
…-queries Transactionalize all the DB queries
…or-eu Fix AutoDriver for EU AI Act
…into hp-apr-24-convert-eu-to-a-framework
There was a problem hiding this comment.
Actionable comments posted: 19
🔭 Outside diff range comments (7)
Servers/utils/subtopic.utils.ts (1)
55-63:⚠️ Potential issue
updateSubtopicByIdQuerycan build an emptySETclause
Whensubtopichas no updatable fields, the generated SQL becomesUPDATE subtopics SET WHERE id = :id, which is invalid. Short-circuit early or throw a 400 error.const setClause = ["title"].filter(…) .map(f => `${f} = :${f}`).join(", "); -if (!setClause) { - return null; // or throw new Error("Nothing to update") -} +if (!setClause) { + throw new Error("No valid fields supplied for update"); +}Servers/utils/vendorRisk.util.ts (3)
70-88:⚠️ Potential issueInconsistent table name casing could break the UPDATE statement.
createNewVendorRiskQueryinserts intovendorRisks, whereas the UPDATE query targetsvendorrisks.
On PostgreSQL unquoted identifiers are folded to lowercase, but the different spelling (vendorRisksvs.vendorrisks) will still point to two distinct tables if either was created quoted or in camelCase previously. Please verify which name is correct and standardise it across all queries.-const query = `UPDATE vendorrisks SET ${setClause} WHERE id = :id RETURNING *;`; +const query = `UPDATE vendorRisks SET ${setClause} WHERE id = :id RETURNING *;`;
81-85: 🛠️ Refactor suggestionGuard against empty UPDATE payloads.
If the caller supplies no updatable fields,
setClausebecomes an empty string and the SQL produced isUPDATE vendorRisks SET WHERE id = …, which is invalid.
Abort early or throw a 4xx error when no updatable keys are present.
102-117:⚠️ Potential issueUse the primitive
booleaninstead of the boxed typeBoolean.Returning
Promise<Boolean>(boxed object) is discouraged; prefer the primitivebooleanfor consistency and to avoid subtle bugs withinstanceof.-export const deleteVendorRiskByIdQuery = async ( - id: number, - transaction: Transaction -): Promise<Boolean> => { +export const deleteVendorRiskByIdQuery = async ( + id: number, + transaction: Transaction +): Promise<boolean> => {Apply the same change to
deleteVendorRisksForVendorQuery.🧰 Tools
🪛 Biome (1.9.4)
[error] 105-105: Don't use 'Boolean' as a type.
Use lowercase primitives for consistency.
Safe fix: Use 'boolean' instead(lint/complexity/noBannedTypes)
Servers/utils/vendor.utils.ts (1)
89-105: 🛠️ Refactor suggestionPotential duplicate (vendor_id, project_id) entries.
addVendorProjectsblindlyINSERTs without checking for existing rows.
If you callcreateNewVendorQuerytwice inside the same transaction or update a vendor without clearing duplicates, a unique-constraint violation may occur. Either:
- Add
ON CONFLICT DO NOTHING(Postgres), or- Keep a UNIQUE constraint and deduplicate the
projectsarray before insert.Servers/utils/controlCategory.util.ts (1)
91-99:⚠️ Potential issuePossible empty
SETclause – statement can break at runtimeWhen none of the whitelisted fields (
titlecurrently) are present incontrolCategory,setClausebecomes an empty string, producingUPDATE controlcategories SET WHERE id = …;which is invalid SQL and will throw.
Guard against an empty clause (or expand the whitelist) before executing.const setClause = /* …existing code… */.join(", "); + if (!setClause) { + // Nothing to update – avoid executing malformed SQL + return null; // or throw an explicit “nothing-to-update” error + }Servers/utils/subControl.utils.ts (1)
145-158: 🛠️ Refactor suggestionLeft-over
console.logand impossible “delete all files” scenario
console.log(currentEvidenceFiles, evidenceUploadedFiles);
should be removed – noisy in production.- You only set
evidence_files/feedback_fileswhen the resulting
list is non-empty. Users can never delete the last file because the
column is left unchanged.See earlier suggestion on EU utils – apply the same fix here:
- if (f == 'evidence_files' && currentEvidenceFiles.length > 0) { + if (f === 'evidence_files') { updateSubControl['evidence_files'] = JSON.stringify(currentEvidenceFiles); return true; } - if (f == 'feedback_files' && currentFeedbackFiles.length > 0) { + if (f === 'feedback_files') { updateSubControl['feedback_files'] = JSON.stringify(currentFeedbackFiles); return true; }
♻️ Duplicate comments (7)
Servers/models/EU/topicStructEU.model.ts (1)
10-15: Type definition missing is_demo fieldThe
TopicStructEUtype definition doesn't include theis_demofield, but it's implemented in the model class. This creates an inconsistency between the type and the model implementation.Update the type definition to include the missing field:
export type TopicStructEU = { id?: number; // auto generated by database title: string; // gets assigned from the structure order_no?: number; // gets assigned from the structure framework_id?: number; // gets assigned from the structure + is_demo?: boolean; // flag to indicate if this is a demo topic };Also applies to: 44-49
Servers/models/EU/controlCategoryStructEU.model.ts (1)
3-3: Remove unused ProjectModel import.The
ProjectModelis imported but not used anywhere in this file. Please remove this unused import.- import { ProjectModel } from "../project.model";Servers/models/EU/assessmentEU.model.ts (1)
14-19: Update type definition to include is_demo fieldThe
is_demofield exists in the model definition but is missing from theAssessmentEUtype definition. For better type safety and consistency, include it in the type definition.export type AssessmentEU = { id?: number; project_id: number; projects_frameworks_id?: number; created_at?: Date; + is_demo?: boolean; };Servers/controllers/project.ctrl.ts (2)
101-105: Hard-coded[1]defeats multi-framework support
Same issue as flagged in the previous review – the passed framework list
is still hard-coded to EU id1, ignoring theframeworkvalue from the
request body.
140-144: Hard-coded framework in update flow
const framework = [1]again ignores the client-supplied array,
breaking the very feature introduced by this PR.🧰 Tools
🪛 Biome (1.9.4)
[error] 144-144: Avoid the delete operator which can impact performance.
Unsafe fix: Use an undefined assignment instead.
(lint/performance/noDelete)
Servers/utils/project.utils.ts (1)
291-333: EU framework create/delete executed unconditionally in loopWhen multiple frameworks map to the same handler (e.g. future v2 of EU)
createEUFrameworkQuery/deleteProjectFrameworkEUQueryare executed
for every occurrence – expensive duplicate work.Move the handler outside the loop or guard with
if (!handled.has(framework)) ….Servers/utils/eu.utils.ts (1)
621-626: Cannot remove the last evidence/feedback fileSame logic issue previously reported: the column is only updated when the
new array is non-empty. Users cannot delete the final file.Apply the unconditional update pattern suggested earlier.
🧹 Nitpick comments (24)
Servers/utils/role.utils.ts (1)
76-76: Use lowercase 'boolean' instead of 'Boolean' for return type.For consistency with TypeScript standards, use the primitive type 'boolean' rather than the object type 'Boolean'.
- export const deleteRoleByIdQuery = async (id: number, transaction: Transaction): Promise<Boolean> => { + export const deleteRoleByIdQuery = async (id: number, transaction: Transaction): Promise<boolean> => {Also applies to: 84-84
🧰 Tools
🪛 Biome (1.9.4)
[error] 76-76: Don't use 'Boolean' as a type.
Use lowercase primitives for consistency.
Safe fix: Use 'boolean' instead(lint/complexity/noBannedTypes)
Servers/database/migrations/20250502184525-create-table-for-frameworks.js (2)
1-1: Remove redundant 'use strict' directive.The 'use strict' directive is unnecessary in JavaScript modules as they are already in strict mode by default.
- 'use strict';🧰 Tools
🪛 Biome (1.9.4)
[error] 1-1: Redundant use strict directive.
The entire contents of JavaScript modules are automatically in strict mode, with no statement needed to initiate it.
Safe fix: Remove the redundant use strict directive.(lint/suspicious/noRedundantUseStrict)
43-43: Fix typo in variable name.There's a typo in the variable name 'prjectsFrameworksInsert'.
- const prjectsFrameworksInsert = projectsQuery[0].map((project) => { + const projectsFrameworksInsert = projectsQuery[0].map((project) => {And update the usage on line 47:
- `INSERT INTO projects_frameworks (project_id, framework_id) VALUES ${prjectsFrameworksInsert};` + `INSERT INTO projects_frameworks (project_id, framework_id) VALUES ${projectsFrameworksInsert};`Servers/utils/assessment.utils.ts (1)
67-67: Use lowercasebooleaninstead ofBooleanFor consistency, use lowercase primitive types instead of constructor types.
-): Promise<Boolean> => { +): Promise<boolean> => {Also applies to: 86-86
🧰 Tools
🪛 Biome (1.9.4)
[error] 67-67: Don't use 'Boolean' as a type.
Use lowercase primitives for consistency.
Safe fix: Use 'boolean' instead(lint/complexity/noBannedTypes)
Servers/utils/projectRisk.utils.ts (1)
145-145: Use lowercasebooleaninstead ofBooleanFor consistency, use lowercase primitive types instead of constructor types.
-): Promise<Boolean> => { +): Promise<boolean> => {🧰 Tools
🪛 Biome (1.9.4)
[error] 145-145: Don't use 'Boolean' as a type.
Use lowercase primitives for consistency.
Safe fix: Use 'boolean' instead(lint/complexity/noBannedTypes)
Servers/models/EU/assessmentEU.model.ts (1)
5-13: Fix typo in documentation commentThere's a typo in the comment where "nothing specific has changedn" should be "nothing specific has changed".
-In fact nothing specific has changedn but we're only +In fact nothing specific has changed but we're onlyServers/utils/control.utils.ts (1)
145-145: Use lowercasebooleaninstead ofBooleanFor consistency, use lowercase primitive types instead of constructor types.
-): Promise<Boolean> => { +): Promise<boolean> => {🧰 Tools
🪛 Biome (1.9.4)
[error] 145-145: Don't use 'Boolean' as a type.
Use lowercase primitives for consistency.
Safe fix: Use 'boolean' instead(lint/complexity/noBannedTypes)
Servers/utils/subtopic.utils.ts (2)
81-91: DELETE query returnsresult.lengthof metadata, not affected rows
sequelize.query()withQueryTypes.DELETEreturns[affectedRows](numeric) in many dialects, not an array of records. Relying onresult.length > 0may be unreliable. Prefer checkingresult > 0.🧰 Tools
🪛 Biome (1.9.4)
[error] 82-82: Don't use 'Boolean' as a type.
Use lowercase primitives for consistency.
Safe fix: Use 'boolean' instead(lint/complexity/noBannedTypes)
130-146: Minor: batch insert would remove N+1 round-trips
createNewSubTopicsQueryiterates and issues oneINSERTper subtopic, then N more for questions. Consider using bulk inserts (INSERT … VALUES (…) , (…) RETURNING *) where supported to reduce latency.Servers/controllers/role.ctrl.ts (1)
69-88: DRY: wrap transactional controller logic in a helper
Each CUD handler now follows the same try/commit/rollback structure. Extracting awithTransaction(handler)higher-order function or middleware will eliminate duplication and guard against future omissions.Also applies to: 95-110
Servers/utils/topic.utils.ts (2)
76-90: Primitivebooleanpreferred overBooleanin Promise result.
deleteTopicByIdQuerycurrently returnsPromise<Boolean>. Switch to the primitive form for type-safety and consistency with TS guidelines.🧰 Tools
🪛 Biome (1.9.4)
[error] 78-78: Don't use 'Boolean' as a type.
Use lowercase primitives for consistency.
Safe fix: Use 'boolean' instead(lint/complexity/noBannedTypes)
106-134: Topic-creation loop executes serially – consider batching for speed.
for (let topicStruct of Topics)waits for eachINSERT+ subtopic cascade before starting the next.
On projects whereTopics.lengthis large this adds noticeable latency. Refactor to launch the topic INSERTs in parallel under the same transaction, e.g.:-const createdTopics = []; -for (const topicStruct of Topics) { - const topic = await insertTopic(topicStruct); - const sub = await createNewSubTopicsQuery(...); - createdTopics.push({ ...topic, subTopics: sub }); -} -return createdTopics; +return Promise.all( + Topics.map(async (t) => { + const topic = await insertTopic(t); + const sub = await createNewSubTopicsQuery( + topic.id!, + t.subtopics, + enable_ai_data_insertion, + transaction, + ); + return { ...topic, subTopics: sub }; + }), +);This keeps atomicity (still one transaction) while utilising parallel network round-trips.
Servers/models/EU/controlEU.model.ts (1)
42-58: Consider NOT NULL + default value for status fields.A newly-created control currently has
status/risk_reviewnullable. If the business logic expects them always to be in"Waiting"or"Acceptable risk", enforce this at schema level:@Column({ type: DataType.ENUM("Waiting", "In progress", "Done"), allowNull: false, defaultValue: "Waiting", })This prevents unexpected
NULLstates in analytics.Servers/utils/vendor.utils.ts (1)
230-254: Replace boxedBooleanwith primitivebooleanin return type.Follow TypeScript best practices to prevent boxing overhead and equality pitfalls.
-export const deleteVendorByIdQuery = async (id: number, transaction: Transaction): Promise<Boolean> => { +export const deleteVendorByIdQuery = async (id: number, transaction: Transaction): Promise<boolean> => {Apply the same change to any other Boolean return types in this file.
🧰 Tools
🪛 Biome (1.9.4)
[error] 230-230: Don't use 'Boolean' as a type.
Use lowercase primitives for consistency.
Safe fix: Use 'boolean' instead(lint/complexity/noBannedTypes)
Servers/utils/controlCategory.util.ts (2)
114-118: Prefer primitivebooleaninstead of object wrapperBooleanTypeScript discourages the boxed type. Update the return type for consistency with the rest of the codebase.
-export const deleteControlCategoryByIdQuery = async ( - id: number, - transaction: Transaction -): Promise<Boolean> => { +export const deleteControlCategoryByIdQuery = async ( + id: number, + transaction: Transaction +): Promise<boolean> => {🧰 Tools
🪛 Biome (1.9.4)
[error] 117-117: Don't use 'Boolean' as a type.
Use lowercase primitives for consistency.
Safe fix: Use 'boolean' instead(lint/complexity/noBannedTypes)
131-160: Serial inserts inside a transaction may become a bottleneck
for … of+awaitexecutes each INSERT sequentially.
For large frameworks this can noticeably slow down migrations.
Because all queries already run in a single transaction, you can safely parallelise at the logical level:- for (let controlCategoryStruct of ControlCategories) { - const result = await sequelize.query(query, { …, transaction }); - /* … */ - } + await Promise.all( + ControlCategories.map(async (struct) => { + const result = await sequelize.query(query, { + replacements: { project_id: projectId, title: struct.title, order_no: struct.order_no }, + mapToModel: true, + model: ControlCategoryModel, + transaction, + }); + const control_category_id = result[0].id!; + const controls = await createNewControlsQuery( + control_category_id, + struct.controls, + enable_ai_data_insertion, + transaction, + ); + createdControlCategories.push({ ...result[0].dataValues, controls }); + }), + );Not mandatory but will noticeably speed things up on large datasets.
Servers/driver/autoDriver.driver.ts (1)
127-135:impact_description: "Alice"looks like placeholder textThe field should describe the potential impact;
"Alice"is probably accidental copy-paste. Replace with a meaningful description to keep demo data realistic.Servers/utils/user.utils.ts (2)
254-256: Risk of generating invalid SQL when nothing is updatedIf
usercontains no updatable properties the generatedsetClauseis"", producingUPDATE users SET WHERE id = …, a syntax error that will surface only at runtime.
Guard against this by returning early or throwing whensetClauseis empty.if (setClause.length === 0) { - const query = `UPDATE users SET ${setClause} WHERE id = :id RETURNING *;`; - ... + // Nothing to update – return current row unmodified + return getUserByIdQuery(id); }
271-316: PreferbooleanoverBoolean& missingawaitcould cause partial rollbacks
- Typescript convention is to use the lowercase primitive (
boolean).- The outer
for…ofloop awaits each innerPromise.allbut not the whole loop; in case of a rejection midway the later FK updates won’t execute while earlier ones stay committed in the same transaction. Wrapping the entire loop in a singlePromise.all(or keeping it sequential) avoids this asymmetry.-export const deleteUserByIdQuery = async (id: number, transaction: Transaction): Promise<Boolean> => { +export const deleteUserByIdQuery = async (id: number, transaction: Transaction): Promise<boolean> => {🧰 Tools
🪛 Biome (1.9.4)
[error] 271-271: Don't use 'Boolean' as a type.
Use lowercase primitives for consistency.
Safe fix: Use 'boolean' instead(lint/complexity/noBannedTypes)
Servers/utils/question.utils.ts (3)
98-141:addFileToQuestion– JSONB handling & type coercion could be cleaner
Casting the array with
JSON.stringifyand injecting it via a string literal risks double-quoting and fails for large payloads. Postgres accepts raw JSON if passed through replacements.
parseInt(f.id)presumesidwill always be a string – favourNumber(f.id)or ensure it’s already a number when stored.- evidence_files = evidence_files.filter(f => !deletedFiles.includes(parseInt(f.id))) + evidence_files = evidence_files.filter( + f => !deletedFiles.includes(Number(f.id)) +) `UPDATE questions SET evidence_files = :evidence_files WHERE id = :id RETURNING *;`, { - replacements: { evidence_files: JSON.stringify(evidenceFiles), id }, + replacements: { evidence_files, id },
229-276: Sequential inserts will crawl with large datasets – use bulk insert
createNewQuestionsQueryinserts questions in afor…ofloop, issuing N round-trips.INSERT … VALUES … , … , … RETURNING *orqueryInterface.bulkInsertcan create all rows in one go and cut latency drastically.Example:
- let createdQuestions: Question[] = [] - for (const q of questions) { - const rows = await sequelize.query(query, { ... }); - createdQuestions.push(...rows); - } - return createdQuestions; +const bulkValues = questions.map(q => ({ + subtopic_id: subTopicId, + question: q.question, + answer_type: q.answer_type, + evidence_required: q.evidence_required, + hint: q.hint, + is_required: q.isrequired, + priority_level: q.priority_level, + answer: enable_ai_data_insertion ? q.answer : null, + order_no: q.order_no ?? null, + input_type: q.input_type, +})); +return (await QuestionModel.bulkCreate(bulkValues, { transaction, returning: true }));
178-195: Return type should beboolean, notBooleanprimitive wrapperSame style concern as in
user.utils.ts.-export const deleteQuestionByIdQuery = async (...): Promise<Boolean> => { +export const deleteQuestionByIdQuery = async (...): Promise<boolean> => {🧰 Tools
🪛 Biome (1.9.4)
[error] 180-180: Don't use 'Boolean' as a type.
Use lowercase primitives for consistency.
Safe fix: Use 'boolean' instead(lint/complexity/noBannedTypes)
Servers/database/migrations/20250502184712-create-structure-for-compliance-tracker.js (1)
1-2: Redundant'use strict'in ES modulesMigrations executed through sequelize-cli already run in strict mode. The directive adds noise and was flagged by Biome.
🧰 Tools
🪛 Biome (1.9.4)
[error] 1-1: Redundant use strict directive.
The entire contents of JavaScript modules are automatically in strict mode, with no statement needed to initiate it.
Safe fix: Remove the redundant use strict directive.(lint/suspicious/noRedundantUseStrict)
Servers/utils/eu.utils.ts (1)
18-35: CapitalisedString/Object& looseObjecttypesUsing
String/Object(capitalised) and the bareObjecttype is
discouraged – it widens the type and trips Biome lint. Replace with
lower-case primitives or explicit shapes, e.g.:-const getDemoAnswers = (): String[] => { +const getDemoAnswers = (): string[] => {and define proper interfaces for
demoControls.🧰 Tools
🪛 Biome (1.9.4)
[error] 18-18: Don't use 'String' as a type.
Use lowercase primitives for consistency.
Safe fix: Use 'string' instead(lint/complexity/noBannedTypes)
[error] 30-30: Don't use 'Object' as a type.
Prefer explicitly define the object shape. This type means "any non-nullable value", which is slightly better than 'unknown', but it's still a broad type.
(lint/complexity/noBannedTypes)
[error] 31-31: Don't use 'Object' as a type.
Prefer explicitly define the object shape. This type means "any non-nullable value", which is slightly better than 'unknown', but it's still a broad type.
(lint/complexity/noBannedTypes)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (47)
Servers/controllers/assessment.ctrl.ts(6 hunks)Servers/controllers/autoDriver.ctrl.ts(2 hunks)Servers/controllers/control.ctrl.ts(10 hunks)Servers/controllers/controlCategory.ctrl.ts(4 hunks)Servers/controllers/eu.ctrl.ts(1 hunks)Servers/controllers/file.ctrl.ts(3 hunks)Servers/controllers/project.ctrl.ts(6 hunks)Servers/controllers/projectRisks.ctrl.ts(4 hunks)Servers/controllers/projectScope.ctrl.ts(4 hunks)Servers/controllers/question.ctrl.ts(5 hunks)Servers/controllers/reporting.ctrl.ts(1 hunks)Servers/controllers/role.ctrl.ts(4 hunks)Servers/controllers/subcontrol.ctrl.ts(6 hunks)Servers/controllers/subtopic.ctrl.ts(4 hunks)Servers/controllers/topic.ctrl.ts(4 hunks)Servers/controllers/user.ctrl.ts(8 hunks)Servers/controllers/vendor.ctrl.ts(6 hunks)Servers/controllers/vendorRisk.ctrl.ts(4 hunks)Servers/database/migrations/20250502184525-create-table-for-frameworks.js(1 hunks)Servers/database/migrations/20250502184633-create-structure-for-assessment-tracker.js(1 hunks)Servers/database/migrations/20250502184712-create-structure-for-compliance-tracker.js(1 hunks)Servers/driver/autoDriver.driver.ts(1 hunks)Servers/index.ts(2 hunks)Servers/models/EU/assessmentEU.model.ts(1 hunks)Servers/models/EU/controlCategoryStructEU.model.ts(1 hunks)Servers/models/EU/controlEU.model.ts(1 hunks)Servers/models/EU/questionStructEU.model.ts(1 hunks)Servers/models/EU/subTopicStructEU.model.ts(1 hunks)Servers/models/EU/topicStructEU.model.ts(1 hunks)Servers/models/project.model.ts(1 hunks)Servers/utils/assessment.utils.ts(3 hunks)Servers/utils/autoDriver.util.ts(1 hunks)Servers/utils/control.utils.ts(6 hunks)Servers/utils/controlCategory.util.ts(5 hunks)Servers/utils/eu.utils.ts(1 hunks)Servers/utils/fileUpload.utils.ts(3 hunks)Servers/utils/project.utils.ts(15 hunks)Servers/utils/projectRisk.utils.ts(5 hunks)Servers/utils/projectScope.utils.ts(5 hunks)Servers/utils/question.utils.ts(10 hunks)Servers/utils/role.utils.ts(4 hunks)Servers/utils/subControl.utils.ts(9 hunks)Servers/utils/subtopic.utils.ts(7 hunks)Servers/utils/topic.utils.ts(7 hunks)Servers/utils/user.utils.ts(10 hunks)Servers/utils/vendor.utils.ts(2 hunks)Servers/utils/vendorRisk.util.ts(5 hunks)
✅ Files skipped from review due to trivial changes (1)
- Servers/index.ts
🚧 Files skipped from review as they are similar to previous changes (5)
- Servers/models/EU/subTopicStructEU.model.ts
- Servers/controllers/file.ctrl.ts
- Servers/models/EU/questionStructEU.model.ts
- Servers/utils/fileUpload.utils.ts
- Servers/controllers/eu.ctrl.ts
🧰 Additional context used
🧬 Code Graph Analysis (17)
Servers/controllers/autoDriver.ctrl.ts (1)
Servers/driver/autoDriver.driver.ts (2)
insertMockData(17-152)deleteMockData(154-168)
Servers/controllers/reporting.ctrl.ts (1)
Servers/database/db.ts (1)
sequelize(82-82)
Servers/controllers/topic.ctrl.ts (2)
Servers/database/db.ts (1)
sequelize(82-82)Servers/utils/topic.utils.ts (3)
createNewTopicQuery(30-44)updateTopicByIdQuery(46-74)deleteTopicByIdQuery(76-90)
Servers/controllers/control.ctrl.ts (4)
Servers/database/db.ts (1)
sequelize(82-82)Servers/utils/control.utils.ts (3)
createNewControlQuery(68-103)updateControlByIdQuery(105-140)deleteControlByIdQuery(142-157)Servers/utils/fileUpload.utils.ts (1)
deleteFileById(51-61)Servers/utils/project.utils.ts (1)
updateProjectUpdatedByIdQuery(205-243)
Servers/controllers/user.ctrl.ts (2)
Servers/database/db.ts (1)
sequelize(82-82)Servers/utils/user.utils.ts (3)
resetPasswordQuery(187-206)getUserByIdQuery(114-124)deleteUserByIdQuery(271-316)
Servers/controllers/subcontrol.ctrl.ts (2)
Servers/database/db.ts (1)
sequelize(82-82)Servers/utils/subControl.utils.ts (2)
updateSubcontrolByIdQuery(127-205)deleteSubcontrolByIdQuery(207-222)
Servers/controllers/projectScope.ctrl.ts (2)
Servers/database/db.ts (1)
sequelize(82-82)Servers/utils/projectScope.utils.ts (3)
createProjectScopeQuery(30-60)updateProjectScopeByIdQuery(62-98)deleteProjectScopeByIdQuery(100-115)
Servers/utils/projectRisk.utils.ts (2)
Servers/models/projectRisk.model.ts (1)
ProjectRisk(7-80)Servers/utils/project.utils.ts (1)
updateProjectUpdatedByIdQuery(205-243)
Servers/controllers/controlCategory.ctrl.ts (2)
Servers/database/db.ts (1)
sequelize(82-82)Servers/utils/controlCategory.util.ts (3)
createControlCategoryQuery(63-84)updateControlCategoryByIdQuery(86-112)deleteControlCategoryByIdQuery(114-129)
Servers/utils/role.utils.ts (4)
Servers/models/role.model.ts (1)
Role(3-8)Servers/database/migrations/20250327205650-add-created_at-field.js (2)
transaction(6-6)transaction(43-43)Servers/database/migrations/20250325162605-convert-files-to-jsonb.js (2)
transaction(6-6)transaction(41-41)Servers/database/db.ts (1)
sequelize(82-82)
Servers/utils/projectScope.utils.ts (3)
Servers/models/projectScope.model.ts (1)
ProjectScope(3-15)Servers/database/migrations/20250327205650-add-created_at-field.js (2)
transaction(6-6)transaction(43-43)Servers/database/migrations/20250325162605-convert-files-to-jsonb.js (2)
transaction(6-6)transaction(41-41)
Servers/models/EU/controlEU.model.ts (3)
Servers/models/EU/subControlEU.model.ts (2)
SubcontrolEU(11-27)Table(29-116)Servers/models/EU/controlCategoryStructEU.model.ts (1)
Table(20-54)Servers/models/projectFrameworks.model.ts (1)
Table(10-34)
Servers/controllers/role.ctrl.ts (2)
Servers/database/db.ts (1)
sequelize(82-82)Servers/utils/role.utils.ts (3)
createNewRoleQuery(28-43)updateRoleByIdQuery(45-74)deleteRoleByIdQuery(76-88)
Servers/utils/control.utils.ts (2)
Servers/models/control.model.ts (1)
Control(11-28)Servers/utils/subControl.utils.ts (1)
createNewSubControlsQuery(224-268)
Servers/utils/controlCategory.util.ts (2)
Servers/models/controlCategory.model.ts (1)
ControlCategory(11-18)Servers/utils/control.utils.ts (1)
createNewControlsQuery(159-211)
Servers/utils/eu.utils.ts (6)
Servers/mocks/controlCategory.mock.data.ts (1)
ControlCategories(3-124)Servers/database/db.ts (1)
sequelize(82-82)Servers/models/EU/controlEU.model.ts (1)
ControlEU(14-29)Servers/models/EU/answerEU.model.ts (1)
AnswerEU(5-14)Servers/models/EU/assessmentEU.model.ts (1)
AssessmentEU(14-19)Servers/models/EU/subControlEU.model.ts (1)
SubcontrolEU(11-27)
Servers/controllers/project.ctrl.ts (4)
Servers/database/db.ts (1)
sequelize(82-82)Servers/models/project.model.ts (1)
Project(6-34)Servers/utils/eu.utils.ts (1)
createEUFrameworkQuery(521-538)Servers/utils/project.utils.ts (1)
deleteProjectByIdQuery(438-486)
🪛 Biome (1.9.4)
Servers/utils/assessment.utils.ts
[error] 49-49: Don't use 'Object' as a type.
Prefer explicitly define the object shape. This type means "any non-nullable value", which is slightly better than 'unknown', but it's still a broad type.
(lint/complexity/noBannedTypes)
[error] 86-86: Don't use 'Boolean' as a type.
Use lowercase primitives for consistency.
Safe fix: Use 'boolean' instead
(lint/complexity/noBannedTypes)
Servers/utils/role.utils.ts
[error] 76-76: Don't use 'Boolean' as a type.
Use lowercase primitives for consistency.
Safe fix: Use 'boolean' instead
(lint/complexity/noBannedTypes)
Servers/utils/vendorRisk.util.ts
[error] 122-122: Don't use 'Boolean' as a type.
Use lowercase primitives for consistency.
Safe fix: Use 'boolean' instead
(lint/complexity/noBannedTypes)
Servers/utils/control.utils.ts
[error] 145-145: Don't use 'Boolean' as a type.
Use lowercase primitives for consistency.
Safe fix: Use 'boolean' instead
(lint/complexity/noBannedTypes)
Servers/database/migrations/20250502184712-create-structure-for-compliance-tracker.js
[error] 1-1: Redundant use strict directive.
The entire contents of JavaScript modules are automatically in strict mode, with no statement needed to initiate it.
Safe fix: Remove the redundant use strict directive.
(lint/suspicious/noRedundantUseStrict)
Servers/utils/user.utils.ts
[error] 271-271: Don't use 'Boolean' as a type.
Use lowercase primitives for consistency.
Safe fix: Use 'boolean' instead
(lint/complexity/noBannedTypes)
Servers/utils/vendor.utils.ts
[error] 230-230: Don't use 'Boolean' as a type.
Use lowercase primitives for consistency.
Safe fix: Use 'boolean' instead
(lint/complexity/noBannedTypes)
Servers/database/migrations/20250502184525-create-table-for-frameworks.js
[error] 1-1: Redundant use strict directive.
The entire contents of JavaScript modules are automatically in strict mode, with no statement needed to initiate it.
Safe fix: Remove the redundant use strict directive.
(lint/suspicious/noRedundantUseStrict)
Servers/database/migrations/20250502184633-create-structure-for-assessment-tracker.js
[error] 1-1: Redundant use strict directive.
The entire contents of JavaScript modules are automatically in strict mode, with no statement needed to initiate it.
Safe fix: Remove the redundant use strict directive.
(lint/suspicious/noRedundantUseStrict)
Servers/utils/controlCategory.util.ts
[error] 117-117: Don't use 'Boolean' as a type.
Use lowercase primitives for consistency.
Safe fix: Use 'boolean' instead
(lint/complexity/noBannedTypes)
Servers/utils/project.utils.ts
[error] 441-441: Don't use 'Boolean' as a type.
Use lowercase primitives for consistency.
Safe fix: Use 'boolean' instead
(lint/complexity/noBannedTypes)
Servers/utils/eu.utils.ts
[error] 18-18: Don't use 'String' as a type.
Use lowercase primitives for consistency.
Safe fix: Use 'string' instead
(lint/complexity/noBannedTypes)
[error] 30-30: Don't use 'Object' as a type.
Prefer explicitly define the object shape. This type means "any non-nullable value", which is slightly better than 'unknown', but it's still a broad type.
(lint/complexity/noBannedTypes)
[error] 31-31: Don't use 'Object' as a type.
Prefer explicitly define the object shape. This type means "any non-nullable value", which is slightly better than 'unknown', but it's still a broad type.
(lint/complexity/noBannedTypes)
[error] 358-358: Don't use 'Object' as a type.
Prefer explicitly define the object shape. This type means "any non-nullable value", which is slightly better than 'unknown', but it's still a broad type.
(lint/complexity/noBannedTypes)
[error] 390-390: Don't use 'String' as a type.
Use lowercase primitives for consistency.
Safe fix: Use 'string' instead
(lint/complexity/noBannedTypes)
[error] 526-526: Don't use 'Object' as a type.
Prefer explicitly define the object shape. This type means "any non-nullable value", which is slightly better than 'unknown', but it's still a broad type.
(lint/complexity/noBannedTypes)
Servers/controllers/project.ctrl.ts
[error] 106-106: Don't use 'Object' as a type.
Prefer explicitly define the object shape. This type means "any non-nullable value", which is slightly better than 'unknown', but it's still a broad type.
(lint/complexity/noBannedTypes)
[error] 144-144: Avoid the delete operator which can impact performance.
Unsafe fix: Use an undefined assignment instead.
(lint/performance/noDelete)
[error] 145-145: Avoid the delete operator which can impact performance.
Unsafe fix: Use an undefined assignment instead.
(lint/performance/noDelete)
🔇 Additional comments (95)
Servers/controllers/autoDriver.ctrl.ts (2)
7-7: Good asynchronous handling implementation.The addition of
awaitis appropriate sinceinsertMockData()is now a transactional operation that returns a Promise. This ensures the mock data is fully inserted before sending the HTTP response.
16-16: Good asynchronous handling implementation.The addition of
awaitis appropriate sincedeleteMockData()is now a transactional operation that returns a Promise. This ensures the mock data is fully deleted before sending the HTTP response.Servers/models/project.model.ts (1)
24-27:✅ Verification successful
Good framework relationship structure.
The addition of the framework property correctly implements the many-to-many relationship between projects and frameworks, which is a core part of this PR's objectives. The typing is well-structured with explicit property types.
However, since this property is optional, make sure that code consuming this type handles the potential undefined case appropriately.
🏁 Script executed:
#!/bin/bash # Find places where Project.framework is accessed without null/undefined checks rg -A 2 -B 2 "project.*\.framework\b" --type tsLength of output: 49
To ensure we catch all direct uses of the optional
frameworkproperty (not just prefixed byproject), let’s broaden the search:
🏁 Script executed:
#!/bin/bash # Search for any .framework property access in TypeScript files and show 2 lines of context rg -n "\.framework\b" -t ts -C2Length of output: 1306
No unguarded
.frameworkaccesses foundI searched the entire codebase for any direct uses of the optional
frameworkproperty and only found commented-out examples and places where you deliberately replace or delete it (e.g. inServers/controllers/project.ctrl.ts). There are currently no unguarded accesses that could lead toundefinederrors.You can consider this concern resolved.
Servers/controllers/vendorRisk.ctrl.ts (6)
55-63: Good transaction management implementation in createVendorRisk.The transaction is properly initialized, passed to the query function, and committed upon successful creation. This ensures data consistency and follows the pattern of transaction management implemented across the application.
68-69: Good error handling with transaction rollback.The transaction rollback in the catch block ensures that no partial data is committed in case of an error, maintaining data integrity.
77-90: Good transaction management implementation in updateVendorRiskById.The transaction is properly initialized, passed to the query function, and committed upon successful update. This ensures data consistency during vendor risk updates.
95-96: Good error handling with transaction rollback.The transaction rollback in the catch block ensures that no partial data is committed in case of an error during the update, maintaining data integrity.
104-112: Good transaction management implementation in deleteVendorRiskById.The transaction is properly initialized, passed to the query function, and committed upon successful deletion. This ensures data consistency during vendor risk deletions.
117-118: Good error handling with transaction rollback.The transaction rollback in the catch block ensures that no partial data is deleted in case of an error, maintaining data integrity.
Servers/controllers/subcontrol.ctrl.ts (4)
13-13: Good addition of sequelize import for transaction support.Including the sequelize instance allows for transaction management across database operations.
55-55: Transaction management properly implemented in createNewSubcontrol.Adding transaction support ensures atomic operations when creating subcontrols, with proper commit on success and rollback on failure.
Also applies to: 81-81, 87-87
96-96: Transaction management correctly implemented in updateSubcontrolById.The transaction is properly initialized, passed to the query function, and committed or rolled back appropriately.
Also applies to: 104-104, 108-108, 114-114
123-123: Transaction management properly added to deleteSubcontrolById.Transaction handling follows the same pattern as other functions, ensuring data integrity during deletion operations.
Also applies to: 127-127, 130-130, 136-136
Servers/controllers/controlCategory.ctrl.ts (4)
14-14: Good addition of sequelize import for transaction support.Adding the sequelize import enables transaction management across database operations.
62-62: Transaction management properly implemented in createControlCategory.Transaction handling ensures atomic operations when creating control categories, with proper commit and rollback handling.
Also applies to: 66-66, 68-68, 71-71
80-80: Transaction management correctly implemented in updateControlCategoryById.The transaction is properly initialized, passed to the query function, and committed or rolled back based on the operation result.
Also applies to: 87-87, 89-89, 92-92
101-101: Transaction management properly added to deleteControlCategoryById.Transaction handling follows the consistent pattern seen in other controller functions, ensuring data integrity.
Also applies to: 105-105, 107-107, 110-110
Servers/models/EU/topicStructEU.model.ts (2)
17-19: Good model configuration with explicit table name.Setting the explicit table name ensures proper database mapping regardless of naming conventions.
38-42: Foreign key relationship properly defined.The
@ForeignKeydecorator correctly establishes the relationship with theFrameworkModel.Servers/controllers/topic.ctrl.ts (4)
18-18: Good addition of sequelize import for transaction support.Including the sequelize instance enables transaction management across database operations.
54-54: Transaction management properly implemented in createNewTopic.Transaction handling ensures atomic operations when creating topics, with proper commit and rollback handling.
Also applies to: 58-58, 61-61, 67-67
76-76: Transaction management correctly implemented in updateTopicById.The transaction is properly initialized, passed to the query function, and committed or rolled back based on the operation result.
Also applies to: 81-81, 84-84, 90-90
99-99: Transaction management properly added to deleteTopicById.Transaction handling follows the consistent pattern seen in other controller functions, ensuring data integrity.
Also applies to: 103-103, 106-106, 112-112
Servers/controllers/assessment.ctrl.ts (4)
33-33: Good addition of the sequelize import for transaction support.This import is necessary for the transaction management being implemented throughout the file.
74-75: Well-implemented transaction management in createAssessment.The transaction is properly initialized, passed to the query function, committed on success, and rolled back on failure. This ensures atomicity of the assessment creation process.
Also applies to: 85-85, 88-89, 94-95
103-104: Transaction handling correctly implemented in updateAssessmentById.The transaction management follows best practices by wrapping the database operation in a try-catch block with appropriate commit and rollback handling.
Also applies to: 117-119, 122-123, 128-129
137-138: Transaction safety added to deleteAssessmentById.The deletion operation is now protected by transaction boundaries, ensuring database consistency even if the operation fails.
Also applies to: 140-140, 143-144, 149-150
Servers/controllers/vendor.ctrl.ts (4)
13-13: Good addition of sequelize import for transaction management.This import enables transaction support throughout the file, consistent with the changes in other controllers.
62-63: Transaction management properly implemented in createVendor.The implementation follows the same pattern as other controllers, ensuring data consistency through proper transaction handling.
Also applies to: 74-74, 77-78, 83-84
92-93: Well-structured transaction handling in updateVendorById.Transaction is properly initialized, used in the query, committed on success, and rolled back on error, maintaining data integrity.
Also applies to: 105-105, 108-109, 114-115
123-124: Transaction safety successfully added to deleteVendorById.The transaction pattern is consistently applied, protecting the database from partial updates in case of failures.
Also applies to: 127-127, 130-131, 136-137
Servers/controllers/control.ctrl.ts (5)
21-21: Good addition of sequelize import for transaction management.This import enables transaction support for all the controller operations in this file.
60-61: Transaction management properly implemented in createControl.The creation operation is now encapsulated within a transaction, ensuring it either completes fully or rolls back completely.
Also applies to: 64-64, 67-68, 73-74
82-83: Transaction safety added to updateControlById.The update operation follows the consistent transaction pattern seen throughout the codebase.
Also applies to: 87-87, 90-91, 96-97
105-106: Transaction management correctly implemented in deleteControlById.The deletion operation is now protected by transaction boundaries, maintaining database consistency.
Also applies to: 109-109, 112-113, 118-119
127-128: Complex saveControls function enhanced with proper transaction management.The complex saveControls function now properly manages transactions across multiple operations including:
- Control updates
- File deletions
- File uploads for evidence and feedback
- Subcontrol updates
- Project update timestamp changes
This is particularly important for this complex function that touches multiple entities and could leave the database in an inconsistent state if partially executed.
Also applies to: 150-150, 154-155, 174-176, 194-196, 233-234, 236-237, 245-247, 251-252
Servers/controllers/projectRisks.ctrl.ts (4)
12-12: Good addition of sequelize import for transaction management.This import is consistent with the pattern across other controller files.
55-56: Transaction management properly implemented in createProjectRisk.The implementation follows the same pattern as other controllers, ensuring atomicity of risk creation.
Also applies to: 59-59, 62-63, 68-69
77-78: Transaction safety added to updateProjectRiskById.The risk update operation is now encapsulated in a transaction with proper error handling.
Also applies to: 84-86, 89-90, 95-96
104-105: Well-implemented transaction management in deleteProjectRiskById.The transaction pattern is consistently applied to the delete operation, protecting database integrity.
Also applies to: 108-108, 111-112, 117-118
Servers/controllers/projectScope.ctrl.ts (7)
11-12: Good job adding necessary imports for transaction management.The addition of the
sequelizeimport from the database module and theProjectScopemodel are essential for implementing transaction support in this controller.
54-58: Proper transaction implementation in createProjectScope.The transaction is correctly initialized and passed to the query function. Using a typed Partial for the request body also improves type safety.
61-67: Correctly handling transaction commit and rollback.The transaction is properly committed on success and rolled back in the catch block, which ensures data integrity in case of errors.
76-84: Well-implemented transaction in updateProjectScopeById.Transaction initialization and propagation to the query function are handled correctly. The parameter typing is also improved for better type safety.
88-94: Transaction commit and rollback logic is properly implemented.The transaction is committed on success and rolled back in the catch block, maintaining data consistency.
103-108: Transaction management correctly implemented in deleteProjectScopeById.The transaction is initialized and correctly passed to the delete query function.
112-118: Good handling of transaction commit and rollback.The transaction is properly committed on success and rolled back on error, ensuring atomic operations.
Servers/controllers/question.ctrl.ts (8)
17-17: Good addition of sequelize import for transaction support.The sequelize import is correctly added to support transaction management in this controller.
59-79: Well-implemented transaction in createQuestion.The transaction is properly initialized and passed to the query function. The validation logic remains intact and the transaction is only passed if all validations pass.
82-88: Transaction commit and rollback are properly handled.The transaction is committed on success and rolled back in the catch block, ensuring data consistency.
97-106: Properly implemented transaction in updateQuestionById.The transaction is correctly initialized and passed to the update query function. Type casting for the question is preserved.
108-115: Good transaction handling with multiple operations.The transaction is correctly rolled back if the question is not found. Both the question update and the project update date operations are included in the same transaction, ensuring atomicity.
119-119: Correctly handling rollback on error.The transaction is rolled back in the catch block, ensuring consistency in case of errors.
128-132: Good transaction implementation in deleteQuestionById.The transaction is initialized and correctly passed to the delete query function.
135-141: Transaction commit and rollback are properly handled.The transaction is committed on success and rolled back in the catch block, ensuring atomicity.
Servers/controllers/subtopic.ctrl.ts (7)
13-13: Good addition of sequelize import for transaction support.The sequelize import is correctly added to support transaction management in this controller.
55-57: Well-implemented transaction in createNewSubtopic.The transaction is correctly initialized and passed to the create query function with proper typing.
60-66: Transaction commit and rollback are properly handled.The transaction is committed on success and rolled back in the catch block, ensuring data consistency.
75-83: Properly implemented transaction in updateSubtopicById.The transaction is correctly initialized and passed to the update query function with proper typing for the request body.
86-92: Transaction commit and rollback are properly implemented.The transaction is committed on success and rolled back in the catch block, maintaining data consistency.
101-105: Good transaction implementation in deleteSubtopicById.The transaction is initialized and correctly passed to the delete query function.
108-114: Transaction commit and rollback are properly handled.The transaction is committed on success and rolled back in the catch block, ensuring atomicity of the delete operation.
Servers/utils/projectScope.utils.ts (7)
3-3: Good addition of sequelize imports for transaction support.The addition of
QueryTypesandTransactionimports from sequelize is necessary for implementing transaction-aware queries.
30-30: Well-defined transaction parameter in createProjectScopeQuery.The function signature is correctly updated to accept a Transaction parameter, allowing the query to participate in a transaction.
56-56: Transaction is correctly passed to the query options.The transaction object is correctly added to the query options to ensure the query participates in the transaction.
64-65: Good update to function signature for transaction support.The updated parameter list correctly includes the transaction parameter, ensuring the function can be used within a transaction context.
94-94: Transaction is correctly passed to the query options.The transaction object is properly added to the update query options.
101-102: Well-defined transaction parameter in deleteProjectScopeByIdQuery.The function signature is correctly updated to accept a Transaction parameter for transactional delete operations.
111-111: Transaction is correctly passed to the query options.The transaction object is properly added to the delete query options, ensuring the operation is part of the transaction.
Servers/controllers/user.ctrl.ts (6)
24-24: Good addition of Sequelize import for transaction support.This enables the transaction handling that has been added to user operations.
76-76: Well-implemented transaction management in createNewUser.The transaction is properly initiated, committed on success, and rolled back on error. This ensures atomicity for user creation operations.
Also applies to: 99-99, 106-106
186-187: Transaction management properly added to resetPassword function.The implementation correctly handles transaction lifecycle with commit on success and rollback on error, ensuring data consistency.
Also applies to: 195-195, 197-197, 204-204
210-211: Good implementation of transaction in updateUserById.Transaction handling is properly implemented with appropriate commit and rollback operations.
Also applies to: 224-224, 226-226, 233-233
239-240: Effective transaction handling in deleteUserById.The function now properly manages database transactions, ensuring atomicity when deleting user data and related references.
Also applies to: 245-246, 253-253
362-362: Transaction management correctly implemented in ChangePassword.The implementation ensures atomic operations when changing passwords, with proper commit and rollback handling.
Also applies to: 402-402, 404-404, 408-408
Servers/models/EU/controlCategoryStructEU.model.ts (3)
1-5: Well-structured imports for the new model.The imports are appropriate for a Sequelize model, bringing in necessary decorators and related model types.
12-18: Good type definition for ControlCategoryStructEU.The type properly defines the structure with appropriate optional and required fields, including the relationship with ControlEU and Framework models.
20-54: Well-defined Sequelize model for control categories.The model correctly:
- Uses appropriate table name
- Defines columns with proper types and constraints
- Sets up primary key and auto-increment
- Includes the is_demo flag with default value
- Establishes foreign key relationship with FrameworkModel
This model provides a solid foundation for the EU framework implementation.
Servers/utils/role.utils.ts (3)
3-3: Good addition of Transaction import from Sequelize.This supports the transaction handling that's being added to the role operations.
28-28: Transaction support added to createNewRoleQuery.The function signature is correctly updated to accept a transaction parameter, and the transaction is properly applied to the query options.
Also applies to: 39-39
45-49: Transaction parameter properly added to updateRoleByIdQuery.The function now accepts a transaction parameter and correctly applies it to the Sequelize query options.
Also applies to: 70-70
Servers/database/migrations/20250502184525-create-table-for-frameworks.js (3)
9-24: Well-designed table schema for frameworks and projects-frameworks relationship.The schema correctly:
- Creates a frameworks table with appropriate columns
- Creates a join table with proper foreign key constraints
- Sets up cascading delete relationships
- Includes an is_demo flag for demonstration data
This provides a solid foundation for the framework-based structure.
28-48: Good data migration implementation for existing projects.The code properly:
- Inserts the EU AI Act framework
- Retrieves the new framework ID
- Fetches all existing projects
- Creates appropriate associations between projects and the framework
This ensures a smooth transition to the new framework-based structure.
56-71: Proper implementation of migration rollback.The down migration correctly:
- Creates a transaction
- Drops tables in the right order (respecting foreign key constraints)
- Properly handles commits and rollbacks
This ensures migrations can be safely rolled back if needed.
Servers/utils/assessment.utils.ts (1)
45-61: Transaction handling looks goodThe addition of transaction support to the assessment creation function ensures atomicity and consistency with related operations.
🧰 Tools
🪛 Biome (1.9.4)
[error] 49-49: Don't use 'Object' as a type.
Prefer explicitly define the object shape. This type means "any non-nullable value", which is slightly better than 'unknown', but it's still a broad type.
(lint/complexity/noBannedTypes)
Servers/utils/projectRisk.utils.ts (2)
33-86: Transaction support properly implementedThe addition of transaction support to the project risk creation function and the call to
updateProjectUpdatedByIdQueryensures atomicity across both operations. The project's timestamp will be updated within the same transaction.
88-140: Improved flexibility withPartial<ProjectRisk>Good change to accept a
Partial<ProjectRisk>instead of a fullProjectRiskobject for updates, making the function more flexible.Servers/models/EU/assessmentEU.model.ts (1)
32-42: Framework schema relationship looks goodThe model correctly defines foreign key relationships to both the Project model and the new ProjectFrameworks model, which aligns with the PR objective of converting EU to a framework-based structure.
Servers/utils/control.utils.ts (2)
68-103: Transaction support properly implementedThe addition of transaction support to the control creation function ensures atomicity with other operations in the same transaction. The change to use
Partial<Control>also improves flexibility.
159-211: Good coordination of transaction through nested operationsThe implementation properly passes the transaction through to the nested
createNewSubControlsQuerycall, ensuring atomicity across the entire control creation hierarchy.Servers/utils/vendorRisk.util.ts (1)
33-60: Ensure transaction parameter remains optional at call-sites.All write helpers now require a
transaction: Transactionargument. Double-check every place in the codebase that callscreateNewVendorRiskQueryto confirm it was updated; otherwise runtime errors (undefinedpassed to Sequelize) will surface.
If you intend the parameter to be mandatory, consider marking the previous signature as deprecated and adding unit tests to catch missing transactions.Servers/models/EU/controlEU.model.ts (1)
87-92: Verify the foreign-key design forprojects_frameworks_id.
ProjectFrameworksModeluses a composite PK of(framework_id, project_id).
Referencing it via a singleprojects_frameworks_idinteger requires an additional surrogate key on that join table. Please confirm the column exists; otherwise the FK will fail.Servers/utils/vendor.utils.ts (1)
107-135:is_demoparameter not part of the domain model.
createNewVendorQueryaccepts anis_demoflag butVendortype does not expose it. Either add it toVendoror remove the argument to avoid implicitanycasts.Servers/driver/autoDriver.driver.ts (1)
49-68: Only the first project is considered – remaining demo projects are ignored
getData("projects")returns an array, but you dereference[0].
If multiple demo projects exist, the function silently skips them and the later “already exists” branch never deletes them. Consider iterating over the full array or applying a deterministic “find or create” strategy.Servers/database/migrations/20250502184633-create-structure-for-assessment-tracker.js (1)
161-166:String.prototype.replaceAllrequires Node ≥ 15Sequelize migrations often execute in Node LTS 14 environments where
replaceAllis undefined.
Fallback toreplace(/''/g, "'")or use a small helper to stay compatible.
| ): Promise<Object> => { | ||
| const result = await sequelize.query( |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Prefer specific return type instead of Object
The return type Object is too generic and should be replaced with a more specific type. Consider using { assessment: Assessment, topics: any[] } or define a dedicated interface for this return value.
-): Promise<Object> => {
+): Promise<{ assessment: Assessment, topics: any[] }> => {📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| ): Promise<Object> => { | |
| const result = await sequelize.query( | |
| ): Promise<{ assessment: Assessment; topics: any[] }> => { | |
| const result = await sequelize.query( |
🧰 Tools
🪛 Biome (1.9.4)
[error] 49-49: Don't use 'Object' as a type.
Prefer explicitly define the object shape. This type means "any non-nullable value", which is slightly better than 'unknown', but it's still a broad type.
(lint/complexity/noBannedTypes)
| export async function insertData(insertQuery: string, transaction: Transaction) { | ||
| const result = await sequelize.query(insertQuery, { transaction }); | ||
| return result | ||
| }; |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Un-sanitised insertQuery mirrors the above risk
Calling code can inject arbitrary SQL. Consider refactoring this helper so it receives structured data (table name + column/value map) and builds the INSERT statement itself, or delete the helper altogether if each caller already knows the concrete query.
| export async function getData(tableName: string, transaction: Transaction) { | ||
| const result = await sequelize.query( | ||
| `SELECT * FROM ${tableName} WHERE is_demo;`, | ||
| { transaction } | ||
| ); | ||
| return result[0]; | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Potential SQL-injection vector in getData – pass the table name safely
tableName is interpolated directly into the raw SQL string. An attacker (or even a typo) could alter the query and fetch / modify unintended tables. Raw identifiers cannot be bound as replacements, so the usual mitigation is an allow-list or a switch statement mapping enum values to fixed table names.
-export async function getData(tableName: string, transaction: Transaction) {
- const result = await sequelize.query(
- `SELECT * FROM ${tableName} WHERE is_demo;`,
+const DEMO_TABLES = new Set(["vendors", "projects", "vendorrisks" /* … */]);
+
+export async function getData(tableName: string, transaction: Transaction) {
+ if (!DEMO_TABLES.has(tableName)) {
+ throw new Error(`Table ${tableName} is not allowed in getData`);
+ }
+ const result = await sequelize.query(
+ `SELECT * FROM ${tableName} WHERE is_demo = true`, // be explicit
{ transaction }
);
return result[0];
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| export async function getData(tableName: string, transaction: Transaction) { | |
| const result = await sequelize.query( | |
| `SELECT * FROM ${tableName} WHERE is_demo;`, | |
| { transaction } | |
| ); | |
| return result[0]; | |
| } | |
| const DEMO_TABLES = new Set([ | |
| "vendors", | |
| "projects", | |
| "vendorrisks" | |
| /* … other allowed demo tables … */ | |
| ]); | |
| export async function getData(tableName: string, transaction: Transaction) { | |
| if (!DEMO_TABLES.has(tableName)) { | |
| throw new Error(`Table ${tableName} is not allowed in getData`); | |
| } | |
| const result = await sequelize.query( | |
| `SELECT * FROM ${tableName} WHERE is_demo = true`, // be explicit | |
| { transaction } | |
| ); | |
| return result[0]; | |
| } |
| export async function deleteDemoVendorsData(transaction: Transaction) { | ||
| const result = await sequelize.query( | ||
| `SELECT id FROM vendors WHERE is_demo;`, | ||
| { | ||
| mapToModel: true, | ||
| model: VendorModel, | ||
| transaction | ||
| } | ||
| ) | ||
| // this might not be needed, but keeping just in case | ||
| await Promise.all(result.map(async r => { | ||
| await sequelize.query(`DELETE FROM vendors_projects WHERE vendor_id = :vendor_id`, | ||
| { | ||
| replacements: { vendor_id: r.id }, | ||
| transaction | ||
| } | ||
| ) | ||
| })) | ||
| await Promise.all(result.map(async r => { | ||
| await sequelize.query(`DELETE FROM vendorrisks WHERE vendor_id = :vendor_id`, | ||
| { | ||
| mapToModel: true, | ||
| model: VendorModel | ||
| replacements: { vendor_id: r.id }, | ||
| transaction | ||
| } | ||
| ) | ||
| await Promise.all(result.map(async r => { | ||
| await sequelize.query(`DELETE FROM vendors_projects WHERE vendor_id = :vendor_id`, | ||
| { | ||
| replacements: { vendor_id: r.id } | ||
| } | ||
| ) | ||
| })) | ||
| } | ||
| await sequelize.query(`DELETE FROM ${tablename} WHERE is_demo;`) | ||
| })) | ||
| await sequelize.query(`DELETE FROM vendors WHERE is_demo;`, { transaction }) |
There was a problem hiding this comment.
🛠️ Refactor suggestion
deleteDemoVendorsData mis-handles the query result and can throw at runtime
sequelize.query() returns [rows, metadata]. The current code iterates over the 2-element wrapper (result.map(...)) which means:
• r is an array of VendorModel instances for the first iteration, not an instance.
• r is the metadata object on the second iteration, which has no .id → undefined, causing every DELETE to use vendor_id = NULL.
Suggested fix + micro-optimisation (bulk deletes instead of N× round-trips):
-export async function deleteDemoVendorsData(transaction: Transaction) {
- const result = await sequelize.query(
- `SELECT id FROM vendors WHERE is_demo;`,
+export async function deleteDemoVendorsData(transaction: Transaction) {
+ const [vendors]: [{ id: number }[]] = await sequelize.query(
+ `SELECT id FROM vendors WHERE is_demo = true`,
{
mapToModel: true,
model: VendorModel,
transaction
}
)
- // this might not be needed, but keeping just in case
- await Promise.all(result.map(async r => {
- await sequelize.query(`DELETE FROM vendors_projects WHERE vendor_id = :vendor_id`,
- {
- replacements: { vendor_id: r.id },
- transaction
- }
- )
- }))
- await Promise.all(result.map(async r => {
- await sequelize.query(`DELETE FROM vendorrisks WHERE vendor_id = :vendor_id`,
- {
- replacements: { vendor_id: r.id },
- transaction
- }
- )
- }))
- await sequelize.query(`DELETE FROM vendors WHERE is_demo;`, { transaction })
+ const ids = vendors.map(v => v.id);
+ if (ids.length === 0) return;
+
+ await sequelize.query(
+ `DELETE FROM vendors_projects WHERE vendor_id IN (:ids)`,
+ { replacements: { ids }, transaction }
+ );
+ await sequelize.query(
+ `DELETE FROM vendorrisks WHERE vendor_id IN (:ids)`,
+ { replacements: { ids }, transaction }
+ );
+ await sequelize.query(
+ `DELETE FROM vendors WHERE id IN (:ids)`,
+ { replacements: { ids }, transaction }
+ );
}Also consider removing the // this might not be needed… comment; stale comments hinder maintainability.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| export async function deleteDemoVendorsData(transaction: Transaction) { | |
| const result = await sequelize.query( | |
| `SELECT id FROM vendors WHERE is_demo;`, | |
| { | |
| mapToModel: true, | |
| model: VendorModel, | |
| transaction | |
| } | |
| ) | |
| // this might not be needed, but keeping just in case | |
| await Promise.all(result.map(async r => { | |
| await sequelize.query(`DELETE FROM vendors_projects WHERE vendor_id = :vendor_id`, | |
| { | |
| replacements: { vendor_id: r.id }, | |
| transaction | |
| } | |
| ) | |
| })) | |
| await Promise.all(result.map(async r => { | |
| await sequelize.query(`DELETE FROM vendorrisks WHERE vendor_id = :vendor_id`, | |
| { | |
| mapToModel: true, | |
| model: VendorModel | |
| replacements: { vendor_id: r.id }, | |
| transaction | |
| } | |
| ) | |
| await Promise.all(result.map(async r => { | |
| await sequelize.query(`DELETE FROM vendors_projects WHERE vendor_id = :vendor_id`, | |
| { | |
| replacements: { vendor_id: r.id } | |
| } | |
| ) | |
| })) | |
| } | |
| await sequelize.query(`DELETE FROM ${tablename} WHERE is_demo;`) | |
| })) | |
| await sequelize.query(`DELETE FROM vendors WHERE is_demo;`, { transaction }) | |
| export async function deleteDemoVendorsData(transaction: Transaction) { | |
| const [vendors]: [{ id: number }[]] = await sequelize.query( | |
| `SELECT id FROM vendors WHERE is_demo = true`, | |
| { | |
| mapToModel: true, | |
| model: VendorModel, | |
| transaction | |
| } | |
| ) | |
| const ids = vendors.map(v => v.id) | |
| if (ids.length === 0) return | |
| await sequelize.query( | |
| `DELETE FROM vendors_projects WHERE vendor_id IN (:ids)`, | |
| { replacements: { ids }, transaction } | |
| ) | |
| await sequelize.query( | |
| `DELETE FROM vendorrisks WHERE vendor_id IN (:ids)`, | |
| { replacements: { ids }, transaction } | |
| ) | |
| await sequelize.query( | |
| `DELETE FROM vendors WHERE id IN (:ids)`, | |
| { replacements: { ids }, transaction } | |
| ) | |
| } |
| const createdProject = await createNewProjectQuery( | ||
| newProject, | ||
| newProject.members | ||
| newProject.members, | ||
| [1], // newProject.framework, | ||
| transaction | ||
| ); | ||
| const assessments: Object = await createNewAssessmentQuery( | ||
| { | ||
| project_id: createdProject.id!, | ||
| }, | ||
| newProject.enable_ai_data_insertion | ||
| ); | ||
| const controls = await createNewControlCategories( | ||
| const frameworks: { [key: string]: Object } = {} | ||
| // if (newProject.framework[0] === 1) { | ||
| const eu = await createEUFrameworkQuery( | ||
| createdProject.id!, | ||
| newProject.enable_ai_data_insertion | ||
| ); | ||
| newProject.enable_ai_data_insertion, | ||
| transaction | ||
| ) | ||
| frameworks["eu"] = eu; | ||
| // } | ||
|
|
||
| if (createdProject) { | ||
| await transaction.commit(); | ||
| return res.status(201).json( | ||
| STATUS_CODE[201]({ | ||
| project: createdProject, | ||
| assessment_tracker: assessments, | ||
| compliance_tracker: controls, | ||
| frameworks | ||
| }) | ||
| ); | ||
| } | ||
|
|
||
| return res.status(503).json(STATUS_CODE[503]({})); | ||
| } catch (error) { | ||
| await transaction.rollback(); | ||
| return res.status(500).json(STATUS_CODE[500]((error as Error).message)); | ||
| } |
There was a problem hiding this comment.
💣 Open transaction is never rolled-back on 503 path
If createNewProjectQuery returns a falsy value you return a 503
response without either commit() or rollback().
The connection remains in an open transaction state and will leak until
Sequelize’s pool evicts it, potentially exhausting the pool under load.
- if (createdProject) {
- await transaction.commit();
- return res.status(201).json(
- STATUS_CODE[201]({
- project: createdProject,
- frameworks
- })
- );
- }
-
- return res.status(503).json(STATUS_CODE[503]({}));
+ if (createdProject) {
+ await transaction.commit();
+ return res.status(201).json(
+ STATUS_CODE[201]({
+ project: createdProject,
+ frameworks
+ }),
+ );
+ }
+
+ // rollback and close – don’t leave the tx hanging
+ await transaction.rollback();
+ return res.status(503).json(STATUS_CODE[503]({}));Consider the sequelize.transaction(async (tx) => { ... }) helper which
auto-handles commit/rollback.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| const createdProject = await createNewProjectQuery( | |
| newProject, | |
| newProject.members | |
| newProject.members, | |
| [1], // newProject.framework, | |
| transaction | |
| ); | |
| const assessments: Object = await createNewAssessmentQuery( | |
| { | |
| project_id: createdProject.id!, | |
| }, | |
| newProject.enable_ai_data_insertion | |
| ); | |
| const controls = await createNewControlCategories( | |
| const frameworks: { [key: string]: Object } = {} | |
| // if (newProject.framework[0] === 1) { | |
| const eu = await createEUFrameworkQuery( | |
| createdProject.id!, | |
| newProject.enable_ai_data_insertion | |
| ); | |
| newProject.enable_ai_data_insertion, | |
| transaction | |
| ) | |
| frameworks["eu"] = eu; | |
| // } | |
| if (createdProject) { | |
| await transaction.commit(); | |
| return res.status(201).json( | |
| STATUS_CODE[201]({ | |
| project: createdProject, | |
| assessment_tracker: assessments, | |
| compliance_tracker: controls, | |
| frameworks | |
| }) | |
| ); | |
| } | |
| return res.status(503).json(STATUS_CODE[503]({})); | |
| } catch (error) { | |
| await transaction.rollback(); | |
| return res.status(500).json(STATUS_CODE[500]((error as Error).message)); | |
| } | |
| const createdProject = await createNewProjectQuery( | |
| newProject, | |
| newProject.members, | |
| [1], // newProject.framework, | |
| transaction | |
| ); | |
| const frameworks: { [key: string]: Object } = {} | |
| // if (newProject.framework[0] === 1) { | |
| const eu = await createEUFrameworkQuery( | |
| createdProject.id!, | |
| newProject.enable_ai_data_insertion, | |
| transaction | |
| ) | |
| frameworks["eu"] = eu; | |
| // } | |
| if (createdProject) { | |
| await transaction.commit(); | |
| return res.status(201).json( | |
| STATUS_CODE[201]({ | |
| project: createdProject, | |
| frameworks | |
| }), | |
| ); | |
| } | |
| // rollback and close – don’t leave the tx hanging | |
| await transaction.rollback(); | |
| return res.status(503).json(STATUS_CODE[503]({})); | |
| } catch (error) { | |
| await transaction.rollback(); | |
| return res.status(500).json(STATUS_CODE[500]((error as Error).message)); | |
| } |
🧰 Tools
🪛 Biome (1.9.4)
[error] 106-106: Don't use 'Object' as a type.
Prefer explicitly define the object shape. This type means "any non-nullable value", which is slightly better than 'unknown', but it's still a broad type.
(lint/complexity/noBannedTypes)
| await Promise.all( | ||
| evidenceFiles!.map(async (file) => { | ||
| const uploadedFile = await uploadFile(file, user_id, project_id, "Compliance tracker group"); | ||
| const uploadedFile = await uploadFile(file, user_id, project_id, "Compliance tracker group", transaction); | ||
| uploadedEvidenceFiles.push({ |
There was a problem hiding this comment.
Possible crash when evidenceFiles / feedbackFiles are omitted
evidenceFiles!.map(...) dereferences a potentially undefined value.
A client sending no files will trigger TypeError: cannot read properties of undefined.
- await Promise.all(
- evidenceFiles!.map(async (file) => {
+ if (evidenceFiles?.length) {
+ await Promise.all(
+ evidenceFiles.map(async (file) => {
…
- })
- );
+ }),
+ );
+ }Do the same guard for feedbackFiles.
| const deleteTable = async ( | ||
| entity: string, | ||
| foreignKey: string, | ||
| id: number, | ||
| // model: Object | ||
| transaction: Transaction | ||
| ) => { | ||
| let tableToDelete = entity; | ||
| if (entity === "vendors") { | ||
| tableToDelete = "vendors_projects" | ||
| // model = VendorsProjectsModel | ||
| }; | ||
|
|
||
| const deleteHelper = async (childObject: Record<string, any>, parent_id: number) => { | ||
| const childTableName = Object.keys(childObject).filter(k => !["foreignKey", "model"].includes(k))[0] | ||
| let childIds: any = {} | ||
| if (childTableName !== "projects_members") { | ||
| if (childTableName === "vendors") { | ||
| childIds = await sequelize.query( | ||
| `SELECT vendor_id FROM vendors_projects WHERE project_id = :project_id`, | ||
| { | ||
| replacements: { project_id: parent_id }, | ||
| mapToModel: true, | ||
| model: VendorsProjectsModel | ||
| } | ||
| ) | ||
| } else { | ||
| childIds = await sequelize.query(`SELECT id FROM ${childTableName} WHERE ${childObject[childTableName].foreignKey} = :x`, | ||
| { | ||
| replacements: { x: parent_id }, | ||
| mapToModel: true, | ||
| model: childObject[childTableName].model | ||
| } | ||
| ) | ||
| } | ||
| await sequelize.query(`DELETE FROM ${tableToDelete} WHERE ${foreignKey} = :x;`, | ||
| { | ||
| replacements: { x: id }, | ||
| mapToModel: true, | ||
| // model: model, | ||
| type: QueryTypes.DELETE, | ||
| transaction | ||
| } | ||
| await Promise.all(Object.keys(childObject[childTableName]) | ||
| .filter(k => !["foreignKey", "model"].includes(k)) | ||
| .map(async k => { | ||
| for (let ch of childIds) { | ||
| let childId = ch.id | ||
| if (childTableName === "vendors") childId = ch.vendor_id | ||
| await deleteHelper({ [k]: childObject[childTableName][k] }, childId) | ||
| ); | ||
| }; |
There was a problem hiding this comment.
Raw table / column interpolation ➜ SQL-injection risk
deleteTable builds a query with back-ticked identifiers substituted
directly from variables:
await sequelize.query(`DELETE FROM ${tableToDelete} WHERE ${foreignKey} = :x`, …)If either tableToDelete or foreignKey is ever derived from external
input this becomes a vector for SQL-injection, and it also breaks when
identifiers are reserved words.
Protect the query by using a whitelist or QueryInterface.quoteIdentifier
to escape names.
- const sql = `DELETE FROM ${tableToDelete} WHERE ${foreignKey} = :x`;
+ const sql = `DELETE FROM ${sequelize.getQueryInterface().quoteIdentifier(tableToDelete)}
+ WHERE ${sequelize.getQueryInterface().quoteIdentifier(foreignKey)} = :x`;| const setClause = [ | ||
| "status", | ||
| "approver", | ||
| "risk_review", | ||
| "owner", | ||
| "reviewer", | ||
| "due_date", | ||
| "implementation_details" | ||
| ].filter(f => { | ||
| if (control[f as keyof ControlEU] !== undefined && control[f as keyof ControlEU]) { | ||
| updateControl[f as keyof ControlEU] = control[f as keyof ControlEU] | ||
| return true | ||
| } | ||
| }).map(f => `${f} = :${f}`).join(", "); | ||
|
|
||
| const query = `UPDATE controls_eu SET ${setClause} WHERE id = :id RETURNING *;`; | ||
|
|
There was a problem hiding this comment.
setClause may be empty → invalid SQL
updateControlEUByIdQuery builds UPDATE controls_eu SET ${setClause}
without checking if any fields were supplied. An empty setClause
produces UPDATE … SET WHERE id …, causing a syntax error.
Add an early-return identical to the one you added for subcontrols:
if (!setClause) {
return sequelize.query(
"SELECT * FROM controls_eu WHERE id = :id",
{ replacements: { id }, mapToModel: true, model: ControlEUModel, transaction }
).then(r => r[0]);
}| const findIsDemo = async (tableName: string, id: number, transaction: Transaction) => { | ||
| const result = await sequelize.query( | ||
| `SELECT is_demo FROM ${tableName} WHERE id = :id`, | ||
| { | ||
| replacements: { id }, transaction | ||
| } | ||
| ) as [{ is_demo: boolean }[], number]; | ||
| return result[0][0].is_demo; | ||
| } |
There was a problem hiding this comment.
findIsDemo crashes when row not found
result[0][0] is accessed without checking the array length.
If the id is invalid an uncaught TypeError will bubble up.
Guard and throw a descriptive 404:
- return result[0][0].is_demo;
+ if (!result[0].length) {
+ throw new Error(`${tableName} row ${id} not found`);
+ }
+ return result[0][0].is_demo;📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| const findIsDemo = async (tableName: string, id: number, transaction: Transaction) => { | |
| const result = await sequelize.query( | |
| `SELECT is_demo FROM ${tableName} WHERE id = :id`, | |
| { | |
| replacements: { id }, transaction | |
| } | |
| ) as [{ is_demo: boolean }[], number]; | |
| return result[0][0].is_demo; | |
| } | |
| const findIsDemo = async (tableName: string, id: number, transaction: Transaction) => { | |
| const result = await sequelize.query( | |
| `SELECT is_demo FROM ${tableName} WHERE id = :id`, | |
| { | |
| replacements: { id }, transaction | |
| } | |
| ) as [{ is_demo: boolean }[], number]; | |
| if (!result[0].length) { | |
| throw new Error(`${tableName} row ${id} not found`); | |
| } | |
| return result[0][0].is_demo; | |
| } |
There was a problem hiding this comment.
Looks good to me @HarshP4585
Let's wait for @samuel-coutinho as well
Describe your changes
Converted existing EU to a framework
Frameworkmodel for the frameworksGET /projectsresponse to haveframeworkobject list: project_framework_id and framework_idRoutes added
GET /eu-ai-act/assessments/byProjectId/<project_framework_id>: Get all the topics, sub-topics, questions and answers for a project based on project_framework_idGET /eu-ai-act/compliances/byProjectId/<project_framework_id>: Get all the control-categories, controls and sub-controls for a project based on project_framework_idGET /eu-ai-act/assessments/progress/<project_framework_id>: Get the progress of assessment tracker for a project based on project_framework_idGET /eu-ai-act/compliances/progress/<project_framework_id>: Get the progress of compliance tracker for a project based on project_framework_idGET /eu-ai-act/all/assessments/progress/<project_framework_id>: Get the progress of all assessment trackersGET /eu-ai-act/all/compliances/progress/<project_framework_id>: Get the progress of all compliance trackersGET /eu-ai-act/topicById?topicId=<topic_id>&projectFrameworkId=<project_framework_id>: Get a topic, sub-topics, questions and answers for a project based on topic_id and project_framework_idGET /eu-ai-act/controlById?controlId=<control_id>&projectFrameworkId=<project_framework_id>: Get a control and its sub-controls for a project based on ** controlId_id** and project_framework_idPATCH /eu-ai-act/saveControls/<control_id>: Update a control and its sub-controls (with evidence files and feedback files) for a compliance based on ** controlId_id**PATCH /eu-ai-act/saveAnswer/<answer_id>: Update an answer and status for a assessment tracker based on ** answer_id**DELETE /eu-ai-act/assessments/byProjectId/<project_framework_id>: Delete all the assessments related metadata ie. answers based on project_framework_idDELETE /eu-ai-act/compliances/byProjectId/<project_framework_id>: Delete all the compliance related metadata ie. controls and sub-controls based on project_framework_idWrite your issue number after "Fixes "
Fixes #1239
Please ensure all items are checked off before requesting a review:
Summary by CodeRabbit
New Features
Enhancements
Bug Fixes
Chores