Skip to content
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.

Sign up for GitHub

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

Jump to bottom

chore(api): Get feedback forward email from process.env #236

Merged
merged 3 commits into from
May 23, 2024
Merged

chore(api): Get feedback forward email from process.env #236

merged 3 commits into from
May 23, 2024

Conversation

Princeyadav05
Copy link
Contributor

@Princeyadav05 Princeyadav05 commented May 22, 2024 •
edited by codiumai-pr-agent-free bot
Loading

User description

Description

  • Created an environmental variable named FEEDBACK_FORWARD_EMAIL
  • Added it in .env.example
  • Added it in zod schema
  • Replaced the hardcoded email

Fixes #235

Dependencies

Mention any dependencies/packages used

Future Improvements

Mention any improvements to be done in future related to any file/feature

Mentions

Mention and tag the people

Screenshots of relevant screens

Add screenshots of relevant screens

Developer's checklist

  • My PR follows the style guidelines of this project
  • I have performed a self-check on my work

If changes are made in the code:

  • I have followed the coding guidelines
  • My changes in code generate no new warnings
  • My changes are breaking another fix/feature of the project
  • I have added test cases to show that my feature works
  • I have added relevant screenshots in my PR
  • There are no UI/UX issues

Documentation Update

  • This PR requires an update to the documentation at docs.keyshade.xyz
  • I have made the necessary updates to the documentation, or no documentation changes are required.

PR Type

enhancement, configuration changes

Description

  • Added FEEDBACK_FORWARD_EMAIL to the environment schema in env.schema.ts.
  • Replaced hardcoded admin email with FEEDBACK_FORWARD_EMAIL from environment variables in feedback.service.ts.
  • Updated user controller test to use FEEDBACK_FORWARD_EMAIL from environment variables in user.e2e.spec.ts.
  • Added FEEDBACK_FORWARD_EMAIL to the example environment file.

Changes walkthrough 📝

Relevant files
Enhancement
env.schema.ts
Add `FEEDBACK_FORWARD_EMAIL` to environment schema.           

apps/api/src/common/env/env.schema.ts

  • Added FEEDBACK_FORWARD_EMAIL to the environment schema.
 +3/-1     
feedback.service.ts
Use FEEDBACK_FORWARD_EMAIL from environment variables in feedback
service.

apps/api/src/feedback/service/feedback.service.ts

  • Replaced hardcoded admin email with FEEDBACK_FORWARD_EMAIL from
    environment variables.
    		•  +1/-1     
    Tests
    user.e2e.spec.ts
    Update user controller test to use `FEEDBACK_FORWARD_EMAIL`.

    apps/api/src/user/user.e2e.spec.ts

  • Updated test to use FEEDBACK_FORWARD_EMAIL from environment variables.

    		•  +1/-1     
    Configuration changes
    .env.example
    Add `FEEDBACK_FORWARD_EMAIL` to example environment file.

    .env.example

    • Added FEEDBACK_FORWARD_EMAIL to the example environment file.
    		 +3/-1     

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

    @Princeyadav05 Princeyadav05 requested a review from rajdip-b as a code owner May 22, 2024 20:08
    @rajdip-b
    Copy link
    Member

    The code looks perfect! Could you please also update the docs? I forgot to mention about this. https://docs.keyshade.xyz/contributing-to-keyshade/environment-variables you would need to update the content for this page!

    @codiumai-pr-agent-free CodiumAI PR-Agent (Free)
    Copy link
    Contributor

    PR Review 🔍

    ⏱️ Estimated effort to review [1-5]

    2, because the changes are straightforward and limited to a few files. The PR involves adding an environment variable and replacing hardcoded values with this variable. The changes are not complex but need to be checked for correctness in terms of environment variable usage and potential impact on existing functionalities.

    🧪 Relevant tests

    No

    ⚡ Possible issues

    Possible Bug: The new environment variable FEEDBACK_FORWARD_EMAIL is used without a fallback. If it's not set, this could lead to runtime errors or failed operations when trying to send feedback emails or create admin users in tests.

    🔒 Security concerns

    No

    @codiumai-pr-agent-free CodiumAI PR-Agent (Free)
    Copy link
    Contributor

    PR Code Suggestions ✨

    CategorySuggestion                                                                                                                                    Score
    Enhancement
    Add email format validation for FEEDBACK_FORWARD_EMAIL

    It is recommended to validate the email format for FEEDBACK_FORWARD_EMAIL to ensure that
    it adheres to standard email formatting. This can prevent runtime errors and issues
    related to invalid email formats being used in the application.

    apps/api/src/common/env/env.schema.ts [71]

    -FEEDBACK_FORWARD_EMAIL: z.string()
+FEEDBACK_FORWARD_EMAIL: z.string().email()
    Suggestion importance[1-10]: 10

    Why: Adding email format validation is crucial to ensure that the FEEDBACK_FORWARD_EMAIL adheres to standard email formatting, preventing potential runtime errors and issues related to invalid email formats.

    		10
    Possible issue
    Use a fixed email value for consistent test setups

    Using an environment variable directly in test setups can lead to inconsistent test
    results depending on the environment configuration. It's better to use a fixed value or a
    mock for consistency.

    apps/api/src/user/user.e2e.spec.ts [44]

    -email: process.env.FEEDBACK_FORWARD_EMAIL,
