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

feat(platform): Improved UI of secret listing #655

Merged
merged 2 commits into from
Jan 25, 2025
Merged

feat(platform): Improved UI of secret listing #655

merged 2 commits into from
Jan 25, 2025

Conversation

rajdip-b
Copy link
Member

@rajdip-b rajdip-b commented Jan 24, 2025 •
edited by codiumai-pr-agent-free bot
Loading

User description

Description

Fixes #622

PR Type

Enhancement

Description

  • Updated secret listing to display "Hidden" for non-decrypted values.

  • Enhanced environment display format to include name and slug.

  • Changed "Secret" tab header to "Value".

  • Displayed user name instead of ID in "last edited by".

Changes walkthrough 📝

Relevant files
Enhancement
page.tsx
Enhance secret listing logic and API integration                 

apps/platform/src/app/(main)/project/[project]/@secret/page.tsx

  • Added isDecrypted logic to determine secret visibility.
  • Updated API call to include decryptValue parameter.
  • Adjusted dependencies in useEffect for reactivity.
  • Modified secret mapping to pass isDecrypted and updated props.
    		• +13/-5   
    index.tsx
    Update secret card display and formatting                               

    apps/platform/src/components/dashboard/secret/secretCard/index.tsx

  • Introduced isDecrypted prop to control secret visibility.
  • Updated environment display to include name and slug.
  • Changed "Secret" tab header to "Value".
  • Displayed user name instead of ID in "last edited by".
    		• +17/-5   
    Need help?
  • Type /help how to ... in the comments thread for any questions about Qodo Merge usage.
  • Check out the documentation for more information.
    •

    @rajdip-b rajdip-b requested a review from kriptonian1 as a code owner January 24, 2025 19:48
    @codiumai-pr-agent-free CodiumAI PR-Agent (Free)
    Copy link
    Contributor

    PR Reviewer Guide 🔍

    Here are some key observations to aid the review process:

    🎫 Ticket compliance analysis ✅

    622 - PR Code Verified

    Compliant requirements:

    • Show "Hidden" for non-decrypted secret values instead of buffer
    • Display environment in format: "Environment Name (environment slug)"
    • Rename "Secret" tab to "Value"
    • Show user name instead of ID in "last edited by"

    Requires further human verification:

    • Verify that "Hidden" text appears correctly for non-decrypted values in UI
    • Validate environment name and slug display format in UI
    ⏱️ Estimated effort to review: 2 🔵🔵⚪⚪⚪
    🧪 No relevant tests
    🔒 No security concerns identified
    ⚡ Recommended focus areas for review

    Error Handling

    The error case in the API call is not properly handled - error message is not displayed to user when API call fails

    const { success, error, data } =
  await ControllerInstance.getInstance().secretController.getAllSecretsOfProject(
    { projectSlug: selectedProject.slug, decryptValue: isDecrypted },
    {}
  )

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

    PR Code Suggestions ✨

    Explore these optional code suggestions:

    CategorySuggestion                                                                                                                                    Score
    Possible issue
    Fix async loading state handling

    Move setIsLoading(false) inside the getAllSecretsByProjectSlug function after the
    API call completes to prevent premature loading state changes.

    apps/platform/src/app/(main)/project/[project]/@secret/page.tsx [61-63]

    +async function getAllSecretsByProjectSlug() {
+  if (!selectedProject?.slug) {
+    setIsLoading(false)
+    return
+  }
+  const { success, data } = await ControllerInstance.getInstance().secretController.getAllSecretsOfProject(
+    { projectSlug: selectedProject.slug, decryptValue: isDecrypted },
+    {}
+  )
+  if (success && data) {
+    setSecrets(data)
+  }
+  setIsLoading(false)
+}
 getAllSecretsByProjectSlug()
 
-setIsLoading(false)
-
    • Apply this suggestion
    Suggestion importance[1-10]: 8

    Why: Moving setIsLoading(false) inside the async function is important for preventing race conditions and ensuring the loading state accurately reflects the API call completion status.

    		8
    Add error state handling

    Add error handling for failed API responses to properly inform users and handle
    error states.

    apps/platform/src/app/(main)/project/[project]/@secret/page.tsx [47-53]

     const { success, error, data } =
   await ControllerInstance.getInstance().secretController.getAllSecretsOfProject(
     { projectSlug: selectedProject.slug, decryptValue: isDecrypted },
     {}
   )
 
 if (success && data) {
   setSecrets(data)
+} else if (error) {
+  setSecrets([])
+  toast.error(`Failed to load secrets: ${error.message}`)
 }
    • Apply this suggestion
    Suggestion importance[1-10]: 7

    Why: Adding proper error handling with user feedback is important for reliability and UX, though the error case is already partially handled by clearing the secrets array.

    		7

    kriptonian1
    kriptonian1 approved these changes Jan 25, 2025
    View reviewed changes
    Copy link
    Contributor

    @kriptonian1 kriptonian1 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 b19de47 into develop Jan 25, 2025
    4 checks passed
    @rajdip-b rajdip-b deleted the feat/improvements-in-secrets-tab branch January 25, 2025 05:34
    rajdip-b pushed a commit that referenced this pull request Jan 25, 2025
    @semantic-release-bot
    chore(release): 2.10.0-stage.3 [skip ci]
    941a815 
    ## [2.10.0-stage.3](v2.10.0-stage.2...v2.10.0-stage.3) (2025-01-25)

