-
Notifications
You must be signed in to change notification settings - Fork 32
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
fix: minio connectivity #139
Conversation
WalkthroughThe recent changes predominantly enhance the file upload and configuration functionality within a NestJS project and its related packages. Major updates include the addition of configuration support using Changes
Poem
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 as PR comments)
Additionally, you can add CodeRabbit Configration 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: 2
Outside diff range and nitpick comments (4)
sample/06-file-upload/README.md (4)
Line range hint
9-9
: Correct verb form for 'setup'.The verb form "setup" should be corrected to "set up" in the context provided.
- how to setup and use the `FileUploadModule`. + how to set up and use the `FileUploadModule`.
Line range hint
19-19
: Grammar and punctuation corrections.Correct the grammatical error and add a comma for clarity as suggested by static analysis.
- This examples depends on minio as a S3 based storage engine to store the uploaded files. + This example depends on Minio as an S3-based storage engine to store the uploaded files, a `docker-compose.yml` file has been provided at the root of this project which sets up a Minio instance for your use.
Line range hint
35-35
: Correct verb form for 'setup'.Again, correct the verb form "setup" to "set up".
- To manually setup the `file-upload` functionality you need to register the `FileUploadModule` module in your module which can be imported from the `@samagra-x/stencil` package. + To manually set up the `file-upload` functionality, you need to register the `FileUploadModule` module in your module which can be imported from the `@samagra-x/stencil` package.
Line range hint
93-93
: Enhance formal tone and add a hyphen.Consider using a more formal alternative to 'amazing' and correct the compound adjective.
- support by the amazing backers. + support by the exceptional backers. - an MIT-licensed open source project. + an MIT-licensed open-source project.
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files ignored due to path filters (1)
packages/common/yarn.lock
is excluded by!**/yarn.lock
,!**/*.lock
Files selected for processing (7)
- .gitignore (1 hunks)
- docs/.gitignore (1 hunks)
- packages/common/package.json (2 hunks)
- packages/common/src/modules/file-upload.module.ts (1 hunks)
- packages/common/src/services/file-upload.service.ts (3 hunks)
- sample/06-file-upload/README.md (1 hunks)
- sample/06-file-upload/env-example (1 hunks)
Files skipped from review due to trivial changes (3)
- .gitignore
- docs/.gitignore
- packages/common/src/modules/file-upload.module.ts
Additional context used
LanguageTool
sample/06-file-upload/README.md
[grammar] ~9-~9: The word “setup” is a noun. The verb is spelled with a space.
Context: ... TypeScript sample app depicting how to setup and use theFileUploadModule
. ## In...(NOUN_VERB_CONFUSION)
[grammar] ~19-~19: Consider using the singular form after the singular determiner “This”.
Context: ... ``` ## Setup dependent services This examples depends on minio as a S3 based storage ...(AGREEMENT_SENT_START)
[uncategorized] ~19-~19: Possible missing comma found.
Context: ...e has been provided at the root of this project which sets up a minio instance for your...(AI_HYDRA_LEO_MISSING_COMMA)
[grammar] ~35-~35: The word “setup” is a noun. The verb is spelled with a white space.
Context: ...up file-upload > Manually To manually setup thefile-upload
functionality you nee...(NOUN_VERB_CONFUSION)
[uncategorized] ~51-~51: Possible missing comma found.
Context: ...ad` functionality automatically via the CLI simply run the following command: ```ba...(AI_HYDRA_LEO_MISSING_COMMA)
[uncategorized] ~93-~93: If this is a compound adjective that modifies the following noun, use a hyphen.
Context: ...knowledgements Nest is an MIT-licensed open source project. It can grow thanks to the spon...(EN_COMPOUND_ADJECTIVE_INTERNAL)
[style] ~93-~93: Consider using a more formal and expressive alternative to ‘amazing’.
Context: ...anks to the sponsors and support by the amazing backers. If you'd like to join them, pl...(AWESOME)
Markdownlint
sample/06-file-upload/README.md
37-37: Expected: 0 or 2; Actual: 1
Trailing spaces(MD009, no-trailing-spaces)
14-14: null
Dollar signs used before commands without showing output(MD014, commands-show-output)
22-22: null
Dollar signs used before commands without showing output(MD014, commands-show-output)
28-28: null
Dollar signs used before commands without showing output(MD014, commands-show-output)
53-53: null
Dollar signs used before commands without showing output(MD014, commands-show-output)
38-38: null
Fenced code blocks should be surrounded by blank lines(MD031, blanks-around-fences)
52-52: null
Fenced code blocks should be surrounded by blank lines(MD031, blanks-around-fences)
Biome
packages/common/src/services/file-upload.service.ts
[error] 24-26: Unnecessary use of boolean literals in conditional expression.
Simplify your code by directly assigning the result without using a ternary operator.
If your goal is negation, you may use the logical NOT (!) or double NOT (!!) operator for clearer and concise code.
Check for more details about NOT operator.
Unsafe fix: Remove the conditional expression with(lint/complexity/noUselessTernary)
[error] 115-125: This else clause can be omitted because previous branches break early.
Unsafe fix: Omit the else clause.
(lint/style/noUselessElse)
[error] 121-124: This else clause can be omitted because previous branches break early.
Unsafe fix: Omit the else clause.
(lint/style/noUselessElse)
Additional comments not posted (8)
sample/06-file-upload/env-example (3)
3-3
: Clarify the comment forSTORAGE_PORT
.The comment "please check your minio api port and update here" is helpful, but it might be more useful to provide a default or example port number for better clarity.
7-7
: Clarify the usage ofSTORAGE_MODE
.The comment "Do you want to use minio ? 'minio' : 'localhost' | ''" could be confusing. Consider rephrasing to "Set to 'minio' to use Minio, 'localhost' for local storage, or leave empty for default settings."
8-8
: Ensure clarity and security for SSL usage.It's good to see the addition of
CLIENT_USE_SSL
. However, consider adding a note on how to obtain and configure SSL certificates iftrue
is set.packages/common/package.json (2)
24-26
: Approve the updated script commands.The script commands have been changed to use Stencil-specific commands which align with the framework's usage.
38-38
: Approve the addition of@nestjs/config
.The addition of
@nestjs/config
at version^3.2.2
is appropriate for managing configuration values within the NestJS framework.packages/common/src/services/file-upload.service.ts (3)
11-11
: Approve the conditional initialization ofuseMinio
.The initialization of
useMinio
based on the environment variableSTORAGE_MODE
is a good practice for dynamic configuration.
94-94
: Clarify logging statements.The logging statement 'using minio' could be more descriptive to improve clarity and debugging.
- this.logger.log('using minio'); + this.logger.log('Using Minio for file storage.');
52-57
: Ensure URL generation is secure and correct.The URL generation logic should ensure that it is correctly formatted and secure. Consider verifying the entire URL generation process to ensure it adheres to best practices.
Verification successful
Ensure URL generation is secure and correct.
The URL generation logic in the
file-upload.service.ts
file appears to be correctly handling the environment variables and constructing the URL securely. The logic checks for SSL usage, constructs the protocol accordingly, and appends the endpoint, port, bucket name, and filename.
- The SSL usage is determined by
process.env.STORAGE_USE_SSL?.toLocaleLowerCase() === 'true'
.- The endpoint and port are fetched from
process.env.STORAGE_ENDPOINT
andprocess.env.STORAGE_PORT
.- The URL is constructed securely with the appropriate protocol and components.
The code snippet is as follows:
`${ process.env.STORAGE_USE_SSL?.toLocaleLowerCase() === 'true' ? 'https' : 'http' }://${process.env.STORAGE_ENDPOINT}:${process.env.STORAGE_PORT}/${ process.env.MINIO_BUCKETNAME }/${filename}`Recommendations:
Ensure that all environment variables (
STORAGE_USE_SSL
,STORAGE_ENDPOINT
,STORAGE_PORT
,MINIO_BUCKETNAME
) are properly validated and sanitized to avoid any potential security issues.Consider using a URL construction library to handle edge cases and ensure the URL is always correctly formatted.
Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify the URL generation logic in `uploadToMinio`. # Test: Check for correct URL format and security. echo "URL generation logic needs thorough review for correctness and security."Length of output: 154
Script:
#!/bin/bash # Locate and display the `uploadToMinio` method in `file-upload.service.ts` to analyze the URL generation logic. ast-grep --lang typescript --pattern 'function uploadToMinio($_) { $$$ }' packages/common/src/services/file-upload.service.tsLength of output: 127
Script:
#!/bin/bash # Search for the snippet provided in the review comment within `file-upload.service.ts` to locate the method containing the URL generation logic. rg --context 10 'process.env.STORAGE_USE_SSL?.toLocaleLowerCase() === '\''true'\''' packages/common/src/services/file-upload.service.tsLength of output: 137
Script:
#!/bin/bash # Search for the usage of `STORAGE_USE_SSL` environment variable across the entire codebase. rg 'process.env.STORAGE_USE_SSL?.toLocaleLowerCase() === '\''true'\'''Length of output: 72
Script:
#!/bin/bash # Search for the usage of `STORAGE_ENDPOINT` and `STORAGE_PORT` environment variables across the entire codebase. rg 'process.env.STORAGE_ENDPOINT' --context 10Length of output: 7612
MINIO_USERNAME=ROOTUSER | ||
MINIO_PASSWORD=CHANGEME123 |
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.
Address potential security risks with default credentials.
Using ROOTUSER
and CHANGEME123
as default credentials can pose a security risk if not changed. Recommend emphasizing the importance of changing these before deployment.
- MINIO_USERNAME=ROOTUSER
- MINIO_PASSWORD=CHANGEME123
+ MINIO_USERNAME=your-username # Change to a secure username
+ MINIO_PASSWORD=your-password # Change to a secure password
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.
MINIO_USERNAME=ROOTUSER | |
MINIO_PASSWORD=CHANGEME123 | |
MINIO_USERNAME=your-username # Change to a secure username | |
MINIO_PASSWORD=your-password # Change to a secure password |
|
||
switch (process.env.STORAGE_MODE?.toLowerCase()) { | ||
case STORAGE_MODE.MINIO: | ||
this.storage = new Client({ | ||
endPoint: process.env.STORAGE_ENDPOINT, | ||
port: parseInt(process.env.STORAGE_PORT), | ||
useSSL: this.useSSL, | ||
useSSL: | ||
process.env.CLIENT_USE_SSL.toLocaleLowerCase() === 'true' | ||
? true | ||
: false, |
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.
Simplify the conditional assignment of useSSL
.
The use of a ternary operator here is unnecessary. Simplify the code by directly assigning the boolean value.
- useSSL: process.env.CLIENT_USE_SSL.toLocaleLowerCase() === 'true' ? true : false,
+ useSSL: process.env.CLIENT_USE_SSL.toLocaleLowerCase() === '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.
switch (process.env.STORAGE_MODE?.toLowerCase()) { | |
case STORAGE_MODE.MINIO: | |
this.storage = new Client({ | |
endPoint: process.env.STORAGE_ENDPOINT, | |
port: parseInt(process.env.STORAGE_PORT), | |
useSSL: this.useSSL, | |
useSSL: | |
process.env.CLIENT_USE_SSL.toLocaleLowerCase() === 'true' | |
? true | |
: false, | |
switch (process.env.STORAGE_MODE?.toLowerCase()) { | |
case STORAGE_MODE.MINIO: | |
this.storage = new Client({ | |
endPoint: process.env.STORAGE_ENDPOINT, | |
port: parseInt(process.env.STORAGE_PORT), | |
useSSL: process.env.CLIENT_USE_SSL.toLocaleLowerCase() === 'true', |
Tools
Biome
[error] 24-26: Unnecessary use of boolean literals in conditional expression.
Simplify your code by directly assigning the result without using a ternary operator.
If your goal is negation, you may use the logical NOT (!) or double NOT (!!) operator for clearer and concise code.
Check for more details about NOT operator.
Unsafe fix: Remove the conditional expression with(lint/complexity/noUselessTernary)
@techsavvyash please review |
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: 3
Outside diff range and nitpick comments (1)
packages/common/src/services/file-upload.service.ts (1)
Line range hint
114-130
: Simplify the download function.Remove the unnecessary else clauses to simplify the code.
- if (this.useService) { - const fileStream = await this.storage.getObject( - this.configService.get<string>('STORAGE_CONTAINER_NAME'), - destination, - ); - return fileStream; - } else { - const localFilePath = path.join(process.cwd(), 'uploads', destination); // don't use __dirname here that'll point to the dist folder and not the top level folder containing the project (and the uploads folder) - if (fs.existsSync(localFilePath)) { - const fileStream = fs.createReadStream(localFilePath); - return fileStream; - } - else{ - this.logger.error(`Error downloading file: File does not exist`); - return null; - } - } + if (this.useService) { + const fileStream = await this.storage.getObject( + this.configService.get<string>('STORAGE_CONTAINER_NAME'), + destination, + ); + return fileStream; + } + const localFilePath = path.join(process.cwd(), 'uploads', destination); // don't use __dirname here that'll point to the dist folder and not the top level folder containing the project (and the uploads folder) + if (fs.existsSync(localFilePath)) { + const fileStream = fs.createReadStream(localFilePath); + return fileStream; + } + this.logger.error(`Error downloading file: File does not exist`); + return null;
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (4)
- packages/common/package.json (2 hunks)
- packages/common/src/services/file-upload.service.ts (4 hunks)
- sample/06-file-upload/package.json (1 hunks)
- sample/06-file-upload/uploads/ayush (1 hunks)
Files skipped from review due to trivial changes (2)
- sample/06-file-upload/package.json
- sample/06-file-upload/uploads/ayush
Files skipped from review as they are similar to previous changes (1)
- packages/common/package.json
Additional context used
Biome
packages/common/src/services/file-upload.service.ts
[error] 26-28: Unnecessary use of boolean literals in conditional expression.
Simplify your code by directly assigning the result without using a ternary operator.
If your goal is negation, you may use the logical NOT (!) or double NOT (!!) operator for clearer and concise code.
Check for more details about NOT operator.
Unsafe fix: Remove the conditional expression with(lint/complexity/noUselessTernary)
[error] 120-130: This else clause can be omitted because previous branches break early.
Unsafe fix: Omit the else clause.
(lint/style/noUselessElse)
[error] 126-129: This else clause can be omitted because previous branches break early.
Unsafe fix: Omit the else clause.
(lint/style/noUselessElse)
Additional comments not posted (2)
packages/common/src/services/file-upload.service.ts (2)
Line range hint
64-88
: LGTM!
Line range hint
98-106
: LGTM!
export class FileUploadService { | ||
private readonly storage: any; | ||
private readonly useMinio: boolean; | ||
private readonly useService: boolean = this.configService.get<string>('STORAGE_MODE')?.toLowerCase() === this.configService.get<string>('STORAGE_MODE.MINIO'); |
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.
Fix the useService
initialization.
The initialization of useService
is incorrect. It should compare the storage mode string directly.
- private readonly useService: boolean = this.configService.get<string>('STORAGE_MODE')?.toLowerCase() === this.configService.get<string>('STORAGE_MODE.MINIO');
+ private readonly useService: boolean = this.configService.get<string>('STORAGE_MODE')?.toLowerCase() === 'minio';
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.
private readonly useService: boolean = this.configService.get<string>('STORAGE_MODE')?.toLowerCase() === this.configService.get<string>('STORAGE_MODE.MINIO'); | |
private readonly useService: boolean = this.configService.get<string>('STORAGE_MODE')?.toLowerCase() === 'minio'; |
`${ | ||
this.configService.get('STORAGE_USE_SSL')?.toLocaleLowerCase() === 'true' | ||
? 'https' | ||
: 'http' | ||
}://${this.configService.get('STORAGE_ENDPOINT')}:${this.configService.get('STORAGE_PORT')}/${ |
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.
Simplify the URL construction.
The use of a ternary operator for constructing the URL can be simplified.
- `${
- this.configService.get('STORAGE_USE_SSL')?.toLocaleLowerCase() === 'true'
- ? 'https'
- : 'http'
- }://${this.configService.get('STORAGE_ENDPOINT')}:${this.configService.get('STORAGE_PORT')}/${
+ `${this.configService.get('STORAGE_USE_SSL')?.toLocaleLowerCase() === 'true' ? 'https' : 'http'}://${this.configService.get('STORAGE_ENDPOINT')}:${this.configService.get('STORAGE_PORT')}/${
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.
`${ | |
this.configService.get('STORAGE_USE_SSL')?.toLocaleLowerCase() === 'true' | |
? 'https' | |
: 'http' | |
}://${this.configService.get('STORAGE_ENDPOINT')}:${this.configService.get('STORAGE_PORT')}/${ | |
`${this.configService.get('STORAGE_USE_SSL')?.toLocaleLowerCase() === 'true' ? 'https' : 'http'}://${this.configService.get('STORAGE_ENDPOINT')}:${this.configService.get('STORAGE_PORT')}/${ |
this.configService.get<string>('CLIENT_USE_SSL').toLocaleLowerCase() === 'true' | ||
? true | ||
: false, |
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.
Simplify the conditional assignment of useSSL
.
The use of a ternary operator here is unnecessary. Simplify the code by directly assigning the boolean value.
- useSSL:
- this.configService.get<string>('CLIENT_USE_SSL').toLocaleLowerCase() === 'true'
- ? true
- : false,
+ useSSL: this.configService.get<string>('CLIENT_USE_SSL').toLocaleLowerCase() === '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.
this.configService.get<string>('CLIENT_USE_SSL').toLocaleLowerCase() === 'true' | |
? true | |
: false, | |
useSSL: this.configService.get<string>('CLIENT_USE_SSL').toLocaleLowerCase() === 'true', |
Tools
Biome
[error] 26-28: Unnecessary use of boolean literals in conditional expression.
Simplify your code by directly assigning the result without using a ternary operator.
If your goal is negation, you may use the logical NOT (!) or double NOT (!!) operator for clearer and concise code.
Check for more details about NOT operator.
Unsafe fix: Remove the conditional expression with(lint/complexity/noUselessTernary)
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
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (2)
- sample/06-file-upload/src/app.service.ts (1 hunks)
- sample/06-file-upload/test/app.e2e-spec.ts (2 hunks)
Files skipped from review due to trivial changes (1)
- sample/06-file-upload/src/app.service.ts
Additional comments not posted (3)
sample/06-file-upload/test/app.e2e-spec.ts (3)
5-6
: Remove unused import.The
readFileSync
import is currently commented out and unused.- import { readFileSync } from 'node:fs';
25-25
: LGTM!
36-36
: Remove commented-out code.The commented-out line for reading file content is unnecessary.
- //const fileContent = readFileSync(filePath);
it('/files/download (GET)', () => { | ||
return request(app.getHttpServer()) | ||
.get('/files/download/ayush') | ||
.expect(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.
Add assertion for file content.
The test case should include an assertion to verify the content of the downloaded file.
- .expect(File);
+ .expect(200)
+ .expect((res) => {
+ expect(res.text).toContain('expected file content');
+ });
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.
it('/files/download (GET)', () => { | |
return request(app.getHttpServer()) | |
.get('/files/download/ayush') | |
.expect(File); | |
}); | |
it('/files/download (GET)', () => { | |
return request(app.getHttpServer()) | |
.get('/files/download/ayush') | |
.expect(200) | |
.expect((res) => { | |
expect(res.text).toContain('expected file content'); | |
}); | |
}); |
|
GitGuardian id | GitGuardian status | Secret | Commit | Filename | |
---|---|---|---|---|---|
- | Generic High Entropy Secret | 4352846 | packages/common/test/file-upload/file-upload.service.spec.ts | View secret | |
- | Generic High Entropy Secret | 3d4d012 | packages/common/test/file-upload/file-upload.service.spec.ts | View secret |
🛠 Guidelines to remediate hardcoded secrets
- Understand the implications of revoking this secret by investigating where it is used in your code.
- Replace and store your secrets safely. Learn here the best practices.
- Revoke and rotate these secrets.
- If possible, rewrite git history. Rewriting git history is not a trivial act. You might completely break other contributing developers' workflow and you risk accidentally deleting legitimate data.
To avoid such incidents in the future consider
- following these best practices for managing and storing secrets including API keys and other credentials
- install secret detection on pre-commit to catch secret before it leaves your machine and ease remediation.
🦉 GitGuardian detects secrets in your source code to help developers and security teams secure the modern development process. You are seeing this because you or someone else with access to this repository has authorized GitGuardian to scan your pull request.
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: 5
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (5)
- packages/common/src/services/file-upload.service.ts (3 hunks)
- packages/common/test/file-upload/file-upload.service.spec.ts (4 hunks)
- sample/06-file-upload/package.json (2 hunks)
- sample/06-file-upload/src/app.controller.spec.ts (1 hunks)
- sample/06-file-upload/uploads/ayush (1 hunks)
Files skipped from review as they are similar to previous changes (2)
- sample/06-file-upload/package.json
- sample/06-file-upload/uploads/ayush
Additional context used
Gitleaks
packages/common/test/file-upload/file-upload.service.spec.ts
28-28: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.
(generic-api-key)
29-29: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.
(generic-api-key)
Biome
packages/common/src/services/file-upload.service.ts
[error] 88-91: This else clause can be omitted because previous branches break early.
Unsafe fix: Omit the else clause.
(lint/style/noUselessElse)
[error] 103-112: This else clause can be omitted because previous branches break early.
Unsafe fix: Omit the else clause.
(lint/style/noUselessElse)
[error] 108-111: This else clause can be omitted because previous branches break early.
Unsafe fix: Omit the else clause.
(lint/style/noUselessElse)
GitHub Check: CodeQL
packages/common/src/services/file-upload.service.ts
[failure] 106-106: Uncontrolled data used in path expression
This path depends on a user-provided value.
Additional comments not posted (3)
sample/06-file-upload/src/app.controller.spec.ts (1)
18-19
: LGTM!The test case has been correctly updated to reflect the new directory path in the greeting message.
packages/common/test/file-upload/file-upload.service.spec.ts (1)
22-34
: LGTM!The mock for
ConfigService
and its usage in the tests are correctly implemented.Tools
Gitleaks
28-28: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.
(generic-api-key)
29-29: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.
(generic-api-key)
packages/common/src/services/file-upload.service.ts (1)
58-58
: Simplify the URL construction.The use of a ternary operator for constructing the URL can be simplified.
- `${this.useSSL ? 'https' : 'http'}://${this.storageEndpoint}:${this.storagePort}/${this.bucketName}/${filename}`, + `${this.useSSL ? 'https' : 'http'}://${this.storageEndpoint}:${this.storagePort}/${this.bucketName}/${filename}`Likely invalid or redundant comment.
STORAGE_ACCESS_KEY: '5wmqDihWraT51LUgH2z1', | ||
STORAGE_SECRET_KEY: '4AzloXYR22h15zPFjVuSmwKaCGPyUZKRovkSzOJW', |
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.
Security Issue: Generic API Key Detected
The STORAGE_ACCESS_KEY
and STORAGE_SECRET_KEY
should not be hardcoded in the test configuration as it may expose sensitive information. Consider using environment variables or a secure vault for storing sensitive keys.
- STORAGE_ACCESS_KEY: '5wmqDihWraT51LUgH2z1',
- STORAGE_SECRET_KEY: '4AzloXYR22h15zPFjVuSmwKaCGPyUZKRovkSzOJW',
+ STORAGE_ACCESS_KEY: process.env.STORAGE_ACCESS_KEY,
+ STORAGE_SECRET_KEY: process.env.STORAGE_SECRET_KEY,
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.
STORAGE_ACCESS_KEY: '5wmqDihWraT51LUgH2z1', | |
STORAGE_SECRET_KEY: '4AzloXYR22h15zPFjVuSmwKaCGPyUZKRovkSzOJW', | |
STORAGE_ACCESS_KEY: process.env.STORAGE_ACCESS_KEY, | |
STORAGE_SECRET_KEY: process.env.STORAGE_SECRET_KEY, |
Tools
Gitleaks
28-28: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.
(generic-api-key)
29-29: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.
(generic-api-key)
accessKey: this.configService.get('STORAGE_ACCESS_KEY'), | ||
secretKey: this.configService.get('STORAGE_SECRET_KEY'), |
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.
Security Issue: Generic API Key Detected
The accessKey
and secretKey
should not be hardcoded in the configuration as it may expose sensitive information. Consider using environment variables or a secure vault for storing sensitive keys.
- accessKey: this.configService.get('STORAGE_ACCESS_KEY'),
- secretKey: this.configService.get('STORAGE_SECRET_KEY'),
+ accessKey: process.env.STORAGE_ACCESS_KEY,
+ secretKey: process.env.STORAGE_SECRET_KEY,
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.
accessKey: this.configService.get('STORAGE_ACCESS_KEY'), | |
secretKey: this.configService.get('STORAGE_SECRET_KEY'), | |
accessKey: process.env.STORAGE_ACCESS_KEY, | |
secretKey: process.env.STORAGE_SECRET_KEY, |
? false | ||
: process.env.STORAGE_USE_SSL?.toLocaleLowerCase() === 'true'; | ||
this.useService = this.configService.get<string>('STORAGE_MODE')?.toLowerCase() === STORAGE_MODE.MINIO; | ||
this.useSSL = this.configService.get<string>('STORAGE_USE_SSL') === 'true'; |
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.
Simplify the conditional assignment of useSSL
.
The use of a ternary operator here is unnecessary. Simplify the code by directly assigning the boolean value.
- this.useSSL = this.configService.get<string>('STORAGE_USE_SSL') === 'true';
+ this.useSSL = this.configService.get<string>('STORAGE_USE_SSL').toLocaleLowerCase() === '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.
this.useSSL = this.configService.get<string>('STORAGE_USE_SSL') === 'true'; | |
this.useSSL = this.configService.get<string>('STORAGE_USE_SSL').toLocaleLowerCase() === 'true'; |
} else { | ||
this.logger.log('Saving file locally'); | ||
return await this.saveLocalFile(destination, filename, 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.
Refactor: Omit unnecessary else clause.
This else clause can be omitted because the previous branch breaks early.
- } else {
- this.logger.log('Saving file locally');
- return await this.saveLocalFile(destination, filename, file);
- }
+ this.logger.log('Saving file locally');
+ return await this.saveLocalFile(destination, filename, file);
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.
} else { | |
this.logger.log('Saving file locally'); | |
return await this.saveLocalFile(destination, filename, file); | |
} | |
this.logger.log('Saving file locally'); | |
return await this.saveLocalFile(destination, filename, file); |
Tools
Biome
[error] 88-91: This else clause can be omitted because previous branches break early.
Unsafe fix: Omit the else clause.
(lint/style/noUselessElse)
} else { | ||
const localFilePath = path.join(process.cwd(), 'uploads', destination); // don't use __dirname here that'll point to the dist folder and not the top level folder containing the project (and the uploads folder) | ||
const localFilePath = path.join(process.cwd(), 'uploads', destination); | ||
if (fs.existsSync(localFilePath)) { | ||
const fileStream = fs.createReadStream(localFilePath); | ||
return fileStream; | ||
} | ||
else{ | ||
this.logger.error(`Error downloading file: File does not exist`); | ||
return null; | ||
const fileStream = fs.createReadStream(localFilePath); | ||
return fileStream; | ||
} else { | ||
this.logger.error('File does not exist'); | ||
throw new InternalServerErrorException('File does not exist'); | ||
} | ||
} |
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.
Refactor: Omit unnecessary else clause.
This else clause can be omitted because the previous branch breaks early.
- } else {
- const localFilePath = path.join(process.cwd(), 'uploads', destination);
- if (fs.existsSync(localFilePath)) {
- const fileStream = fs.createReadStream(localFilePath);
- return fileStream;
- } else {
- this.logger.error('File does not exist');
- throw new InternalServerErrorException('File does not exist');
- }
+ const localFilePath = path.join(process.cwd(), 'uploads', destination);
+ if (fs.existsSync(localFilePath)) {
+ const fileStream = fs.createReadStream(localFilePath);
+ return fileStream;
+ }
+ this.logger.error('File does not exist');
+ throw new InternalServerErrorException('File does not exist');
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.
} else { | |
const localFilePath = path.join(process.cwd(), 'uploads', destination); // don't use __dirname here that'll point to the dist folder and not the top level folder containing the project (and the uploads folder) | |
const localFilePath = path.join(process.cwd(), 'uploads', destination); | |
if (fs.existsSync(localFilePath)) { | |
const fileStream = fs.createReadStream(localFilePath); | |
return fileStream; | |
} | |
else{ | |
this.logger.error(`Error downloading file: File does not exist`); | |
return null; | |
const fileStream = fs.createReadStream(localFilePath); | |
return fileStream; | |
} else { | |
this.logger.error('File does not exist'); | |
throw new InternalServerErrorException('File does not exist'); | |
} | |
} | |
const localFilePath = path.join(process.cwd(), 'uploads', destination); | |
if (fs.existsSync(localFilePath)) { | |
const fileStream = fs.createReadStream(localFilePath); | |
return fileStream; | |
} | |
this.logger.error('File does not exist'); | |
throw new InternalServerErrorException('File does not exist'); |
Tools
Biome
[error] 103-112: This else clause can be omitted because previous branches break early.
Unsafe fix: Omit the else clause.
(lint/style/noUselessElse)
[error] 108-111: This else clause can be omitted because previous branches break early.
Unsafe fix: Omit the else clause.
(lint/style/noUselessElse)
GitHub Check: CodeQL
[failure] 106-106: Uncontrolled data used in path expression
This path depends on a user-provided value.
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: 0
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (1)
- packages/common/test/file-upload/file-upload.service.spec.ts (4 hunks)
Additional comments not posted (5)
packages/common/test/file-upload/file-upload.service.spec.ts (5)
6-6
: LGTM! ImportingConfigService
.The import is necessary for mocking
ConfigService
in the test setup.
22-34
: LGTM! MockingConfigService
.The mock setup correctly returns configuration values for the keys used in the service.
41-41
: LGTM! InstantiatingFileUploadService
withmockConfigService
.This change ensures that the service uses the mocked configuration values during tests.
82-82
: LGTM! Testing directory creation errors.The test correctly verifies the error handling and logging behavior of the service.
95-103
: LGTM! Testing file upload to Minio.The test correctly verifies the file upload functionality to Minio using the mocked configuration values.
* Update nest-cli.json * Update package.json * Update package.json * Update package.json * Update package.json * Update package.json
…amagraX-Stencil#117) * Fix/npm-scripts * readme:add-cmd-stencil-cli --------- Co-authored-by: Yash Mittal <mittal.yash12@hotmail.com>
…X-Stencil#107) * tests * response-time-test
* Create CONTRIBUTING.md * docs: add CONTRIBUTING.md --------- Co-authored-by: Yash Mittal <mittal.yash12@hotmail.com>
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: 7
Outside diff range and nitpick comments (14)
docs/docs/cli/introduction.md (1)
14-24
: Documentation looks good with minor grammatical issues.The added documentation is clear and provides useful information about the new commands. However, there are minor grammatical issues that can be fixed.
- In this CLI tool, several new commands have been added to expedite the manual setup of a server environment. Let's delve deeper into each of these commands and understand how they can benefit users: + In this CLI tool, several new commands have been added to expedite the manual setup of a server environment. Let's delve deeper into each of these commands and understand how they can benefit users: - 1. `service-prisma` : Creates a new `prisma.service.ts` and adds it as a provider to `app.module.ts` + 1. `service-prisma`: Creates a new `prisma.service.ts` and adds it as a provider to `app.module.ts` - 2. `prisma` : Adds a sample `User` model to the `schema.prisma` file + 2. `prisma`: Adds a sample `User` model to the `schema.prisma` file - 3. `service-user` : Adds the necessary imports for `@samagra-x/user-service` in `app.module.ts` + 3. `service-user`: Adds the necessary imports for `@samagra-x/user-service` in `app.module.ts` - 4. `fixtures` : Generate the docker related files along with the shell sciripts needed to run the `pre-commit` file in husky + 4. `fixtures`: Generate the docker related files along with the shell scripts needed to run the `pre-commit` file in husky - 5. `husky` : Creates the `pre-commit` file in the `.husky` folder + 5. `husky`: Creates the `pre-commit` file in the `.husky` folder - 6. `github` : Creates a new `.github` folder and the `ISSUE_TEMPLATE` and `Workflow` subdirectory in it. + 6. `github`: Creates a new `.github` folder and the `ISSUE_TEMPLATE` and `Workflow` subdirectory in it. - 7. `devcontainer` : Generates the `devcontainer.json` in the .devcontainer directory. + 7. `devcontainer`: Generates the `devcontainer.json` in the .devcontainer directory. - Overall, these commands enhance the usability of your CLI tool by automating repetitive and error-prone tasks, enabling developers to set up their server environments quickly and efficiently. By leveraging these custom schematics, developers can focus more on writing code and less on manual setup, ultimately improving productivity and code quality which is the prime goal for this setup. + Overall, these commands enhance the usability of your CLI tool by automating repetitive and error-prone tasks, enabling developers to set up their server environments quickly and efficiently. By leveraging these custom schematics, developers can focus more on writing code and less on manual setup, ultimately improving productivity and code quality, which is the prime goal for this setup.Tools
LanguageTool
[uncategorized] ~16-~16: Loose punctuation mark.
Context: ...can benefit users: 1.service-prisma
: Creates a newprisma.service.ts
and a...(UNLIKELY_OPENING_PUNCTUATION)
[uncategorized] ~22-~22: Loose punctuation mark.
Context: ...subdirectory in it. 7.
devcontainer: Generates the
devcontainer.json` in th...(UNLIKELY_OPENING_PUNCTUATION)
[uncategorized] ~24-~24: A comma might be missing here.
Context: ...imately improving productivity and code quality which is the prime goal for this setup....(AI_EN_LECTOR_MISSING_PUNCTUATION_COMMA)
docs/docs/cli/installation.md (1)
15-15
: Specify the language for the code block.Fenced code blocks should have a language specified for better syntax highlighting.
- ``` + ```bashREADME.md (4)
20-20
: Add a comma after "project".A comma might be missing here to improve readability.
- stencil new <PROJECT_NAME> + stencil new <PROJECT_NAME>,Tools
LanguageTool
[uncategorized] ~20-~20: A comma might be missing here.
Context: ...ncil new <PROJECT_NAME> ``` During the setup the CLI will prompt you to select all t...(AI_EN_LECTOR_MISSING_PUNCTUATION_COMMA)
[style] ~20-~20: ‘pick and choose’ might be wordy. Consider a shorter alternative.
Context: ...ll the toolings you want by letting you pick and choose between what you want to have in your s...(EN_WORDINESS_PREMIUM_PICK_AND_CHOOSE)
20-20
: Consider shortening "pick and choose".The phrase "pick and choose" might be wordy. Consider using a shorter alternative like "select".
- pick and choose + selectTools
LanguageTool
[uncategorized] ~20-~20: A comma might be missing here.
Context: ...ncil new <PROJECT_NAME> ``` During the setup the CLI will prompt you to select all t...(AI_EN_LECTOR_MISSING_PUNCTUATION_COMMA)
[style] ~20-~20: ‘pick and choose’ might be wordy. Consider a shorter alternative.
Context: ...ll the toolings you want by letting you pick and choose between what you want to have in your s...(EN_WORDINESS_PREMIUM_PICK_AND_CHOOSE)
38-38
: Add a preposition before "directory level READMEs".A preposition might be missing before "directory level READMEs".
- refer directory level READMEs + refer to directory level READMEsTools
LanguageTool
[style] ~38-~38: You have already used this phrasing in nearby sentences. Consider replacing it to add variety to your writing.
Context: ...ate to the required directory where you want to hack around and refer directory level R...(REP_WANT_TO_VB)
[uncategorized] ~38-~38: Possible missing preposition found.
Context: ...where you want to hack around and refer directory level READMEs to understand more about ...(AI_EN_LECTOR_MISSING_PREPOSITION)
54-54
: Consider using "Elasticsearch" instead of "Elastic Search".The word "Elastic Search" is normally spelled as one word, "Elasticsearch".
- Elastic Search + ElasticsearchTools
LanguageTool
[misspelling] ~54-~54: This word is normally spelled as one.
Context: ...) - Grafana - Elastic Search - [Logstash](...(EN_COMPOUNDS_ELASTIC_SEARCH)
CONTRIBUTING.md (1)
51-57
: Improve consistency in resource descriptions.Ensure consistency in the descriptions of resources.
- In case you're unfamiliar with the technology being used for the project and how to understand the project better this section will act as a repository to provide some base material around them to help you get started + In case you're unfamiliar with the technology being used for the project, this section will act as a repository to provide some base material to help you get started.sample/06-file-upload/README.md (1)
82-93
: Improve clarity in file upload steps.Make the instructions more concise and clear.
- Send a POST request in the given route: /files/upload-file + Send a POST request to the route: /files/upload-file - The destination and file name should be passed along with the post request as query parameters. + Pass the destination and file name as query parameters in the POST request. - The file should be attached in the request body (as form-data) with field-name(key) as 'file' + Attach the file in the request body (as form-data) with the field name 'file'. - The destination and filename fields can be set as per user's choice + The destination and filename fields can be set as per the user's choice.packages/common/test/interceptor/response-time.interceptor.spec.ts (3)
78-85
: Uncomment theobserve
method call for completeness.The test case is correctly verifying the response time logging, but the
observe
method call should be uncommented.- // expect(mockHistogram.observe).toHaveBeenCalledWith(100); + expect(mockHistogram.observe).toHaveBeenCalledWith(100);
87-98
: Uncomment theobserve
method call for completeness.The test case is correctly verifying the response time and error logging, but the
observe
method call should be uncommented.- // expect(mockHistogram.observe).toHaveBeenCalledWith(200); + expect(mockHistogram.observe).toHaveBeenCalledWith(200);
100-107
: Uncomment theobserve
method call for completeness.The test case is correctly verifying the handling of JSON parsing errors, but the
observe
method call should be uncommented.- // expect(mockHistogram.observe).toHaveBeenCalledWith(100); + expect(mockHistogram.observe).toHaveBeenCalledWith(100);docs/docs/monitoring/nestjs-monitor.md (1)
Line range hint
120-120
:
Fix grammatical issues and improve formality.
- Remove redundant phrases.
- Use more formal language.
- It can grow thanks to the sponsors and support by the amazing backers. + It can grow thanks to the sponsors and the support of the dedicated backers.Tools
LanguageTool
[style] ~30-~30: This phrase is redundant. Consider writing “points” or “times”.
Context: ...umber of requests received at different points in time. It is useful for identifying traffic t...(MOMENT_IN_TIME)
sample/05-temporal-package/README.md (2)
30-30
: Markdownlint: Trailing punctuation in headingRemove the trailing period in the heading.
- ## Run the below command from the root of the project to start a temporal instance locally to be used in this example. + ## Run the below command from the root of the project to start a temporal instance locally to be used in this example
133-133
: Markdownlint: Multiple spaces after hash on atx style headingRemove the extra spaces after the hash in the heading.
- ## Setup temporal package via Stencil [CLI](https://github.com/SamagraX-stencil/stencil-cli): + ## Setup temporal package via Stencil [CLI](https://github.com/SamagraX-stencil/stencil-cli):Tools
Markdownlint
133-133: null
Multiple spaces after hash on atx style heading(MD019, no-multiple-space-atx)
133-133: Punctuation: ':'
Trailing punctuation in heading(MD026, no-trailing-punctuation)
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files ignored due to path filters (5)
packages/common/package-lock.json
is excluded by!**/package-lock.json
packages/common/yarn.lock
is excluded by!**/yarn.lock
,!**/*.lock
packages/user-service/yarn.lock
is excluded by!**/yarn.lock
,!**/*.lock
sample/06-file-upload/yarn.lock
is excluded by!**/yarn.lock
,!**/*.lock
yarn.lock
is excluded by!**/yarn.lock
,!**/*.lock
Files selected for processing (32)
- .github/pull_request_template.md (1 hunks)
- .github/workflows/codeql.yml (1 hunks)
- .gitignore (1 hunks)
- CONTRIBUTING.md (1 hunks)
- README.md (2 hunks)
- docs/.gitignore (1 hunks)
- docs/docs/cli/installation.md (1 hunks)
- docs/docs/cli/introduction.md (1 hunks)
- docs/docs/monitoring/nestjs-monitor.md (1 hunks)
- packages/common/jest.config.js (1 hunks)
- packages/common/package.json (4 hunks)
- packages/common/src/interceptors/file-upload.interceptor.ts (1 hunks)
- packages/common/src/modules/file-upload.module.ts (1 hunks)
- packages/common/src/services/file-upload.service.ts (3 hunks)
- packages/common/test/file-upload.interceptor.spec.ts (1 hunks)
- packages/common/test/file-upload/file-upload.service.spec.ts (1 hunks)
- packages/common/test/interceptor/file-upload.interceptor.spec.ts (1 hunks)
- packages/common/test/interceptor/response-time.interceptor.spec.ts (1 hunks)
- packages/common/test/monitoring/monitoring.service.spec.ts (1 hunks)
- packages/common/test/temporal/temporal.service.spec.ts (1 hunks)
- sample/02-monitoring/package.json (1 hunks)
- sample/03-response-formatting/package.json (1 hunks)
- sample/04-logging/package.json (1 hunks)
- sample/05-temporal-package/README.md (2 hunks)
- sample/05-temporal-package/package.json (1 hunks)
- sample/06-file-upload/README.md (2 hunks)
- sample/06-file-upload/env-example (1 hunks)
- sample/06-file-upload/package.json (3 hunks)
- sample/06-file-upload/src/app.controller.spec.ts (1 hunks)
- sample/06-file-upload/src/app.service.ts (1 hunks)
- sample/06-file-upload/test/app.e2e-spec.ts (2 hunks)
- sample/06-file-upload/uploads/ayush (1 hunks)
Files skipped from review due to trivial changes (4)
- .github/pull_request_template.md
- packages/common/jest.config.js
- sample/02-monitoring/package.json
- sample/05-temporal-package/package.json
Files skipped from review as they are similar to previous changes (11)
- .gitignore
- docs/.gitignore
- packages/common/package.json
- packages/common/src/services/file-upload.service.ts
- packages/common/test/file-upload/file-upload.service.spec.ts
- sample/06-file-upload/env-example
- sample/06-file-upload/package.json
- sample/06-file-upload/src/app.controller.spec.ts
- sample/06-file-upload/src/app.service.ts
- sample/06-file-upload/test/app.e2e-spec.ts
- sample/06-file-upload/uploads/ayush
Additional context used
LanguageTool
docs/docs/cli/introduction.md
[uncategorized] ~16-~16: Loose punctuation mark.
Context: ...can benefit users: 1.service-prisma
: Creates a newprisma.service.ts
and a...(UNLIKELY_OPENING_PUNCTUATION)
[uncategorized] ~22-~22: Loose punctuation mark.
Context: ...subdirectory in it. 7.
devcontainer: Generates the
devcontainer.json` in th...(UNLIKELY_OPENING_PUNCTUATION)
[uncategorized] ~24-~24: A comma might be missing here.
Context: ...imately improving productivity and code quality which is the prime goal for this setup....(AI_EN_LECTOR_MISSING_PUNCTUATION_COMMA)
README.md
[uncategorized] ~20-~20: A comma might be missing here.
Context: ...ncil new <PROJECT_NAME> ``` During the setup the CLI will prompt you to select all t...(AI_EN_LECTOR_MISSING_PUNCTUATION_COMMA)
[style] ~20-~20: ‘pick and choose’ might be wordy. Consider a shorter alternative.
Context: ...ll the toolings you want by letting you pick and choose between what you want to have in your s...(EN_WORDINESS_PREMIUM_PICK_AND_CHOOSE)
[style] ~38-~38: You have already used this phrasing in nearby sentences. Consider replacing it to add variety to your writing.
Context: ...ate to the required directory where you want to hack around and refer directory level R...(REP_WANT_TO_VB)
[uncategorized] ~38-~38: Possible missing preposition found.
Context: ...where you want to hack around and refer directory level READMEs to understand more about ...(AI_EN_LECTOR_MISSING_PREPOSITION)
[uncategorized] ~44-~44: You might be missing the article “a” here.
Context: ...tencil-docs.vercel.app/) **If you have request for a specific tool to be setup automat...(AI_EN_LECTOR_MISSING_DETERMINER_A)
[grammar] ~44-~44: Make sure that the noun ‘setup’ is correct. Did you mean the past participle “set up”?
Context: ... have request for a specific tool to be setup automatically, please open a issue tick...(BE_VB_OR_NN)
[misspelling] ~44-~44: Use “an” instead of ‘a’ if the following word starts with a vowel sound, e.g. ‘an article’, ‘an hour’.
Context: ... to be setup automatically, please open a issue ticket and we'll try to get it ad...(EN_A_VS_AN)
[uncategorized] ~44-~44: Use a comma before ‘and’ if it connects two independent clauses (unless they are closely connected and short).
Context: ...utomatically, please open a issue ticket and we'll try to get it added at the earlie...(COMMA_COMPOUND_SENTENCE)
[misspelling] ~54-~54: This word is normally spelled as one.
Context: ...) - Grafana - Elastic Search - [Logstash](...(EN_COMPOUNDS_ELASTIC_SEARCH)
CONTRIBUTING.md
[grammar] ~23-~23: The word “setup” is a noun. The verb is spelled with a space.
Context: ...vidual functionalities and how they can setup or migrated in your nest application vi...(NOUN_VERB_CONFUSION)
[misspelling] ~23-~23: Did you mean “next”?
Context: ... how they can setup or migrated in your nest application via stencil. - `docker-comp...(CONFUSION_OF_PRPS_NEST_NEXT)
[uncategorized] ~30-~30: This adjective is normally spelled with a hyphen.
Context: ...erence. ## Ways to Contribute - As a first time contributor, navigating through the pro...(TIME_HYPHEN)
[misspelling] ~32-~32: Use “an” instead of ‘a’ if the following word starts with a vowel sound, e.g. ‘an article’, ‘an hour’.
Context: ... understand the project better. - Open a issue ticket for any gap in documentati...(EN_A_VS_AN)
[uncategorized] ~36-~36: Possible missing comma found.
Context: ...ors out. - Lastly and the most obvious one feel free to pick up open issue tickets...(AI_HYDRA_LEO_MISSING_COMMA)
[uncategorized] ~39-~39: Possible missing comma found.
Context: ... ## Community Issue Assignment Policy Ideally all the tickets are open for the open s...(AI_HYDRA_LEO_MISSING_COMMA)
[misspelling] ~40-~40: Use “an” instead of ‘a’ if the following word starts with a vowel sound, e.g. ‘an article’, ‘an hour’.
Context: ...to contribute to, but the assignment of a issue comes only after the contributor ...(EN_A_VS_AN)
sample/06-file-upload/README.md
[uncategorized] ~120-~120: If this is a compound adjective that modifies the following noun, use a hyphen.
Context: ...knowledgements Nest is an MIT-licensed open source project. It can grow thanks to the spon...(EN_COMPOUND_ADJECTIVE_INTERNAL)
[style] ~120-~120: Consider using a more formal and expressive alternative to ‘amazing’.
Context: ...anks to the sponsors and support by the amazing backers. If you'd like to join them, pl...(AWESOME)
docs/docs/monitoring/nestjs-monitor.md
[style] ~30-~30: This phrase is redundant. Consider writing “points” or “times”.
Context: ...umber of requests received at different points in time. It is useful for identifying traffic t...(MOMENT_IN_TIME)
sample/05-temporal-package/README.md
[grammar] ~19-~19: Consider using the singular form after the singular determiner “This”.
Context: ... ``` ## Setup dependent services This examples requires a running temporal server to w...(AGREEMENT_SENT_START)
[uncategorized] ~19-~19: Possible missing comma found.
Context: ...s been provided in thedocker-compose
folder which has been taking from the official...(AI_HYDRA_LEO_MISSING_COMMA)
Markdownlint
docs/docs/cli/installation.md
14-14: null
Fenced code blocks should have a language specified(MD040, fenced-code-language)
CONTRIBUTING.md
22-22: Expected: asterisk; Actual: dash
Unordered list style(MD004, ul-style)
23-23: Expected: asterisk; Actual: dash
Unordered list style(MD004, ul-style)
24-24: Expected: asterisk; Actual: dash
Unordered list style(MD004, ul-style)
25-25: Expected: asterisk; Actual: dash
Unordered list style(MD004, ul-style)
30-30: Expected: asterisk; Actual: dash
Unordered list style(MD004, ul-style)
32-32: Expected: asterisk; Actual: dash
Unordered list style(MD004, ul-style)
34-34: Expected: asterisk; Actual: dash
Unordered list style(MD004, ul-style)
36-36: Expected: asterisk; Actual: dash
Unordered list style(MD004, ul-style)
44-44: Expected: asterisk; Actual: dash
Unordered list style(MD004, ul-style)
46-46: Expected: asterisk; Actual: dash
Unordered list style(MD004, ul-style)
9-9: null
Fenced code blocks should have a language specified(MD040, fenced-code-language)
3-3: null
Link fragments should be valid(MD051, link-fragments)
4-4: null
Link fragments should be valid(MD051, link-fragments)
5-5: null
Link fragments should be valid(MD051, link-fragments)
sample/05-temporal-package/README.md
32-32: Punctuation: '.'
Trailing punctuation in heading(MD026, no-trailing-punctuation)
133-133: null
Multiple spaces after hash on atx style heading(MD019, no-multiple-space-atx)
133-133: Punctuation: ':'
Trailing punctuation in heading(MD026, no-trailing-punctuation)
Additional comments not posted (33)
packages/common/src/modules/file-upload.module.ts (2)
4-4
: Import looks good.The import of
ConfigModule
from@nestjs/config
is necessary for the configuration module being added in the imports array.
6-10
: Configuration module addition looks good.Adding
ConfigModule.forRoot({ isGlobal: true })
to the imports array makes the configuration settings globally accessible, which is a recommended practice.packages/common/src/interceptors/file-upload.interceptor.ts (1)
56-56
: Closing brace looks good.The closing brace is necessary to properly close the function definition.
.github/workflows/codeql.yml (1)
1-59
: CodeQL workflow looks good.The added CodeQL workflow is well-defined and follows best practices for setting up CodeQL analysis.
packages/common/test/interceptor/file-upload.interceptor.spec.ts (1)
1-57
: Tests forFastifyFileInterceptor
look good!The added tests cover the functionality well, including error handling.
sample/04-logging/package.json (1)
9-13
: Script command updates look good!The changes to update the script commands from NestJS to Stencil are appropriate.
sample/03-response-formatting/package.json (1)
9-13
: Update script commands to Stencil.The script commands have been updated from NestJS to Stencil commands:
"build": "stencil build"
"start": "stencil start"
"start:dev": "stencil start --watch"
"start:debug": "stencil start --debug --watch"
These changes align with the PR objective and look good.
packages/common/test/temporal/temporal.service.spec.ts (5)
1-5
: Add imports and mockWorkflowClient
.The imports and mocking of
WorkflowClient
are correctly set up:
import { TemporalWorkflowService } from '../../src/services/temporal.service';
import { WorkflowClient } from '@temporalio/client';
jest.mock('@temporalio/client');
These changes are necessary for the unit tests.
10-22
: Initialize and inject mockedWorkflowClient
.The
beforeEach
setup correctly initializes and injects the mockedWorkflowClient
into theTemporalWorkflowService
:
mockWorkflowClient = new WorkflowClient() as jest.Mocked<WorkflowClient>;
service = new TemporalWorkflowService();
service['workflowClient'] = mockWorkflowClient;
This setup is essential for testing the service methods.
24-26
: Clear mocks after each test.The
afterEach
block correctly clears all mocks to ensure test isolation:
jest.clearAllMocks();
This is a good practice for unit tests.
28-30
: Test service definition.The
it('should be defined')
test ensures that the service is defined:
expect(service).toBeDefined();
This is a basic but necessary test.
32-48
: TeststartWorkflow
method.The tests for the
startWorkflow
method cover various scenarios:
- Starting a workflow and returning the result.
- Using default values if optional parameters are not provided.
- Handling workflow client errors.
The tests are comprehensive and well-structured.
packages/common/test/file-upload.interceptor.spec.ts (8)
1-5
: Add imports and describeFastifyFileInterceptor
.The imports and description for
FastifyFileInterceptor
are correctly set up:
import { ExecutionContext, NestInterceptor } from '@nestjs/common';
import { FastifyFileInterceptor } from '../src/interceptors/file-upload.interceptor';
These changes are necessary for the unit tests.
8-11
: Initialize interceptor.The
beforeEach
setup correctly initializes theFastifyFileInterceptor
:
const interceptorClass = FastifyFileInterceptor('file', {});
interceptor = new interceptorClass();
This setup is essential for testing the interceptor methods.
13-15
: Test interceptor definition.The
it('should be defined')
test ensures that the interceptor is defined:
expect(interceptor).toBeDefined();
This is a basic but necessary test.
17-26
: Test file upload handling.The test for handling file upload verifies that the file is correctly processed and added to the request:
expect(context.switchToHttp().getRequest().file).toEqual(file);
expect(nextHandler.handle).toHaveBeenCalled();
This test is comprehensive and well-structured.
30-44
: Test error handling.The test for handling errors verifies that the interceptor correctly throws an error if the file upload fails:
jest.spyOn(interceptor['multer'], 'single').mockImplementation(() => { callback(new Error(errorMessage)); });
await expect(interceptor.intercept(context, nextHandler)).rejects.toThrow(errorMessage);
This test is comprehensive and well-structured.
47-59
: Test handling of missing or null file.The test for handling cases where the file is missing or null verifies that the interceptor correctly processes these scenarios:
expect(contextWithUndefinedFile.switchToHttp().getRequest().file).toBeUndefined();
expect(contextWithNullFile.switchToHttp().getRequest().file).toBeNull();
expect(nextHandler.handle).toHaveBeenCalled();
This test is comprehensive and well-structured.
63-69
: Create mock context.The
createMockContext
function correctly creates a mock execution context:
const mockHttpContext = { getRequest: jest.fn().mockReturnValue({ raw: { headers: { 'content-type': 'multipart/form-data' } }, file }), getResponse: jest.fn().mockReturnValue({}) };
This function is necessary for the unit tests.
71-73
: Create mock next handler.The
createMockNextHandler
function correctly creates a mock next handler:
return { handle: jest.fn().mockReturnValue(response) };
This function is necessary for the unit tests.
packages/common/test/monitoring/monitoring.service.spec.ts (8)
1-4
: Add imports and describeMonitoringService
.The imports and description for
MonitoringService
are correctly set up:
import { MonitoringService } from '../../src/monitoring/monitoring.service';
import { CACHE_MANAGER } from '@nestjs/cache-manager';
import { Counter, register } from 'prom-client';
These changes are necessary for the unit tests.
10-41
: Initialize and inject mocks.The
beforeEach
setup correctly initializes and injects the mocks forCACHE_MANAGER
andCounter
:
register.clear();
redisClientMock = { incr: jest.fn() };
cacheManagerMock = { get: jest.fn().mockResolvedValue('0'), set: jest.fn(), del: jest.fn(), reset: jest.fn(), wrap: jest.fn(), keys: jest.fn(), ttl: jest.fn(), store: { getClient: jest.fn().mockReturnValue(redisClientMock) } };
const counterMock = { inc: jest.fn(), get: jest.fn().mockReturnValue({ values: [{ value: 0 }] }) };
service = new MonitoringService(cacheManagerMock);
(service as any).requestCounter = counterMock;
This setup is essential for testing the service methods.
43-45
: Clear mocks after each test.The
afterEach
block correctly clears all mocks to ensure test isolation:
jest.clearAllMocks();
This is a good practice for unit tests.
47-49
: Test service definition.The
it('should be defined')
test ensures that the service is defined:
expect(service).toBeDefined();
This is a basic but necessary test.
51-57
: TestinitializeAsync
method.The test for the
initializeAsync
method verifies that the counter is initialized with the cached value:
await service.initializeAsync();
expect(cacheManagerMock.get).toHaveBeenCalledWith('requestCount');
expect((service as any).requestCounter.inc).toHaveBeenCalledWith(0);
This test is comprehensive and well-structured.
60-66
: TestgetRequestCounter
method.The test for the
getRequestCounter
method verifies that the cached request count is returned:
const count = await service.getRequestCounter();
expect(cacheManagerMock.get).toHaveBeenCalledWith('requestCount');
expect(count).toBe('0');
This test is comprehensive and well-structured.
69-76
: TestincrementRequestCounter
method.The test for the
incrementRequestCounter
method verifies that the request counter is incremented and the cache is updated:
await service.incrementRequestCounter();
expect((service as any).requestCounter.inc).toHaveBeenCalled();
expect(cacheManagerMock.store.getClient).toHaveBeenCalled();
expect(redisClientMock.incr).toHaveBeenCalledWith('requestCount');
This test is comprehensive and well-structured.
79-96
: TestonExit
method.The tests for the
onExit
method cover various scenarios:
- Incrementing the request counter on exit.
- Logging an error if
incrementRequestCounter
fails.The tests are comprehensive and well-structured.
packages/common/test/interceptor/response-time.interceptor.spec.ts (2)
9-31
: Mock implementations are correctly set up.The mock implementations for
prom-client
,fs
, andperf_hooks
are correctly set up to simulate the behavior of the respective modules.
74-76
: Test caseshould be defined
is correctly verifying the existence of the interceptor instance.The test case ensures that the interceptor instance is defined.
docs/docs/monitoring/nestjs-monitor.md (2)
15-32
: Features section is well-written and clear.The features are well-explained and provide a comprehensive overview of the monitoring capabilities.
Tools
LanguageTool
[style] ~30-~30: This phrase is redundant. Consider writing “points” or “times”.
Context: ...umber of requests received at different points in time. It is useful for identifying traffic t...(MOMENT_IN_TIME)
38-42
: Demo section is clear and informative.The demo link and description provide valuable context for users.
sample/05-temporal-package/README.md (1)
21-21
: Typo: Singular form after singular determinerConsider using the singular form "example" after the singular determiner "This".
- This examples requires a running temporal server to work. + This example requires a running temporal server to work.Likely invalid or redundant comment.
|
||
```bash | ||
cd docker-compose && docker-compose up -d | ||
cd docker-compose |
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.
Typo: Missing comma
Consider adding a comma for clarity.
- folder which has been taking from the official docker-compose repository from temporal's github.
+ folder, which has been taken from the official docker-compose repository from temporal's GitHub.
Committable suggestion was skipped due to low confidence.
You can refer to the technical documentation which lists all the available functionality [here](https://stencil-docs.vercel.app/) | ||
|
||
**If you have request for a specific tool to be setup automatically, please open a issue ticket and we'll try to get it added at the earliest.** |
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.
Fix grammatical issues in the sentence.
There are multiple grammatical issues in this sentence. Correct the use of articles and verb forms.
- If you have request for a specific tool to be setup automatically, please open a issue ticket and we'll try to get it added at the earliest.
+ If you have a request for a specific tool to be set up automatically, please open an issue ticket, and we'll try to get it added at the earliest.
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.
**If you have request for a specific tool to be setup automatically, please open a issue ticket and we'll try to get it added at the earliest.** | |
**If you have a request for a specific tool to be set up automatically, please open an issue ticket, and we'll try to get it added at the earliest.** |
Tools
LanguageTool
[uncategorized] ~44-~44: You might be missing the article “a” here.
Context: ...tencil-docs.vercel.app/) **If you have request for a specific tool to be setup automat...(AI_EN_LECTOR_MISSING_DETERMINER_A)
[grammar] ~44-~44: Make sure that the noun ‘setup’ is correct. Did you mean the past participle “set up”?
Context: ... have request for a specific tool to be setup automatically, please open a issue tick...(BE_VB_OR_NN)
[misspelling] ~44-~44: Use “an” instead of ‘a’ if the following word starts with a vowel sound, e.g. ‘an article’, ‘an hour’.
Context: ... to be setup automatically, please open a issue ticket and we'll try to get it ad...(EN_A_VS_AN)
[uncategorized] ~44-~44: Use a comma before ‘and’ if it connects two independent clauses (unless they are closely connected and short).
Context: ...utomatically, please open a issue ticket and we'll try to get it added at the earlie...(COMMA_COMPOUND_SENTENCE)
|
||
Ideally all the tickets are open for the open source and C4GT Community to contribute to, but the assignment of a issue comes only after the contributor has raised a draft PR implementing the partial functionality required by the issue ticket. |
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.
Fix grammatical errors.
- Use "an" instead of "a" before vowel sounds.
- Add missing comma.
- Ideally all the tickets are open for the open source and C4GT Community to contribute to, but the assignment of a issue comes only after the contributor has raised a draft PR implementing the partial functionality required by the issue ticket.
+ Ideally, all the tickets are open for the open-source and C4GT Community to contribute to, but the assignment of an issue comes only after the contributor has raised a draft PR implementing the partial functionality required by the issue ticket.
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.
Ideally all the tickets are open for the open source and C4GT Community to contribute to, but the assignment of a issue comes only after the contributor has raised a draft PR implementing the partial functionality required by the issue ticket. | |
Ideally, all the tickets are open for the open-source and C4GT Community to contribute to, but the assignment of an issue comes only after the contributor has raised a draft PR implementing the partial functionality required by the issue ticket. |
Tools
LanguageTool
[uncategorized] ~39-~39: Possible missing comma found.
Context: ... ## Community Issue Assignment Policy Ideally all the tickets are open for the open s...(AI_HYDRA_LEO_MISSING_COMMA)
[misspelling] ~40-~40: Use “an” instead of ‘a’ if the following word starts with a vowel sound, e.g. ‘an article’, ‘an hour’.
Context: ...to contribute to, but the assignment of a issue comes only after the contributor ...(EN_A_VS_AN)
- `packages` directory is where the main code resides, it is where all the custom components published and provided by `@samagra-x/stencil` package can be found. All the featuers that you code will be implemented here. | ||
- `sample` directory contains reference applications showcasing individual functionalities and how they can setup or migrated in your nest application via stencil. | ||
- `docker-compose` directory contains the commonly used docker-compose setups which we find handy. | ||
- `docs` this folder contains the documentation for the framework. This is a docusaurus app which can be started for easy reference. |
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.
Fix grammatical errors and unordered list style.
- The word "setup" should be "set up" (verb form).
- The word "nest" should be "next".
- Use asterisks for unordered lists for consistency.
- - `sample` directory contains reference applications showcasing individual functionalities and how they can setup or migrated in your nest application via stencil.
+ * `sample` directory contains reference applications showcasing individual functionalities and how they can be set up or migrated in your next application via stencil.
- - `docker-compose` directory contains the commonly used docker-compose setups which we find handy.
+ * `docker-compose` directory contains the commonly used docker-compose setups which we find handy.
- - `docs` this folder contains the documentation for the framework. This is a docusaurus app which can be started for easy reference.
+ * `docs` this folder contains the documentation for the framework. This is a docusaurus app which can be started for easy reference.
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.
- `packages` directory is where the main code resides, it is where all the custom components published and provided by `@samagra-x/stencil` package can be found. All the featuers that you code will be implemented here. | |
- `sample` directory contains reference applications showcasing individual functionalities and how they can setup or migrated in your nest application via stencil. | |
- `docker-compose` directory contains the commonly used docker-compose setups which we find handy. | |
- `docs` this folder contains the documentation for the framework. This is a docusaurus app which can be started for easy reference. | |
* `packages` directory is where the main code resides, it is where all the custom components published and provided by `@samagra-x/stencil` package can be found. All the featuers that you code will be implemented here. | |
* `sample` directory contains reference applications showcasing individual functionalities and how they can be set up or migrated in your next application via stencil. | |
* `docker-compose` directory contains the commonly used docker-compose setups which we find handy. | |
* `docs` this folder contains the documentation for the framework. This is a docusaurus app which can be started for easy reference. |
Tools
LanguageTool
[grammar] ~23-~23: The word “setup” is a noun. The verb is spelled with a space.
Context: ...vidual functionalities and how they can setup or migrated in your nest application vi...(NOUN_VERB_CONFUSION)
[misspelling] ~23-~23: Did you mean “next”?
Context: ... how they can setup or migrated in your nest application via stencil. - `docker-comp...(CONFUSION_OF_PRPS_NEST_NEXT)
Markdownlint
22-22: Expected: asterisk; Actual: dash
Unordered list style(MD004, ul-style)
23-23: Expected: asterisk; Actual: dash
Unordered list style(MD004, ul-style)
24-24: Expected: asterisk; Actual: dash
Unordered list style(MD004, ul-style)
25-25: Expected: asterisk; Actual: dash
Unordered list style(MD004, ul-style)
- As a first time contributor, navigating through the project, it can be quite daunting for you. Please make sure to go through the [repository structure](#repository-structure) to understand the project a bit better, if you want to understand the project better. | ||
|
||
- Open a issue ticket for any gap in documentation or bug or feature requests that you have, opening up issue tickets is always a good way to contribute. | ||
|
||
- You can help review open PRs for best backend and devops realted code practices and help the contributors out. | ||
|
||
- Lastly and the most obvious one feel free to pick up open issue tickets and raise a PR for it resolving the bug, feature or documentation request. |
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.
Fix grammatical errors.
- Add hyphen for "first-time".
- Use "an" instead of "a" before vowel sounds.
- Add missing comma.
- - As a first time contributor, navigating through the project, it can be quite daunting for you. Please make sure to go through the [repository structure](#repository-structure) to understand the project a bit better, if you want to understand the project better.
+ * As a first-time contributor, navigating through the project, it can be quite daunting for you. Please make sure to go through the [repository structure](#repository-structure) to understand the project a bit better.
- - Open a issue ticket for any gap in documentation or bug or feature requests that you have, opening up issue tickets is always a good way to contribute.
+ * Open an issue ticket for any gap in documentation or bug or feature requests that you have. Opening up issue tickets is always a good way to contribute.
- - Lastly and the most obvious one feel free to pick up open issue tickets and raise a PR for it resolving the bug, feature or documentation request.
+ * Lastly, and the most obvious one, feel free to pick up open issue tickets and raise a PR for it resolving the bug, feature, or documentation request.
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.
- As a first time contributor, navigating through the project, it can be quite daunting for you. Please make sure to go through the [repository structure](#repository-structure) to understand the project a bit better, if you want to understand the project better. | |
- Open a issue ticket for any gap in documentation or bug or feature requests that you have, opening up issue tickets is always a good way to contribute. | |
- You can help review open PRs for best backend and devops realted code practices and help the contributors out. | |
- Lastly and the most obvious one feel free to pick up open issue tickets and raise a PR for it resolving the bug, feature or documentation request. | |
* As a first-time contributor, navigating through the project, it can be quite daunting for you. Please make sure to go through the [repository structure](#repository-structure) to understand the project a bit better. | |
* Open an issue ticket for any gap in documentation or bug or feature requests that you have. Opening up issue tickets is always a good way to contribute. | |
* You can help review open PRs for best backend and devops related code practices and help the contributors out. | |
* Lastly, and the most obvious one, feel free to pick up open issue tickets and raise a PR for it resolving the bug, feature, or documentation request. |
Tools
LanguageTool
[uncategorized] ~30-~30: This adjective is normally spelled with a hyphen.
Context: ...erence. ## Ways to Contribute - As a first time contributor, navigating through the pro...(TIME_HYPHEN)
[misspelling] ~32-~32: Use “an” instead of ‘a’ if the following word starts with a vowel sound, e.g. ‘an article’, ‘an hour’.
Context: ... understand the project better. - Open a issue ticket for any gap in documentati...(EN_A_VS_AN)
[uncategorized] ~36-~36: Possible missing comma found.
Context: ...ors out. - Lastly and the most obvious one feel free to pick up open issue tickets...(AI_HYDRA_LEO_MISSING_COMMA)
Markdownlint
30-30: Expected: asterisk; Actual: dash
Unordered list style(MD004, ul-style)
32-32: Expected: asterisk; Actual: dash
Unordered list style(MD004, ul-style)
34-34: Expected: asterisk; Actual: dash
Unordered list style(MD004, ul-style)
36-36: Expected: asterisk; Actual: dash
Unordered list style(MD004, ul-style)
@@ -73,12 +73,39 @@ $ yarn run start:prod | |||
$ yarn run test | |||
|
|||
# e2e tests | |||
$ yarn run test:e2e | |||
$ yarn run test:e2e, |
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.
Fix syntax error in test:e2e
command.
Remove the trailing comma in the test:e2e
command.
- $ yarn run test:e2e,
+ $ yarn run test:e2e
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.
$ yarn run test:e2e, | |
$ yarn run test:e2e |
## Steps To Download File | ||
|
||
Send a GET request in the given route: /files/download/:destination | ||
|
||
Note: The deafult starting destination is /uploads of your root directory (same level as that of node_modules directory) | ||
'destination' is relative to this directory | ||
|
||
API GET Request: | ||
/files/download/resume | ||
|
||
The location of this file is 'your_project'/uploads/resume | ||
|
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.
Fix typo and improve clarity in file download steps.
- Correct the typo "deafult" to "default".
- Make the instructions more concise and clear.
- Note: The deafult starting destination is /uploads of your root directory (same level as that of node_modules directory)
+ Note: The default starting destination is /uploads in your root directory (same level as the node_modules directory).
- 'destination' is relative to this directory
+ The 'destination' is relative to this directory.
- The location of this file is 'your_project'/uploads/resume
+ The location of the file is 'your_project'/uploads/resume.
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.
## Steps To Download File | |
Send a GET request in the given route: /files/download/:destination | |
Note: The deafult starting destination is /uploads of your root directory (same level as that of node_modules directory) | |
'destination' is relative to this directory | |
API GET Request: | |
/files/download/resume | |
The location of this file is 'your_project'/uploads/resume | |
## Steps To Download File | |
Send a GET request in the given route: /files/download/:destination | |
Note: The default starting destination is /uploads in your root directory (same level as the node_modules directory). | |
The 'destination' is relative to this directory. | |
API GET Request: | |
/files/download/resume | |
The location of the file is 'your_project'/uploads/resume. | |
This PR's commit is merged in #159 |
out-2.mp4
Summary by CodeRabbit
New Features
Bug Fixes
getHello()
method from'Hello World from sample/07-file-upload!'
to'Hello World from sample/06-file-upload!'
.Chores
.gitignore
anddocs/.gitignore
to ignoredata
anddist
directories.package.json
to use Stencil commands instead of NestJS commands.@nestjs/config
,@samagra-x/stencil
, andminio
.env-example
for file upload settings.Tests