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): Show all the existing variables inside a project #591

Open
wants to merge 1 commit into
base: develop
Choose a base branch
from
Open

feat(platform): Show all the existing variables inside a project #591

wants to merge 1 commit into from

Conversation

poswalsameer
Copy link
Contributor

@poswalsameer poswalsameer commented Dec 16, 2024 •
edited by codiumai-pr-agent-free bot
Loading

User description

Description

This PR adds the feature of showing all the existing variables present inside a project on the page load on variables tab.

Fixes #558

Screenshots of relevant screens

Screenshot 2024-12-17 031743
Screenshot 2024-12-17 031804

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

Description

This PR implements the variable management feature in the platform:

  • Added variable listing page showing all variables in a project

    • Collapsible sections for each variable showing values across environments
    • Empty state UI when no variables exist
    • Integration with API to fetch variable data

  • Implemented variable creation functionality

    • Dialog form to add new variables with name, note and environment values
    • Success/error toast notifications
    • API integration for variable creation

  • UI Improvements

    • Added new shared SVG icons
    • Created reusable collapsible component
    • Updated project routing to use slugs

  • The changes follow the Figma design and integrate with the backend API endpoints

Changes walkthrough 📝

Relevant files
Enhancement
index.ts
Add new shared SVG icon components                                             