### 🚀 Features

* **platform:** Improved UI of [secure] listing ([#655](#655)) ([b19de47](b19de47))

### 🔧 Miscellaneous Chores

* **ci:** Fixed misplaced sentry sourcemaps commands ([fbd6f3b](fbd6f3b))

### 🔨 Code Refactoring

* **api:** Update getSelf function ([fe752ce](fe752ce))
    @rajdip-b
    Copy link
    Member Author

    🎉 This PR is included in version 2.10.0-stage.3 🎉

    The release is available on GitHub release

    Your semantic-release bot 📦🚀

    rajdip-b pushed a commit that referenced this pull request Jan 28, 2025
    @semantic-release-bot
    chore(release): 2.10.0 [skip ci]
    8ae824d 
    ## [2.10.0](v2.9.1...v2.10.0) (2025-01-28)

### 🚀 Features

* **api:** Secret rotation ([#652](#652)) ([ad9a808](ad9a808))
* **platform:** Implement delete project ([#671](#671)) ([d243c89](d243c89))
* **platform:** Improved UI of [secure] listing ([#655](#655)) ([b19de47](b19de47))
* **platform:** Operate on environments ([#670](#670)) ([f45c5fa](f45c5fa))

### 🐛 Bug Fixes

* Added lockfile ([856eb3c](856eb3c))
* **api:** Only user's default workspace returns isDefault: true ([#647](#647)) ([870b4dc](870b4dc))
* **cli:** Workspace membership API client payload fixed ([#614](#614)) ([#648](#648)) ([e23057b](e23057b))
* **platform:** Refactor layout structure to improve Navbar positioning & child component ([#661](#661)) ([31067f3](31067f3))
* **Platfrom:** Replace manual date calculation with dayjs to improve better calculation ([#668](#668)) ([990eb86](990eb86))

### 🔧 Miscellaneous Chores

* **ci:** Add manual trigger ([cfbf4b9](cfbf4b9))
* **ci:** Add missing LATEST_TAG variable ([a2ea2ed](a2ea2ed))
* **ci:** Fixed misplaced sentry sourcemaps commands ([fbd6f3b](fbd6f3b))
* **ci:** Fixed scripts ([374f7ed](374f7ed))
* **ci:** Update API sentry dist folder ([2bc9afb](2bc9afb))
* **CI:** Update pipeline ([fd63b70](fd63b70))
* **ci:** Update sourcemap upload commands ([c7e8e45](c7e8e45))
* **cli:** Bumped CLI version to 2.5.0 ([7b772f8](7b772f8))
* Fix prerelease branch config ([7e84021](7e84021))
* **release:** 2.10.0-stage.1 [skip ci] ([a4f8414](a4f8414)), closes [#652](#652)
* **release:** 2.10.0-stage.2 [skip ci] ([00ee123](00ee123)), closes [#647](#647)
* **release:** 2.10.0-stage.3 [skip ci] ([941a815](941a815)), closes [#655](#655)
* **release:** 2.10.0-stage.4 [skip ci] ([ae7c44f](ae7c44f)), closes [#614](#614) [#648](#648)
* **release:** 2.10.0-stage.5 [skip ci] ([d718483](d718483)), closes [#661](#661)
* **release:** 2.10.0-stage.6 [skip ci] ([4e63f47](4e63f47)), closes [#668](#668)
* **release:** 2.10.0-stage.7 [skip ci] ([4a35fe7](4a35fe7)), closes [#671](#671)
* **release:** 2.10.0-stage.8 [skip ci] ([36ef21d](36ef21d)), closes [#670](#670)
* **release:** 2.9.2-stage.1 [skip ci] ([443f8d4](443f8d4))

### 🔨 Code Refactoring

* **api:** Update getSelf function ([fe752ce](fe752ce))
    @rajdip-b
    Copy link
    Member Author

    🎉 This PR is included in version 2.10.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

    @kriptonian1 kriptonian1 kriptonian1 approved these changes

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

    Successfully merging this pull request may close these issues.

    PLATFORM: Improvements in displaying a secret
    2 participants
    @rajdip-b @kriptonian1