refactor(api)!: Refactor environment, secret and variable functionality

[rajdip-b]

User description

Description

This PR introduces a lot of breaking changes. Listing them down below:

• The approval feature is now removed
• Variable and secrets now store the revisions in a clustered manner, grouped by environments.
• Environments don't have a default tag anymore

---

PR Type

Enhancement, Tests

---

Description

• Removed approval-related code and logic across multiple services.
• Refactored secret and variable services to handle multiple environments.
• Simplified project creation, update, and deletion logic.
• Updated tests to reflect changes in services and removed approval-related tests.
• Simplified authority checks by removing approval-related logic.

---

Changes walkthrough 📝

	Relevant files
Enhancement	secret.service.ts Refactor secret service to support multiple environments and remove approval logic. apps/api/src/secret/service/secret.service.ts Removed approval-related code. Refactored secret creation to handle multiple environments. Updated secret update logic to handle multiple environments. Simplified secret deletion logic. +316/-587 variable.service.ts Refactor variable service to support multiple environments and remove approval logic. apps/api/src/variable/service/variable.service.ts Removed approval-related code. Refactored variable creation to handle multiple environments. Updated variable update logic to handle multiple environments. Simplified variable deletion logic. +281/-528 project.service.ts Refactor project service to remove approval logic and handle multiple environments. apps/api/src/project/service/project.service.ts Removed approval-related code. Updated project creation to handle multiple environments without default tags. Simplified project update and deletion logic. +197/-368 authority-checker.service.ts Simplify authority checker service by removing approval-related checks. apps/api/src/common/authority-checker.service.ts Removed approval-related checks. Simplified authority checks for projects, environments, variables, and secrets. +23/-107 create.environment.ts Simplify environment creation DTO by removing unnecessary validation. apps/api/src/environment/dto/create.environment/create.environment.ts Removed IsBoolean import and usage. +1/-5
Tests	secret.e2e.spec.ts Update secret service tests to reflect refactored logic and multiple environments. apps/api/src/secret/secret.e2e.spec.ts Updated tests to reflect changes in secret service. Removed tests related to approval logic. Added tests for handling multiple environments. +87/-354 variable.e2e.spec.ts Update variable service tests to reflect refactored logic and multiple environments. apps/api/src/variable/variable.e2e.spec.ts Updated tests to reflect changes in variable service. Removed tests related to approval logic. Added tests for handling multiple environments. +64/-311

---

💡 PR-Agent usage:
Comment /help on the PR to get a list of all available PR-Agent tools and their descriptions

[codiumai-pr-agent-free]

PR Reviewer Guide 🔍

⏱️ Estimated effort to review [1-5]	5
🧪 Relevant tests	Yes
🔒 Security concerns	No
⚡ Key issues to review	Possible Bug: The refactoring removes the concept of default environments and approval features. Ensure that this change aligns with the intended use cases and does not affect existing functionalities unexpectedly.
	Data Integrity: The migration script associated with this PR should be carefully reviewed to ensure that it handles data correctly and preserves the integrity of existing data during the transition.
	Dependency Check: The removal of approval-related features might affect other parts of the system that depend on these functionalities. Verify that all dependencies are correctly updated or removed.

[codiumai-pr-agent-free]

PR Code Suggestions ✨