apps/platform/public/svg/shared/index.ts

  • Added new SVG imports for Message, Vector, and Error icons
  • Updated exports to include the new SVG components
    		•  +7/-1     
    page.tsx
    Implement variable listing page with collapsible sections

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

  • Implemented variable listing page with collapsible sections for each
    variable
  • Added empty state UI when no variables exist
  • Integrated with API to fetch project variables
  • Added table view to display variable values across environments
    		•  +170/-3 
    layout.tsx
    Add variable creation functionality with UI feedback         

    apps/platform/src/app/(main)/project/[project]/layout.tsx

  • Added variable creation dialog with form inputs
  • Integrated with API to create new variables
  • Added toast notifications for success/error states
  • Updated project layout to handle variable tab
    		•  +328/-52
    index.tsx
    Update project card to use slug for routing                           

    apps/platform/src/components/dashboard/projectCard/index.tsx

    • Updated project link to use slug instead of id
    		 +2/-1     
    collapsible.tsx
    Add collapsible UI component                                                         

    apps/platform/src/components/ui/collapsible.tsx

    • Added new collapsible component using Radix UI primitives
    		 +12/-0   

    💡 PR-Agent usage: Comment /help "your question" on any pull request to receive relevant information

    @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 ✅

    558 - Fully compliant

    Compliant requirements:

    • Show data of all variables of a project in the platform
    • Implement feature in specified file path
    • Follow Figma design
    ⏱️ Estimated effort to review: 4 🔵🔵🔵🔵⚪
    🧪 No relevant tests
    🔒 Security concerns

    Sensitive information exposure:
    The variable values are displayed in plain text in the UI table. Consider masking sensitive values or adding a reveal/hide toggle functionality.

    ⚡ Recommended focus areas for review

    Error Handling
    Variable creation error handling only checks for name conflicts but not other potential API errors

    Performance Issue
    Variable data fetching happens on every component mount without any caching or state management

    Form Validation
    Missing input validation for variable name and value fields in the create variable form

    @poswalsameer poswalsameer changed the title Feat: Added feature to show all the existing variables inside a project feat: Added feature to show all the existing variables inside a project Dec 16, 2024
    @codiumai-pr-agent-free CodiumAI PR-Agent (Free)
    Copy link
    Contributor

    PR Code Suggestions ✨

    Explore these optional code suggestions:

    CategorySuggestion                                                                                                                                    Score
    Possible issue
    Add proper error handling for API calls to improve reliability and user feedback

    Add error handling for the variable creation API call. Currently, the error case
    only shows a toast but doesn't handle API failures gracefully. Consider adding a
    try-catch block and proper error state management.

    apps/platform/src/app/(main)/project/[project]/layout.tsx [118-127]

    -const { success, error, data } = await variableController.createVariable(
-  request,
-  {}
-)
+try {
+  const { success, error, data } = await variableController.createVariable(
+    request,
+    {}
+  )
 
-if( success ){
-  toast(...)
-}
-if( error ){
-  toast(...)
+  if (success) {
+    toast(...)
+    setIsOpen(false)
+  } else {
+    toast(...)
+    console.error('Failed to create variable:', error)
+  }
+} catch (err) {
+  console.error('API call failed:', err)
+  toast(
+    <div className='flex flex-col justify-center items-start h-[4.5rem] w-[23.438rem] rounded-md bg-[#450A0A] text-[#E92D1F]'>
+      <div className='flex justify-center items-center gap-x-4'>
+        <ErrorSVG />
+        <div className='flex flex-col justify-center'>
+          <p className='text-sm font-semibold'>Failed to create variable</p>
+          <p className='text-xs font-normal'>An unexpected error occurred. Please try again.</p>
+        </div>
+      </div>
+    </div>
+  )
 }
    • Apply this suggestion
    Suggestion importance[1-10]: 8

    Why: The suggestion significantly improves error handling by adding try-catch and proper error states, which is critical for API reliability and user experience.

    		8
    Add cleanup handling to prevent memory leaks and state updates on unmounted components

    The useEffect hook fetching variables doesn't handle cleanup properly, which could
    lead to memory leaks and state updates on unmounted components. Add a cleanup
    function to cancel pending requests.

    apps/platform/src/app/(main)/project/[project]/@variable/page.tsx [52-77]

     useEffect(() => {
+  let isSubscribed = true;
+  
   const getVariables = async () => {
-    const {
-      success,
-      error,
-      data
-    }: ClientResponse<GetAllVariablesOfProjectResponse> =
-      await variableController.getAllVariablesOfProject(
-        { projectSlug: currentProject?.slug },
-        {}
-      )
+    try {
+      const {
+        success,
+        error,
+        data
+      }: ClientResponse<GetAllVariablesOfProjectResponse> =
+        await variableController.getAllVariablesOfProject(
+          { projectSlug: currentProject?.slug },
+          {}
+        )
 
-    if (success && data) {
-      setAllVariables(data.items)
-    } else {
-      console.error(error)
+      if (isSubscribed) {
+        if (success && data) {
+          setAllVariables(data.items)
+        } else {
+          console.error(error)
+        }
+      }
+    } catch (err) {
+      if (isSubscribed) {
+        console.error('Failed to fetch variables:', err)
+      }
     }
   }
 
-  getVariables()
+  if (currentProject?.slug) {
+    getVariables()
+  }
+
+  return () => {
+    isSubscribed = false
+  }
 }, [currentProject])
    • Apply this suggestion
    Suggestion importance[1-10]: 7

    Why: The suggestion prevents memory leaks and race conditions by adding proper cleanup in useEffect, which is important for component lifecycle management.

    		7
    General
    Improve type safety by properly typing form event handlers

    The form submission handler addVariable uses any type for the event parameter and
    doesn't prevent default form behavior properly. Type the event correctly and prevent
    form submission.

    apps/platform/src/app/(main)/project/[project]/layout.tsx [96-97]

    -const addVariable = async (e: any) => {
+const addVariable = async (e: React.FormEvent<HTMLFormElement>) => {
   e.preventDefault()
    • Apply this suggestion
    Suggestion importance[1-10]: 5

    Why: The suggestion improves type safety by replacing any with proper event type, reducing potential runtime errors but has moderate impact on functionality.

    		5

    @rajdip-b rajdip-b changed the title feat: Added feature to show all the existing variables inside a project feat(platform): Added feature to show all the existing variables inside a project Dec 17, 2024
    @rajdip-b rajdip-b changed the title feat(platform): Added feature to show all the existing variables inside a project feat(platform): Show all the existing variables inside a project Dec 17, 2024
    Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
    Reviewers

    @kriptonian1 kriptonian1 Awaiting requested review from kriptonian1 kriptonian1 is a code owner

    At least 1 approving review is required to merge this pull request.

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

    Successfully merging this pull request may close these issues.

    PLATFORM: Show all variables of project
    1 participant
    @poswalsameer