-
Notifications
You must be signed in to change notification settings - Fork 577
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[Refactor] Package Utils/Constants #8748
Conversation
Caution Review failedThe pull request is closed. WalkthroughThis pull request introduces a new Changes
Sequence DiagramsequenceDiagram
participant Constants as @gauzy/constants
participant Common as @gauzy/common
participant Core as @gauzy/core
participant Utils as @gauzy/utils
Constants->>Common: Provide centralized constants
Common->>Core: Import constants via new package
Utils->>Constants: Use constants for code generation
Core->>Constants: Reference metadata and configuration constants
Possibly related PRs
Poem
📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (1)
✨ Finishing Touches
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
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 (
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
🧹 Nitpick comments (13)
packages/constants/src/lib/github.ts (1)
9-12
: Consider future-proofing with an enum or string literal union.
While the current setup is fine, an enum could potentially allow for simpler auto-completion and usage if you anticipate more sync tags.packages/constants/src/lib/code.ts (1)
6-10
: Security awareness: Avoid hardcoding magic codes unless necessary.
Storing demo or magic codes in a code constant can introduce a security risk if used in production. Consider environment variables or secure storage for sensitive constants.packages/utils/src/lib/code-generator.ts (1)
9-19
: Use a cryptographically secure random generator for sensitive codes.
For general alpha-numeric generation,Math.random()
is acceptable, but if these codes are used for authentication or security, consider using a crypto-based approach.-export function generateAlphaNumericCode(length: number = ALPHA_NUMERIC_CODE_LENGTH): string { - const characters = 'ABCDEFGHIJKLMNOPQRSTUVWXYZ0123456789'; - let code = ''; - for (let i = 0; i < length; i++) { - const index = Math.floor(Math.random() * characters.length); - code += characters[index]; - } - return code; +import { randomBytes } from 'crypto'; +export function generateAlphaNumericCode( + length: number = ALPHA_NUMERIC_CODE_LENGTH +): string { + const characters = 'ABCDEFGHIJKLMNOPQRSTUVWXYZ0123456789'; + let code = ''; + const randomBuffer = randomBytes(length); + for (let i = 0; i < length; i++) { + // Use randomBuffer to ensure cryptographic randomness + const index = randomBuffer[i] % characters.length; + code += characters[index]; + } + return code; }packages/utils/src/lib/array-to-object.ts (2)
1-20
: Well-documented function usage
The doc comments are comprehensive and helpful, illustrating the function's purpose and usage with a clear example. Consider adding a note about how duplicate keys (if any) are handled, so users can anticipate that later items overwrite earlier items.
21-30
: Refine the return type for better type safety
Currently, the return type isRecord<string, any>
, which could be made more specific to improve type safety. If your key is guaranteed to be something convertible to a string, and you want to store property values accurately, consider returningRecord<string, T[V]>
:-export function arrayToObject<T, K extends keyof T, V extends keyof T>( - array: T[], - key: K, - value: V -): Record<string, any> { - return array.reduce((acc: Record<string, any>, item: T) => { +export function arrayToObject<T extends Record<string, any>, K extends keyof T, V extends keyof T>( + array: T[], + key: K, + value: V +): Record<string, T[V]> { + return array.reduce((acc: Record<string, T[V]>, item: T) => { acc[String(item[key])] = item[value]; return acc; }, {}); }packages/config/src/lib/default-configuration.ts (1)
5-7
: Unused import from@gauzy/contracts
Your new imports for API constants are correct. However, the lineimport {} from '@gauzy/contracts';
brings in nothing and appears to be unused. If no symbols are needed from@gauzy/contracts
, consider removing it to avoid confusion.-import {} from '@gauzy/contracts';
packages/core/src/lib/email-template/queries/handlers/email-template.generate-preview.handler.ts (1)
85-87
: Default user/tenant data
The default tenant and user information, along with theresetLink
, is likely for demonstration or local testing. If it is a placeholder, consider removing it or documenting it explicitly to avoid confusion in production.packages/ui-core/shared/src/lib/project/project-mutation/project-mutation.component.ts (1)
625-625
: Conditional fallback for syncTagUsing
SyncTags.GAUZY
as a fallback is helpful. Double-check if different integrations or project settings may require a different default sync tag, ensuring coverage in your testing.packages/plugins/integration-github/src/lib/github/github-sync.service.ts (1)
381-381
: Maintainability reminder: Expandability of label checks.
Hardcoding an array of labels is fine here, but if additional labels or dynamic configurations become necessary in the future, consider implementing a more flexible approach.packages/core/src/lib/invite/invite.service.ts (2)
8-8
: Remove unused import if not utilized.
ALPHA_NUMERIC_CODE_LENGTH
appears to be imported but not used. Consider removing it to keep the import list clean.- import { ALPHA_NUMERIC_CODE_LENGTH, DEFAULT_INVITE_EXPIRY_PERIOD } from '@gauzy/constants'; + import { DEFAULT_INVITE_EXPIRY_PERIOD } from '@gauzy/constants';
34-34
: Check necessity of multiple imports.
If any of these imports (MultiORMEnum
,freshTimestamp
, etc.) are unused or rarely used, consider pruning or reorganizing them to keep imports minimal.packages/core/src/lib/email-reset/email-reset.service.ts (1)
Line range hint
41-91
: Add test coverage for email reset functionality.The email reset logic contains critical security-related functionality but lacks comprehensive test coverage.
Would you like me to help generate unit tests for the following scenarios?
- Successful email reset request
- Duplicate email handling
- Invalid code verification
- Expired code handling
packages/common/src/lib/guards/feature-flag-enabled.guard.ts (1)
67-67
: Simplify boolean expression.The double negation
!!flagFeatures[flag]
is redundant and can be simplified.- if (!!flagFeatures[flag]) { + if (flagFeatures[flag]) {🧰 Tools
🪛 Biome (1.9.4)
[error] 67-67: Avoid redundant double-negation.
It is not necessary to use double-negation when a value will already be coerced to a boolean.
Unsafe fix: Remove redundant double-negation(lint/complexity/noExtraBooleanCast)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (48)
packages/common/package.json
(1 hunks)packages/common/src/index.ts
(0 hunks)packages/common/src/lib/constants.ts
(0 hunks)packages/common/src/lib/decorators/feature-flag.decorator.ts
(1 hunks)packages/common/src/lib/decorators/public.decorator.ts
(2 hunks)packages/common/src/lib/guards/feature-flag-enabled.guard.ts
(4 hunks)packages/config/package.json
(1 hunks)packages/config/src/lib/default-configuration.ts
(1 hunks)packages/constants/src/index.ts
(1 hunks)packages/constants/src/lib/ai.ts
(1 hunks)packages/constants/src/lib/api.ts
(1 hunks)packages/constants/src/lib/code.ts
(1 hunks)packages/constants/src/lib/github.ts
(1 hunks)packages/constants/src/lib/reflect-metadata.ts
(1 hunks)packages/contracts/src/lib/github.model.ts
(1 hunks)packages/core/src/lib/auth/auth.service.ts
(6 hunks)packages/core/src/lib/auth/email-confirmation.service.ts
(2 hunks)packages/core/src/lib/constants.ts
(0 hunks)packages/core/src/lib/core/utils.ts
(1 hunks)packages/core/src/lib/dev-config.ts
(1 hunks)packages/core/src/lib/email-reset/email-reset.service.ts
(2 hunks)packages/core/src/lib/email-template/queries/handlers/email-template.generate-preview.handler.ts
(2 hunks)packages/core/src/lib/invite/invite.service.ts
(4 hunks)packages/core/src/lib/organization-team-join-request/dto/validate-join-request.dto.ts
(1 hunks)packages/core/src/lib/organization-team-join-request/organization-team-join-request.service.ts
(4 hunks)packages/core/src/lib/shared/decorators/permissions.decorator.ts
(1 hunks)packages/core/src/lib/shared/decorators/roles.decorator.ts
(1 hunks)packages/core/src/lib/shared/guards/auth.guard.ts
(1 hunks)packages/core/src/lib/shared/guards/feature-flag.guard.ts
(1 hunks)packages/core/src/lib/shared/guards/organization-permission.guard.ts
(1 hunks)packages/core/src/lib/shared/guards/permission.guard.ts
(1 hunks)packages/core/src/lib/shared/guards/role.guard.ts
(1 hunks)packages/core/src/lib/shared/guards/tenant-permission.guard.ts
(1 hunks)packages/core/src/lib/stats/stats.guard.ts
(1 hunks)packages/core/src/lib/user/dto/user-code.dto.ts
(1 hunks)packages/plugin/src/lib/constants.ts
(0 hunks)packages/plugin/src/lib/plugin-metadata.ts
(1 hunks)packages/plugin/src/lib/plugin.ts
(1 hunks)packages/plugins/integration-ai/src/lib/constants.ts
(0 hunks)packages/plugins/integration-ai/src/lib/gauzy-ai.module.ts
(3 hunks)packages/plugins/integration-ai/src/lib/request-config.provider.ts
(1 hunks)packages/plugins/integration-github-ui/src/lib/components/view/view.component.ts
(3 hunks)packages/plugins/integration-github/src/lib/github/github-sync.service.ts
(2 hunks)packages/ui-core/shared/src/lib/project/project-mutation/project-mutation.component.ts
(3 hunks)packages/utils/package.json
(1 hunks)packages/utils/src/index.ts
(1 hunks)packages/utils/src/lib/array-to-object.ts
(1 hunks)packages/utils/src/lib/code-generator.ts
(1 hunks)
💤 Files with no reviewable changes (5)
- packages/plugins/integration-ai/src/lib/constants.ts
- packages/plugin/src/lib/constants.ts
- packages/common/src/index.ts
- packages/core/src/lib/constants.ts
- packages/common/src/lib/constants.ts
✅ Files skipped from review due to trivial changes (18)
- packages/constants/src/lib/ai.ts
- packages/core/src/lib/user/dto/user-code.dto.ts
- packages/core/src/lib/shared/decorators/permissions.decorator.ts
- packages/core/src/lib/shared/guards/permission.guard.ts
- packages/common/src/lib/decorators/public.decorator.ts
- packages/core/src/lib/stats/stats.guard.ts
- packages/core/src/lib/shared/guards/organization-permission.guard.ts
- packages/core/src/lib/shared/guards/role.guard.ts
- packages/common/src/lib/decorators/feature-flag.decorator.ts
- packages/constants/src/lib/api.ts
- packages/core/src/lib/shared/guards/feature-flag.guard.ts
- packages/plugin/src/lib/plugin.ts
- packages/constants/src/lib/reflect-metadata.ts
- packages/core/src/lib/shared/decorators/roles.decorator.ts
- packages/core/src/lib/shared/guards/tenant-permission.guard.ts
- packages/core/src/lib/organization-team-join-request/dto/validate-join-request.dto.ts
- packages/contracts/src/lib/github.model.ts
- packages/core/src/lib/shared/guards/auth.guard.ts
🧰 Additional context used
🪛 Biome (1.9.4)
packages/common/src/lib/guards/feature-flag-enabled.guard.ts
[error] 67-67: Avoid redundant double-negation.
It is not necessary to use double-negation when a value will already be coerced to a boolean.
Unsafe fix: Remove redundant double-negation
(lint/complexity/noExtraBooleanCast)
⏰ Context from checks skipped due to timeout of 90000ms (3)
- GitHub Check: Codacy Static Code Analysis
- GitHub Check: test
- GitHub Check: build
🔇 Additional comments (51)
packages/plugin/src/lib/plugin-metadata.ts (8)
2-2
: Doc comment provides excellent clarity
The explanation is concise, and it accurately describes the purpose of the metadata keys.
5-7
: Good documentation forENTITIES
Explicitly describing the role of entities in a plugin aids in maintainability.
10-12
: Clear explanation ofSUBSCRIBERS
This comment mirrors the pattern established forENTITIES
, promoting consistency.
15-17
: Well-labeledEXTENSIONS
key
Providing explicit documentation for extensions further streamlines plugin usage.
20-22
: Sufficient detail forCONFIGURATION
key
The comment conveys its purpose effectively and aligns with the rest of the keys.
24-24
: Usingas const
is a best practice
This ensures that TypeScript infers literal types for each property, preserving type precision.
26-28
: Suitable documentation for the type definition
The descriptive comment clarifies the intent behind extracting valid plugin metadata keys.
29-29
: Type extraction approach is very effective
This ensures only valid metadata keys are used, reducing the likelihood of runtime errors.packages/utils/src/index.ts (1)
9-9
: Ensure the new export is fully tested and documentedBy re-exporting the
code-generator
module, you make these utilities easily accessible throughout the codebase and improve maintainability. However, please verify that thecode-generator
functionality is adequately tested (unit tests, integration tests, etc.) and documented, so that any related dependencies in other packages remain stable and coherent.Below is a quick script you can run to locate test files related to
code-generator
across the repo:packages/plugins/integration-ai/src/lib/request-config.provider.ts (2)
2-2
: Good import usage.The import statement for
GAUZY_AI_CONFIG_TOKEN
aligns well with the new centralized constants approach.
11-11
: Verify the removal of any leftover references.The injection token is now updated to
GAUZY_AI_CONFIG_TOKEN
. Ensure that there are no remaining references to the old token (GAUZY_AI_CONFIG_OPTIONS
) across the codebase.Below is a script to detect any stale references:
✅ Verification successful
Migration to
GAUZY_AI_CONFIG_TOKEN
is complete and verified ✅The old token has been completely removed from the codebase, and the new token is properly defined and consistently used across all relevant files. No further action is needed.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Search for references to the old token GAUZY_AI_CONFIG_OPTIONS in the entire codebase. rg "GAUZY_AI_CONFIG_OPTIONS" -A 5 -B 5Length of output: 38
Script:
#!/bin/bash # Search for both tokens with various patterns echo "=== Searching for old token (case-insensitive) ===" rg -i "GAUZY.*CONFIG.*OPTIONS" echo -e "\n=== Searching for new token references ===" rg "GAUZY_AI_CONFIG_TOKEN" echo -e "\n=== Searching in TypeScript/JavaScript files specifically ===" fd -e ts -e js -x rg -i "GAUZY.*CONFIG"Length of output: 26675
packages/plugins/integration-ai/src/lib/gauzy-ai.module.ts (3)
4-4
: Updated import path is consistent.Importing
GAUZY_AI_CONFIG_TOKEN
from@gauzy/constants
is consistent with your refactoring strategy.
44-44
: Proper use of the new constant in providers.Using
GAUZY_AI_CONFIG_TOKEN
in the provider ensures consistent configuration injection. This helps avoid conflicts with the old token name.
53-53
: Export statement aligns with the refactored token.Exporting
GAUZY_AI_CONFIG_TOKEN
provides a cleaner, centralized approach for sharing the configuration token.packages/constants/src/index.ts (2)
1-5
: Nice consolidation of exports for improved discoverability.
Exporting sub-libraries individually from a single entry point can make it easier for developers to locate related constants.
7-7
: Confirm that each import aligns with existing usage.
Double-check downstream files to ensure that updated import paths reference the correct sub-library and that no naming collisions occur with these new exports.✅ Verification successful
Export verification successful - all imports properly aligned
The task-related exports are correctly used across the codebase in contracts, core entities, and UI components with consistent import paths and proper usage patterns. No naming collisions or misalignments detected.🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Verify usage consistency of newly exported modules from @gauzy/constants rg -A 3 "@gauzy/constants" | grep "from './lib"Length of output: 50
Script:
#!/bin/bash # Find all imports from @gauzy/constants and their usage echo "=== Files importing from @gauzy/constants ===" rg -l "@gauzy/constants" echo -e "\n=== Import statements from @gauzy/constants ===" ast-grep --pattern 'import { $$$ } from "@gauzy/constants"' echo -e "\n=== Content of lib/task ===" fd "task.ts" --exec cat {}Length of output: 37544
Script:
#!/bin/bash # Find the actual task exports echo "=== Content of lib/task.ts in constants package ===" fd -p "packages/constants/src/lib/task.ts" --exec cat {} echo -e "\n=== Imports specifically referencing task items ===" ast-grep --pattern 'import { $$$ } from "@gauzy/constants"' | grep -i "task"Length of output: 1164
Script:
#!/bin/bash echo "=== Usage of task-related constants ===" rg -l "TaskProofOfCompletionTypeEnum|DEFAULT_TASK_NOTIFY_PERIOD|DEFAULT_AUTO_CLOSE_ISSUE_PERIOD|DEFAULT_AUTO_ARCHIVE_ISSUE_PERIOD|DEFAULT_PROOF_COMPLETION_TYPE" --type ts echo -e "\n=== Specific import statements and usage ====" rg "TaskProofOfCompletionTypeEnum|DEFAULT_TASK_NOTIFY_PERIOD|DEFAULT_AUTO_CLOSE_ISSUE_PERIOD|DEFAULT_AUTO_ARCHIVE_ISSUE_PERIOD|DEFAULT_PROOF_COMPLETION_TYPE" --type ts -A 1Length of output: 8612
packages/constants/src/lib/github.ts (1)
4-7
: Appropriate use ofas const
for literal type enforcement.
This pattern helps ensure that the sync tags remain strictly typed. No changes needed.packages/utils/src/lib/code-generator.ts (1)
1-2
: Stay consistent with naming and usage conventions in imports.
Confirm that@gauzy/constants
is the intended import path. If so, it looks good.packages/core/src/lib/dev-config.ts (2)
1-2
: Migration of constants to@gauzy/constants
Switching the import of API constants from@gauzy/common
to@gauzy/constants
is consistent with the new centralized constants approach. It keeps your code aligned with the overall refactoring strategy.
5-7
: Clearer documentation fordevConfig
Converting the preceding comment into JSDoc format enhances code readability and clarity, making it easier for new developers to understand the purpose ofdevConfig
.packages/core/src/lib/email-template/queries/handlers/email-template.generate-preview.handler.ts (4)
5-5
: Adoption ofgenerateAlphaNumericCode
Replacing older code generation functions withgenerateAlphaNumericCode
encourages consistency across the codebase. Ensure the new function provides the required code length or complexity for your use case.
10-11
: Concise constructor format
Combining the constructor parameters into a single line makes the code more succinct. This change is purely stylistic, so ensure the team's style guidelines allow for it.
13-13
: Method signature remains clear
Consolidating the method signature onto one line doesn't impact functionality. The code remains readable.
76-78
: UsinggenerateAlphaNumericCode
forinviteCode
andverificationCode
Directly callinggenerateAlphaNumericCode()
for these codes is succinct and avoids repeating logic. Confirm there's no need for any optional parameters (e.g., length) to meet specific security or uniqueness requirements.packages/core/src/lib/auth/email-confirmation.service.ts (2)
17-18
: Check dependency usage & consistencyThe new import of
generateAlphaNumericCode
looks consistent with the overall refactoring approach. However, ensure that thedeepMerge
import remains necessary here. Any unused imports can be removed to keep the code lean.
55-55
: Confirm generated code lengthSwitching from a fixed-length code to
generateAlphaNumericCode()
without a specified length may produce codes of different lengths depending on its default behavior. Double-check that this dynamic length meets the project's requirements (e.g., a longer or shorter code might impact user experience or security).packages/core/src/lib/organization-team-join-request/organization-team-join-request.service.ts (4)
4-6
: Validate module importsThe imports for
moment
andenvironment
appear correct. Confirm that the environment configuration is needed in this file for all relevant references. If any import is unused, please remove it to simplify maintenance.
17-17
: Adopt the new code generator consistentlyThe import of
generateAlphaNumericCode
aligns with the broader refactor. Confirm that all references to the old random code generator have been removed and that length consistency is handled if different code lengths are required across the app.
94-94
: Use of generateAlphaNumericCode in create methodThe switch to
generateAlphaNumericCode()
makes the code generation consistent with the rest of the refactor. Ensure that existing unit tests are updated to cover this new approach if code length or format has changed.
215-215
: Use of generateAlphaNumericCode in resendConfirmationCode methodSimilarly here, confirm that all business logic (such as validation or user prompts requiring a fixed code format) is up to date for the newly generated alphanumeric code.
packages/core/src/lib/core/utils.ts (1)
18-18
: Removal of old code generation functionsImporting
ALPHA_NUMERIC_CODE_LENGTH
from@gauzy/constants
and removing the legacy code generators helps unify code generation. Verify that no references to the removed functions remain in the codebase.packages/ui-core/shared/src/lib/project/project-mutation/project-mutation.component.ts (2)
11-11
: New SyncTags importImporting
SyncTags
from@gauzy/constants
is aligned with the centralized constants strategy. Confirm that all relevant references to synchronization tags were updated project-wide so no orphan references remain.
34-35
: Environment import & additional UI referencesThe addition and rearrangement of imports (e.g.,
environment
from@gauzy/ui-config
andDUMMY_PROFILE_IMAGE
from@gauzy/ui-core/common
) appear clean. Validate that there is no duplication of environment references or conflicting constants.packages/plugins/integration-github/src/lib/github/github-sync.service.ts (1)
6-6
: Good use of centralized tags.
The new import ofSyncTags
from@gauzy/constants
aligns well with the refactoring goal of consolidating constants.packages/core/src/lib/invite/invite.service.ts (3)
31-31
: Utility import looks correct.
The use ofgenerateAlphaNumericCode
andisNotEmpty
aligns with the new utility-based approach for code generation and validations.
173-173
: Verify code length requirements.
Switching from a fixed-length code generator togenerateAlphaNumericCode()
can impact existing flows if the length was previously a requirement. Confirm that this does not affect invitation acceptance or validation logic.
366-366
: Confirm consistency with previous code length.
Same consideration as line 173 above regarding the potential need for a specific code length.packages/plugins/integration-github-ui/src/lib/components/view/view.component.ts (3)
11-11
: Consistency with centralized constants.
ImportingSyncTags
from the constants package helps keep tag usage uniform across the codebase.
598-598
: Adoption of GAUZY sync tag.
UsingSyncTags.GAUZY
here aligns with the refactoring effort to maintain a single source of truth for tag references.
685-685
: Consistent sync tag usage.
Again, good consistency in referencingSyncTags.GAUZY
to avoid duplication of constants.packages/core/src/lib/auth/auth.service.ts (6)
17-17
: Demo environment constant usage.
TheDEMO_PASSWORD_LESS_MAGIC_CODE
can be helpful in local/demo setups. Double-check that it’s not enabled inadvertently in production environments to avoid security risks.
46-46
: New utility import adopted successfully.
ImportinggenerateAlphaNumericCode
from@gauzy/utils
ensures consistency with other modules, reducing custom code duplication.
57-57
: Use of freshTimestamp is acceptable.
No concerns here. Keeping timestamps consistent is good practice.
184-184
: Validate code length across user flows.
If users or other internal calls expect a fixed numeric code length, test the new approach thoroughly to confirm everything works.
309-309
: Repeated pattern warrants a check.
As with line 184, ensure no length constraints break any existing logic or expectations.
958-958
: Magic code length check.
Again, confirm that open-ended code generation doesn’t introduce collisions or break flows that expected a specific length.packages/utils/package.json (1)
30-30
: LGTM! Version is consistent across packages.The added dependency
@gauzy/constants
with version^0.1.0
aligns with the versions used in other packages.packages/common/package.json (1)
31-31
: LGTM! Dependencies are properly configured.The addition of
@gauzy/constants
is consistent with the refactoring to centralize constants.packages/config/package.json (1)
30-31
: LGTM! Dependencies are properly configured.The addition of both
@gauzy/constants
and@gauzy/common
is consistent with the refactoring strategy and maintains version consistency across packages.packages/core/src/lib/email-reset/email-reset.service.ts (1)
5-5
: Verify the alpha-numeric code length.The removal of
ALPHA_NUMERIC_CODE_LENGTH
parameter means we're now using the default length fromgenerateAlphaNumericCode()
. Please ensure this default length matches the previous requirements.Also applies to: 56-56
✅ Verification successful
Code length verification successful
The default length in
generateAlphaNumericCode()
uses the sameALPHA_NUMERIC_CODE_LENGTH
constant (6 characters) as before, so the code length remains unchanged.🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Search for previous usage of ALPHA_NUMERIC_CODE_LENGTH rg "ALPHA_NUMERIC_CODE_LENGTH" -A 2Length of output: 2746
packages/common/src/lib/guards/feature-flag-enabled.guard.ts (1)
1-3
: LGTM! Import path updated correctly.The import of
FEATURE_METADATA
from@gauzy/constants
aligns with the centralization of constants.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 2
🧹 Nitpick comments (1)
packages/core/src/lib/invite/invite.service.ts (1)
Line range hint
1-1000
: Consider architectural improvements for better maintainability.The service has grown large and handles multiple responsibilities. Consider:
- Splitting into smaller, focused services (e.g.,
InviteCreationService
,InviteValidationService
)- Implementing a more consistent error handling strategy
- Adding stronger typing for the invite workflows
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
packages/core/src/lib/core/utils.ts
(0 hunks)packages/core/src/lib/invite/invite.service.ts
(3 hunks)
💤 Files with no reviewable changes (1)
- packages/core/src/lib/core/utils.ts
⏰ Context from checks skipped due to timeout of 90000ms (3)
- GitHub Check: Codacy Static Code Analysis
- GitHub Check: build
- GitHub Check: test
🔇 Additional comments (1)
packages/core/src/lib/invite/invite.service.ts (1)
31-31
: LGTM! Import changes are well-organized.The imports are properly organized with external dependencies at the top followed by internal imports.
Also applies to: 34-34
View your CI Pipeline Execution ↗ for commit ba8db69.
☁️ Nx Cloud last updated this comment at |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
🔭 Outside diff range comments (1)
packages/plugins/integration-ai/src/lib/integration-ai.service.ts (1)
Null safety check for
integration.settings
is indeed necessaryThe review comment is valid. Other parts of the codebase, such as
task.update-or-create.handler.ts
, explicitly validateintegration.settings
before use. The suggested nullish coalescing operator would align with existing validation patterns and prevent potential runtime errors.
- The fix should be applied as suggested to maintain consistency with other integration services in the codebase.
🔗 Analysis chain
Line range hint
75-79
: Verify handling of empty or missingintegration.settings
arrayWhile
arrayToObject
is helpful, consider confirming thatintegration.settings
is never null or undefined. If the API or database layer can return empty or invalid settings, you may want to include a fallback or validation to avoid possible runtime errors.const { apiKey, apiSecret, openAiSecretKey, openAiOrganizationId } = arrayToObject( - integration.settings, + integration.settings ?? [], 'settingsName', 'settingsValue' );🏁 Scripts executed
The following scripts were executed for the analysis:
Error: No shell scripts found
Script:
#!/bin/bash # Search for arrayToObject implementation and usage ast-grep --pattern 'function arrayToObject' ast-grep --pattern 'const arrayToObject' # Search for integration.settings usage rg "integration\.settings" -A 2 -B 2 # Look for any validation patterns rg "integration\?.settings|integration\s+\?\s+settings"Length of output: 3311
Script:
#!/bin/bash # Search for arrayToObject in utility files and its imports/exports rg "arrayToObject" --type ts -B 2 -A 2 rg "from.*arrayToObject|export.*arrayToObject" --type tsLength of output: 10737
🧹 Nitpick comments (2)
packages/plugins/integration-upwork/src/lib/upwork.service.ts (1)
124-124
: Token presence check is good; consider validating values.
EnsuringaccessToken
andaccessTokenSecret
aren’t empty strings would enhance reliability.packages/plugins/integration-ai/src/lib/integration-ai.service.ts (1)
Line range hint
107-137
: Consider concurrency implications of sharedrequestConfigProvider
usageThis method temporarily sets a global-ish config for a single tenant and resets it afterward. If multiple tenants invoke updates concurrently, their API keys might override each other within the shared provider. Consider revisiting the approach to avoid potential race conditions—e.g., passing config objects directly to
_gauzyAIService.updateOneTenantApiKey
instead of storing them in_requestConfigProvider
.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (11)
apps/api/src/plugin-config.ts
(2 hunks)packages/plugins/integration-ai/src/lib/integration-ai.middleware.ts
(1 hunks)packages/plugins/integration-ai/src/lib/integration-ai.service.ts
(1 hunks)packages/plugins/integration-github/package.json
(1 hunks)packages/plugins/integration-github/src/lib/github/commands/handlers/github-installation.delete.handler.ts
(1 hunks)packages/plugins/integration-github/src/lib/github/commands/handlers/task.update-or-create.handler.ts
(1 hunks)packages/plugins/integration-github/src/lib/github/github-sync.service.ts
(3 hunks)packages/plugins/integration-github/src/lib/github/github.middleware.ts
(1 hunks)packages/plugins/integration-github/src/lib/github/github.service.ts
(2 hunks)packages/plugins/integration-upwork/src/lib/upwork.service.ts
(7 hunks)packages/ui-core/common/src/lib/utils/shared-utils.ts
(0 hunks)
💤 Files with no reviewable changes (1)
- packages/ui-core/common/src/lib/utils/shared-utils.ts
✅ Files skipped from review due to trivial changes (4)
- packages/plugins/integration-github/src/lib/github/github.middleware.ts
- packages/plugins/integration-github/src/lib/github/commands/handlers/github-installation.delete.handler.ts
- packages/plugins/integration-github/src/lib/github/commands/handlers/task.update-or-create.handler.ts
- packages/plugins/integration-ai/src/lib/integration-ai.middleware.ts
🚧 Files skipped from review as they are similar to previous changes (1)
- packages/plugins/integration-github/src/lib/github/github-sync.service.ts
⏰ Context from checks skipped due to timeout of 90000ms (4)
- GitHub Check: build
- GitHub Check: Codacy Static Code Analysis
- GitHub Check: Socket Security: Pull Request Alerts
- GitHub Check: test
🔇 Additional comments (13)
packages/plugins/integration-github/package.json (1)
31-31
: LGTM! The dependency addition aligns with the refactoring goals.The addition of
@gauzy/constants
dependency is consistent with other@gauzy/*
package versions and correctly placed in the dependencies section.Let's verify version consistency across all package.json files:
✅ Verification successful
✅ Version consistency verified - dependency pattern is correct
The
^0.1.0
version in the GitHub integration plugin is correct. Some apps use local file dependencies by design for development purposes, while plugins and libraries consistently use the versioned dependency.🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Check for version consistency of @gauzy/constants across all package.json files # Expected: All @gauzy/constants dependencies should use version ^0.1.0 echo "Checking @gauzy/constants version consistency..." fd --type f "package.json" --exec grep -l "@gauzy/constants" {} \; | \ xargs grep "\"@gauzy/constants\":" | \ grep -v "\"^0.1.0\"" # If no output, all versions are consistentLength of output: 517
packages/plugins/integration-github/src/lib/github/github.service.ts (3)
6-6
: Good consolidation underSyncTags
.
This import reorganizes constants into a coherent module, improving maintainability.
13-13
: No issues withIntegrationEnum
.
This import is consistent with the rest of the code and usage.
101-101
: ReplacedSYNC_TAG_GITHUB
withSyncTags.GITHUB
.
This aligns well with the new constants approach, ensuring consistency across the application.packages/plugins/integration-upwork/src/lib/upwork.service.ts (6)
39-40
: AddedITimeLog
andIIntegrationSetting
imports.
These inclusions strengthen type definitions and clarify the nature of data handled within the service.
42-42
: Import from@gauzy/core
looks fine.
No concerns with the updated import statements.
73-73
: Extended import from@gauzy/utils
.
UsingisObject
alongsideisEmpty
andisNotEmpty
properly centralizes utility checks.
111-111
: ExplicitIIntegrationSetting[]
type.
Declaring the array type of integration settings improves readability and type safety.
222-222
: ExplicitIIntegrationSetting[]
usage again.
This consistent type usage continues to improve clarity across the service.
281-281
: Further consistency withIIntegrationSetting[]
.
No issues noted. This is well-aligned with the prior changes.apps/api/src/plugin-config.ts (2)
59-59
: Appreciate the clearer comment.Specifically indicating production usage helps maintainers quickly understand deployment contexts. This small doc update can prevent confusion, especially when toggling between environments.
3-4
: Ensure the new imports remain compatible across packages.You've introduced
ApplicationPluginConfig
from@gauzy/common
and migrated the default configuration constants to@gauzy/constants
. Please verify thatApplicationPluginConfig
is still correctly exported from@gauzy/common
and confirm there are no conflicting definitions or naming overlaps after this refactor.✅ Verification successful
Imports are correctly structured and compatible across packages
The verification confirms:
ApplicationPluginConfig
is properly exported from@gauzy/common
- All default configuration constants are correctly exported from
@gauzy/constants
- No conflicting definitions or naming overlaps exist
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Search for conflicting or duplicated definitions of ApplicationPluginConfig. # Look for the definition or export of ApplicationPluginConfig within @gauzy/common rg -A 5 "export.*ApplicationPluginConfig" packages/common # Check if any references to ApplicationPluginConfig remain in constants or other packages rg -A 5 "ApplicationPluginConfig" packages/constants packages/*Length of output: 51479
Script:
#!/bin/bash # Check if the default configuration constants are properly exported from @gauzy/constants rg -A 5 "export.*DEFAULT_API_PORT|export.*DEFAULT_GRAPHQL_API_PATH|export.*DEFAULT_API_HOST|export.*DEFAULT_API_BASE_URL" packages/constantsLength of output: 498
packages/plugins/integration-ai/src/lib/integration-ai.service.ts (1)
10-10
: Added import ofarrayToObject
andisNotEmpty
The addition of
arrayToObject
from@gauzy/utils
is a welcome change, enabling concise and robust transformation from an array of settings to an object. IncludingisNotEmpty
also aligns well with existing checks and simplifies conditional logic.
PR
Please note: we will close your PR without comment if you do not check the boxes above and provide ALL requested information.
Summary by CodeRabbit
Release Notes
New Features
@gauzy/constants
package to centralize shared constants across the application.Improvements
Changes
Dependencies
@gauzy/constants
package as a dependency in multiple packages.