+email: '[email protected]',
    Suggestion importance[1-10]: 9

    Why: Using a fixed email value in test setups ensures consistency and reliability of tests, regardless of the environment configuration. This helps in avoiding flaky tests and ensures predictable outcomes.

    		9
    Ensure FEEDBACK_FORWARD_EMAIL has a fallback value

    It's crucial to validate the presence of FEEDBACK_FORWARD_EMAIL in the environment
    variables before using it. This can prevent sending emails to an undefined address, which
    would cause the application to throw an error.

    apps/api/src/feedback/service/feedback.service.ts [17]

    -const adminEmail = process.env.FEEDBACK_FORWARD_EMAIL;
+const adminEmail = process.env.FEEDBACK_FORWARD_EMAIL || '[email protected]';
    Suggestion importance[1-10]: 8

    Why: Providing a fallback value for FEEDBACK_FORWARD_EMAIL is important to prevent the application from throwing errors when the environment variable is not set. However, using a hardcoded default email might not be ideal in all scenarios.

    		8
    Best practice
    Add a default value for FEEDBACK_FORWARD_EMAIL

    Provide a default value for FEEDBACK_FORWARD_EMAIL in the .env.example file to guide
    developers on the expected format and to avoid issues when setting up the environment.

    .env.example [43]

    -FEEDBACK_FORWARD_EMAIL=
+[email protected]
    Suggestion importance[1-10]: 7

    Why: Providing a default value in the .env.example file can guide developers on the expected format and prevent issues during environment setup. However, the default value should be generic and not expose any real email addresses.

    		7

    @Princeyadav05
    Copy link
    Contributor Author

    The code looks perfect! Could you please also update the docs? I forgot to mention about this. https://docs.keyshade.xyz/contributing-to-keyshade/environment-variables you would need to update the content for this page!

    Done. Let me know if the definition for this env variable needs to be updated.

    @rajdip-b rajdip-b changed the title chore(api): feedback forward email from process env chore(api): Get feedback forward email from process.env May 23, 2024
    rajdip-b
    rajdip-b requested changes May 23, 2024
    View reviewed changes
    apps/api/src/user/user.e2e.spec.ts Outdated Show resolved Hide resolved
    docs/contributing-to-keyshade/environment-variables.md Outdated Show resolved Hide resolved
    @sonarqubecloud SonarQubeCloud
    Copy link

    Quality Gate Passed Quality Gate passed

    Issues
    0 New issues
    0 Accepted issues

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

    See analysis details on SonarCloud

    rajdip-b
    rajdip-b approved these changes May 23, 2024
    View reviewed changes
    Copy link
    Member

    @rajdip-b rajdip-b left a comment

    Choose a reason for hiding this comment

    The reason will be displayed to describe this comment to others. Learn more.

    LGTM!

    @rajdip-b rajdip-b merged commit 204c9d1 into keyshade-xyz:develop May 23, 2024
    7 checks passed
    rajdip-b pushed a commit that referenced this pull request May 24, 2024
    @semantic-release-bot
    chore(release): 1.4.0 [skip ci]
    55862df 
    ## [1.4.0](v1.3.0...v1.4.0) (2024-05-24)

### 🚀 Features

* add example for health and email auth ([b834d25](b834d25))
* **api:** Add `minio-client` provider ([#237](#237)) ([cd71c5a](cd71c5a))
* **api:** Add feature to fork projects ([#239](#239)) ([3bab653](3bab653))
* **api:** Added feedback form module ([#210](#210)) ([ae1efd8](ae1efd8))
* **api:** Added Project Level Access  ([#221](#221)) ([564f5ed](564f5ed))
* **api:** Added support for changing email of users ([#233](#233)) ([5ea9a10](5ea9a10))
* implemented auth, ui for most, and fixed cors ([#217](#217)) ([feace86](feace86))
* **platfrom:** add delete method in api client ([#225](#225)) ([55cf09f](55cf09f))
* **postman:** add example for get_self and update_self ([e015acf](e015acf))
* **web:** Add and link privacy and tnc page ([#226](#226)) ([ec81eb9](ec81eb9))

### 🐛 Bug Fixes

* **web:** docker next config not found ([#228](#228)) ([afe3160](afe3160))

### 📚 Documentation

* Added docs regarding postman, and refactored architecture diagrams ([f1c9777](f1c9777))
* Fix typo in organization-of-code.md ([#234](#234)) ([11244a2](11244a2))

### 🔧 Miscellaneous Chores

* **api:** Get feedback forward email from process.env ([#236](#236)) ([204c9d1](204c9d1))
* **postman:** Initialized postman ([bb76384](bb76384))
* **release:** Update changelog config ([af91283](af91283))
* Remove swagger docs ([#220](#220)) ([7640299](7640299))

### 🔨 Code Refactoring

* **api:** Replaced OTP code from alphanumeric to numeric ([#230](#230)) ([f16162a](f16162a))
    @rajdip-b
    Copy link
    Member

    🎉 This PR is included in version 1.4.0 🎉

    The release is available on GitHub release

    Your semantic-release bot 📦🚀

    Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
    Reviewers

    @rajdip-b rajdip-b rajdip-b approved these changes

    Assignees
    No one assigned
    Labels
    enhancement Review effort [1-5]: 2
    Projects
    None yet
    Milestone
    No milestone
    Development

    Successfully merging this pull request may close these issues.

    API: Get the email to send feedbacks to from process.env
    2 participants
    @Princeyadav05 @rajdip-b