Category	Suggestion	Score
Security	Ensure that the environment belongs to the project during variable creation Implement a check to ensure that the environmentId provided in each entry of dto.entries actually belongs to the project associated with projectId. This prevents users from mistakenly or maliciously attempting to create variable versions in environments not associated with the given project. apps/api/src/variable/service/variable.service.ts [62-69] const environmentIds = dto.entries.map((entry) => entry.environmentId) for (const environmentId of environmentIds) { + const environment = await this.prisma.environment.findUnique({ + where: { id: environmentId }, + select: { projectId: true } + }); + if (environment.projectId !== projectId) { + throw new ForbiddenException(`Environment ${environmentId} is not part of the project ${projectId}`); + } await this.authorityCheckerService.checkAuthorityOverEnvironment({ userId: user.id, entity: { id: environmentId }, authority: Authority.READ_ENVIRONMENT, prisma: this.prisma }) } Suggestion importance[1-10]: 10 Why: This suggestion addresses a significant security concern by ensuring that environments are correctly associated with the project, preventing unauthorized access or manipulation.	10
Data integrity	Verify existence of workspace before project creation to maintain data integrity Implement a check to ensure that the workspaceId provided exists in the database before proceeding with project creation. This will prevent the creation of projects in non-existent workspaces, thus maintaining data integrity. apps/api/src/project/service/project.service.ts [47-48] +const workspaceExists = await this.prisma.workspace.findUnique({ where: { id: workspaceId } }); +if (!workspaceExists) { + throw new Error('Workspace does not exist.'); +} const workspace = ... Suggestion importance[1-10]: 10 Why: Implementing a check to ensure that the workspaceId exists in the database before proceeding with project creation is essential for maintaining data integrity. This prevents the creation of projects in non-existent workspaces, which could lead to data inconsistencies.	10
Possible issue	Add error handling to the createSecret method to improve robustness Consider adding error handling for the createSecret method to manage cases where the secret creation fails due to database issues or other exceptions. This will improve the robustness of the method. apps/api/src/secret/service/secret.service.ts [47-115] -const secret = await this.prisma.secret.create({ - data: { - name: dto.name, - note: dto.note, - rotateAt: addHoursToDate(dto.rotateAfter), - versions: { - createMany: { - data: await Promise.all( - dto.entries.map(async (entry) => ({ - value: await encrypt(project.publicKey, entry.value), - version: 1, - createdById: user.id, - environmentId: entry.environmentId - })) - ) +let secret; +try { + secret = await this.prisma.secret.create({ + data: { + name: dto.name, + note: dto.note, + rotateAt: addHoursToDate(dto.rotateAfter), + versions: { + createMany: { + data: await Promise.all( + dto.entries.map(async (entry) => ({ + value: await encrypt(project.publicKey, entry.value), + version: 1, + createdById: user.id, + environmentId: entry.environmentId + })) + ) + } + }, + project: { + connect: { + id: projectId + } } }, - project: { - connect: { - id: projectId + include: { + project: { + select: { + workspaceId: true + } + }, + versions: { + select: { + environmentId: true, + value: true + } } } - }, - include: { - project: { - select: { - workspaceId: true - } - }, - versions: { - select: { - environmentId: true, - value: true - } - } - } -}) + }); +} catch (error) { + this.logger.error(`Failed to create secret: ${error}`); + throw new InternalServerErrorException(`Failed to create secret`); +} Suggestion importance[1-10]: 9 Why: The suggestion correctly identifies a potential issue with the createSecret method where database or other exceptions could cause the method to fail without proper error handling. Adding a try-catch block and logging the error improves the robustness and maintainability of the code.	9
	Add error handling for key pair generation to improve method robustness Consider adding error handling for the createKeyPair() function to manage potential failures in key generation. This will ensure the robustness of the createProject method by handling scenarios where key generation might fail. apps/api/src/project/service/project.service.ts [62-63] -const { publicKey, privateKey } = createKeyPair() +let publicKey, privateKey; +try { + ({ publicKey, privateKey } = createKeyPair()); +} catch (error) { + throw new Error('Failed to generate key pair: ' + error.message); +} Suggestion importance[1-10]: 9 Why: Adding error handling for the createKeyPair() function is crucial for robustness. It ensures that any failure in key generation is properly managed, preventing potential runtime errors and improving the reliability of the createProject method.	9
	Add validation to check if environment2.id exists before using it Consider checking if environment2.id exists before using it in the entries array for creating secrets. This ensures that the environment is valid and prevents potential runtime errors or misconfigurations. apps/api/src/secret/secret.e2e.spec.ts [163-164] -entries: [ - { - environmentId: environment2.id, - value: 'Secret 1 value' - } -] +if (environment2 && environment2.id) { + entries: [ + { + environmentId: environment2.id, + value: 'Secret 1 value' + } + ] +} Suggestion importance[1-10]: 8 Why: This suggestion adds a validation step to ensure environment2.id exists before using it, which can prevent potential runtime errors or misconfigurations. This is a good practice for improving code robustness.	8
	Check usage of defaultWorkspace before deletion to avoid potential issues When deleting properties from an object, consider if other parts of the application might still be using them. If defaultWorkspace is used elsewhere, this operation might lead to undefined behavior or errors. apps/api/src/secret/secret.e2e.spec.ts [105] -delete createUser1.defaultWorkspace +// Ensure no other parts of the application require `defaultWorkspace` before deleting +if (canDeleteDefaultWorkspace(createUser1)) { + delete createUser1.defaultWorkspace +} Suggestion importance[1-10]: 6 Why: This suggestion ensures that defaultWorkspace is not used elsewhere before deletion, which can prevent undefined behavior or errors. It is a good practice but not crucial for the current context.	6
Robustness	Add error handling for the variable creation process Consider adding error handling for the case where the variable creation fails due to database constraints or other issues. This will improve the robustness of the createVariable method. apps/api/src/variable/service/variable.service.ts [73-87] -const variable = await this.prisma.variable.create({ - data: { - name: dto.name, - note: dto.note, - versions: { - createMany: { - data: dto.entries.map((entry) => ({ - value: entry.value, - createdById: user.id, - environmentId: entry.environmentId - })) - } - }, - project: { - connect: { - id: projectId +let variable; +try { + variable = await this.prisma.variable.create({ + data: { + name: dto.name, + note: dto.note, + versions: { + createMany: { + data: dto.entries.map((entry) => ({ + value: entry.value, + createdById: user.id, + environmentId: entry.environmentId + })) + } + }, + project: { + connect: { + id: projectId + } } } - } -}) + }); +} catch (error) { + throw new InternalServerErrorException(`Failed to create variable: ${error.message}`); +} Suggestion importance[1-10]: 9 Why: Adding error handling for the variable creation process is crucial for robustness, ensuring that any issues during the database operation are properly caught and managed.	9
	Implement a rollback mechanism for update operations To maintain consistency and ensure data integrity, consider implementing a rollback mechanism in the updateVariable method to handle any failures during the update operations. apps/api/src/variable/service/variable.service.ts [230-232] -const tx = await this.prisma.$transaction(op) -const updatedVariable = tx[0] +let updatedVariable; +try { + const tx = await this.prisma.$transaction(op); + updatedVariable = tx[0]; +} catch (error) { + await this.prisma.$rollback(); + throw new InternalServerErrorException(`Update failed: ${error.message}`); +} Suggestion importance[1-10]: 8 Why: Implementing a rollback mechanism enhances robustness by ensuring data integrity in case of failures during update operations, although the existing transaction mechanism already provides some level of atomicity.	8
Best practice	Implement error handling for the secret creation process Add error handling for the createSecret method to manage exceptions or failed secret creation attempts effectively. apps/api/src/secret/secret.e2e.spec.ts [155-169] -secret1 = (await secretService.createSecret( - user1, - { - name: 'Secret 1', - rotateAfter: '24', - note: 'Secret 1 note', - entries: [ - { - environmentId: environment2.id, - value: 'Secret 1 value' - } - ] - }, - project1.id -)) as Secret +try { + secret1 = (await secretService.createSecret( + user1, + { + name: 'Secret 1', + rotateAfter: '24', + note: 'Secret 1 note', + entries: [ + { + environmentId: environment2.id, + value: 'Secret 1 value' + } + ] + }, + project1.id + )) as Secret +} catch (error) { + console.error('Failed to create secret:', error); +} Suggestion importance[1-10]: 9 Why: Adding error handling for the createSecret method is a best practice that can help manage exceptions or failed attempts effectively, improving the robustness and reliability of the code.	9
	Use specific types instead of any to enhance type safety and maintainability Replace the generic type any with a more specific type or interface for the data object in the createProject method to improve type safety and code maintainability. apps/api/src/project/service/project.service.ts [66-67] -const data: any = { +interface ProjectData { + name: string; + description: string; + storePrivateKey: boolean; + publicKey: string; + accessLevel: string; +} +const data: ProjectData = { name: dto.name, description: dto.description, ... } Suggestion importance[1-10]: 8 Why: Replacing the generic type any with a more specific type or interface for the data object improves type safety and code maintainability. This change helps catch type-related errors at compile time and makes the code more readable and maintainable.	8
Data validation	Validate dto properties to ensure all required fields are provided and valid Add validation for the dto object properties in the createProject method to ensure that all required fields are provided and valid before proceeding with project creation. apps/api/src/project/service/project.service.ts [66-67] +if (!dto.name || !dto.description) { + throw new Error('Missing required project details.'); +} const data: any = { name: dto.name, description: dto.description, ... } Suggestion importance[1-10]: 9 Why: Adding validation for the dto object properties ensures that all required fields are provided and valid before proceeding with project creation. This prevents potential runtime errors and ensures that the project creation process is robust and reliable.	9
Performance	Optimize environment authority checks by using asynchronous batch processing Optimize the loop that checks for authority over environments by batching the requests or restructuring the logic to reduce the number of await calls inside the loop, which can improve performance. apps/api/src/variable/service/variable.service.ts [154-161] -for (const environmentId of environmentIds) { - await this.authorityCheckerService.checkAuthorityOverEnvironment({ +await Promise.all(environmentIds.map(environmentId => + this.authorityCheckerService.checkAuthorityOverEnvironment({ userId: user.id, entity: { id: environmentId }, authority: Authority.READ_ENVIRONMENT, prisma: this.prisma }) -} +)); Suggestion importance[1-10]: 7 Why: This optimization can improve performance by reducing the number of await calls inside the loop, but it is a minor enhancement compared to the other suggestions.	7
Enhancement	Clarify the time unit in the rotateAfter property Ensure that the rotateAfter property is correctly formatted as a time unit (e.g., hours, days). Currently, it's just a string '24', which might be ambiguous or incorrect depending on the expected format. apps/api/src/secret/secret.e2e.spec.ts [159] -rotateAfter: '24' +rotateAfter: '24h' # Assuming 'h' stands for hours Suggestion importance[1-10]: 7 Why: Clarifying the time unit in the rotateAfter property improves code readability and prevents ambiguity. However, it is a minor enhancement rather than a critical fix.	7

[codecov]

Codecov Report

Attention: Patch coverage is 92.95775% with 20 lines in your changes missing coverage. Please review.

Project coverage is 88.33%. Comparing base (ce50743) to head (2fc5dcf).
Report is 28 commits behind head on develop.

Files	Patch %	Lines
apps/api/src/secret/service/secret.service.ts	90.80%	8 Missing ⚠️
...rc/variable/dto/create.variable/create.variable.ts	54.54%	5 Missing ⚠️
apps/api/src/variable/service/variable.service.ts	94.44%	4 Missing ⚠️
.../api/src/secret/dto/create.secret/create.secret.ts	70.00%	3 Missing ⚠️

Additional details and impacted files

@@             Coverage Diff             @@
##           develop     #270      +/-   ##
===========================================
- Coverage    91.71%   88.33%   -3.39%     
===========================================
  Files          111      106       -5     
  Lines         2510     2271     -239     
  Branches       469      351     -118     
===========================================
- Hits          2302     2006     -296     
- Misses         208      265      +57     

Flag	Coverage Δ
api-e2e-tests	88.33% <92.95%> (-3.39%)	⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

[sonarcloud]

Quality Gate passed

Issues
1 New issue
2 Accepted issues

Measures
0 Security Hotspots
No data about Coverage
7.0% Duplication on New Code

See analysis details on SonarCloud

[rajdip-b]

🎉 This PR is included in version 2.0.0 🎉

The release is available on GitHub release

Your semantic-release bot 📦🚀