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.

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

feat: Configured extra check for waitlisted users already in the list and created toast message for them #492

Conversation

poswalsameer
Copy link
Contributor

@poswalsameer poswalsameer commented Oct 15, 2024

User description

Description

This PR solves the issue by showing an error/warning toast message to already existing users on the waitlist. Before this PR, every user with a single email was getting the success toast message multiple times. Ideally, this should not happen, every user should be able to join the waitlist one time through one email. This PR fixes this problem.

Fixes #463

Dependencies

No dependencies or packages are used.

Future Improvements

Right now, the data for waitlisted users is stored in the local storage but this logic can be improved by mapping them to a database in the future.

Screenshots of relevant screens

Screenshot (94)

Developer's checklist

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

If changes are made in the code:

  • [ x] I have followed the coding guidelines
  • [ x] 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
  • [ x] I have added relevant screenshots in my PR
  • [ x] There are no UI/UX issues

Documentation Update

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

PR Type

enhancement, bug_fix


Description

  • Implemented a check to verify if a user's email is already in the waitlist, preventing duplicate entries.
  • Displayed a custom toast message to inform users if they are already on the waitlist.
  • Updated the local storage to include new emails added to the waitlist.
  • Cleared the email input field after a successful waitlist addition to improve user experience.

Changes walkthrough 📝

Relevant files
Enhancement
index.tsx
Add email validation and waitlist check with toast notifications

apps/web/src/components/hero/index.tsx

  • Added logic to check if an email is already in the waitlist.
  • Displayed a custom toast message for already waitlisted users.
  • Updated local storage with new waitlisted emails.
  • Cleared the email input field after successful submission.
  • +26/-2   

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

    Copy link
    Contributor

    PR Reviewer Guide 🔍

    Here are some key observations to aid the review process:

    🎫 Ticket compliance analysis 🔶

    463 - Partially compliant

    Fully compliant requirements:

    • Clear the text field after adding the user to the waitlist
    • Check if user has already whitelisted

    Not compliant requirements:

    • Perform validation on the email field
    • Only show toast when the whitelist response is successful
    ⏱️ Estimated effort to review: 2 🔵🔵⚪⚪⚪
    🧪 No relevant tests
    🔒 No security concerns identified
    ⚡ Recommended focus areas for review

    Possible Bug
    The email validation is still using the existing emailSchema, which may not provide sufficient validation. Consider implementing a more robust email validation.

    Performance Issue
    The waitlist data is being stored in both state and localStorage. Consider using only localStorage to avoid redundancy and potential synchronization issues.

    Code Smell
    The unused variable _waitListData is declared with a TypeScript ignore comment. Consider removing the unused variable or utilizing it if necessary.

    Copy link
    Contributor

    codiumai-pr-agent-free bot commented Oct 15, 2024

    PR Code Suggestions ✨

    Explore these optional code suggestions:

    CategorySuggestion                                                                                                                                    Score
    Possible issue
    Add error handling for localStorage operations to improve user experience

    Consider adding error handling and user feedback for the case when localStorage is
    not available or fails to update.

    apps/web/src/components/hero/index.tsx [67-71]

     setWaitListData((prevData) => {
       const updatedData: string[] = [...prevData, email]
    -  localStorage.setItem('waitListData', JSON.stringify(updatedData))
    +  try {
    +    localStorage.setItem('waitListData', JSON.stringify(updatedData))
    +  } catch (error) {
    +    console.error('Failed to update localStorage:', error)
    +    toast.error('Failed to save your email. Please try again.')
    +  }
       return updatedData
     })
    • Apply this suggestion
    Suggestion importance[1-10]: 9

    Why: Adding error handling for localStorage operations is crucial for robustness, especially in environments where localStorage might be unavailable or fail. This improves user experience by providing feedback when an error occurs.

    9
    Performance
    Use a Set data structure for more efficient email lookup in the waitlist

    Consider using a Set instead of an array for waitListedEmails to improve lookup
    performance when checking if an email is already in the waitlist.

    apps/web/src/components/hero/index.tsx [32-36]

     const dataInStorage: string | null = localStorage.getItem('waitListData')
    -const waitListedEmails: string[] = dataInStorage ? (JSON.parse(dataInStorage) as string[]) : []
    +const waitListedEmails: Set<string> = new Set(dataInStorage ? JSON.parse(dataInStorage) as string[] : [])
     
     // actual logic where we are checking if this email is already in waitlisted users or not
    -if (waitListedEmails.includes(email)) {
    +if (waitListedEmails.has(email)) {
    • Apply this suggestion
    Suggestion importance[1-10]: 8

    Why: Using a Set instead of an array for email lookup improves performance due to faster average time complexity for lookups. This change is relevant and beneficial for the functionality of checking if an email is already in the waitlist.

    8
    Maintainability
    Extract toast message content into a separate component for better code organization

    Consider extracting the toast message content into a separate component or constant
    to improve code readability and maintainability.

    apps/web/src/components/hero/index.tsx [37-44]

    -toast.custom(() => (
    +const AlreadyWaitlistedToast = () => (
       <div className="text-brandBlue border-brandBlue/20 w-[90vw] rounded-lg border bg-[#852b2c] p-2 shadow-2xl backdrop-blur-3xl md:w-[20vw]">
         <p className="text-sm">
           You have been already added to the waitlist. We will notify you once
           we launch.{' '}
         </p>
       </div>
    -))
    +);
     
    +toast.custom(AlreadyWaitlistedToast)
    +
    • Apply this suggestion
    Suggestion importance[1-10]: 6

    Why: Extracting the toast message into a separate component enhances code readability and maintainability by reducing duplication and improving organization. This is a good practice for managing UI components.

    6
    Best practice
    ✅ Use a more descriptive variable name or remove unused state variable
    Suggestion Impact:The unused state variable _waitListData was removed, which aligns with the suggestion to remove it if unused.

    code diff:

    -  //@typescript-eslint/no-unused-vars
    -  const [_waitListData, setWaitListData] = useState<string[]>([])

    Consider using a more descriptive variable name instead of _waitListData to improve
    code readability, or remove it if unused.

    apps/web/src/components/hero/index.tsx [15-16]

    -//@typescript-eslint/no-unused-vars
    -const [_waitListData, setWaitListData] = useState<string[]>([])
    +const [waitListData, setWaitListData] = useState<string[]>([])
    • Apply this suggestion
    Suggestion importance[1-10]: 5

    Why: Renaming _waitListData to waitListData improves code readability by using a more descriptive name. However, the variable is marked as unused, so the suggestion could also consider removing it if it remains unused.

    5

    💡 Need additional feedback ? start a PR chat

    @poswalsameer
    Copy link
    Contributor Author

    Hi @rajdip-b and @kriptonian1, this is a fresh PR that I have created for issue #463. You can review the code and let me know if any more changes are required. Thanks, loved working on this!

    @rajdip-b
    Copy link
    Member

    Can you resolve the conflict?

    @poswalsameer
    Copy link
    Contributor Author

    Can you resolve the conflict?

    Let me see.

    @poswalsameer
    Copy link
    Contributor Author

    Can you resolve the conflict?

    Cannot figure out why is this showing as a conflict here, because that is the code which I have added to add the email in the state ( inside the input box ). Would like a small help in this!

    @kriptonian1
    Copy link
    Contributor

    kriptonian1 commented Oct 15, 2024

    @poswalsameer Would you like to get on a call on discord?

    drop your discord username or maybe tag me on discord

    @poswalsameer
    Copy link
    Contributor Author

    @poswalsameer Would you like to get on a call on discord?

    yeah sure

    @poswalsameer
    Copy link
    Contributor Author

    Hi @kriptonian1, done with the changes as we discussed. You can have a final look.

    apps/web/src/components/hero/index.tsx Outdated Show resolved Hide resolved
    apps/web/src/components/hero/index.tsx Outdated Show resolved Hide resolved
    apps/web/src/components/hero/index.tsx Outdated Show resolved Hide resolved
    apps/web/src/components/hero/index.tsx Outdated Show resolved Hide resolved
    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, but we will really appreciate it if you can do the last request we made

    apps/web/src/components/hero/index.tsx Outdated Show resolved Hide resolved
    apps/web/src/components/hero/index.tsx Outdated Show resolved Hide resolved
    apps/web/src/components/hero/index.tsx Outdated Show resolved Hide resolved
    apps/web/src/components/hero/index.tsx Outdated Show resolved Hide resolved
    @kriptonian1 kriptonian1 added the hacktoberfest-accepted Your PR has this = Congrats! label Oct 16, 2024
    @kriptonian1 kriptonian1 requested a review from rajdip-b October 18, 2024 18:09
    @kriptonian1
    Copy link
    Contributor

    @rajdip-b if everything is okey from your end then we can merge

    @rajdip-b rajdip-b merged commit 2ddd0ef into keyshade-xyz:develop Oct 19, 2024
    4 checks passed
    rajdip-b pushed a commit that referenced this pull request Oct 24, 2024
    ## [2.6.0](v2.5.0...v2.6.0) (2024-10-24)
    
    ### 🚀 Features
    
    * **api:**  Add icon and remove description field from workspace ([#435](#435)) ([a99c0db](a99c0db))
    * **api-client:** Added workspace-membership and related tests ([#452](#452)) ([6a1c091](6a1c091))
    * **api-client:** Create controller for User module ([#484](#484)) ([f9d8e83](f9d8e83))
    * **api:** Add prod env schema in env file ([#436](#436)) ([21c3004](21c3004))
    * **api:** Add resend otp implementation ([#445](#445)) ([4dc6aa1](4dc6aa1))
    * **api:** Fetch total count of environments, [secure]s and variables in project ([#434](#434)) ([0c9e50a](0c9e50a))
    * **api:** Replace `projectId` with `name` and `slug` in workspace-role response.  ([#432](#432)) ([af06071](af06071))
    * **cli:** Add functionality to operate on Secrets ([#504](#504)) ([1b4bf2f](1b4bf2f))
    * **cli:** Add project command ([#451](#451)) ([70448e1](70448e1))
    * **cli:** Add workspace operations ([#441](#441)) ([ed38d22](ed38d22))
    * **cli:** implement commands to get, list, update, and delete, workspace roles ([#469](#469)) ([957ea8d](957ea8d))
    * **cli:** Implemented pagination support ([#453](#453)) ([feb1806](feb1806))
    * **cli:** Secret scan ([#438](#438)) ([85cb8ab](85cb8ab))
    * **cli:** Update environment command outputs ([f4af874](f4af874))
    * **platform:** Clearing email field after waitlisting the user email ([#481](#481)) ([256d659](256d659))
    * Remove project IDs from workspace role export data and update tests ([#448](#448)) ([8fdb328](8fdb328))
    * **web:** Configured extra check for waitlisted users already in the list and created toast message for them ([#492](#492)) ([2ddd0ef](2ddd0ef))
    * **web:** show the toast only when email add successfully ([#490](#490)) ([783c411](783c411))
    
    ### 🐛 Bug Fixes
    
    * **api,api-client:** Add environmentSlug in multiple places across the variable module ([#468](#468)) ([d970aff](d970aff))
    * **api:** Replace the id with slug in the global-search service ([#455](#455)) ([74804b1](74804b1))
    * **platform:** Fixed duplicate Google Logo UI fix  ([#450](#450)) ([fb0d6f7](fb0d6f7))
    * resolve footer website name cut-off or overlap issue ([#444](#444)) ([fe03ba2](fe03ba2))
    * **web:** Horizontal Scrolling issue on the website ([#440](#440)) ([655177b](655177b))
    
    ### 📚 Documentation
    
    * Add documentation for environment in CLI ([#462](#462)) ([dad7394](dad7394))
    * Add documentation for project in CLI ([#466](#466)) ([341fb32](341fb32))
    * Add documentation for scan in CLI ([#461](#461)) ([72281e6](72281e6))
    * Add documentation for workspace command ([#464](#464)) ([4aad8a2](4aad8a2))
    * Add instructions for resetting the local Prisma database ([#495](#495)) ([#501](#501)) ([b07ea17](b07ea17))
    * Added docker support documentation ([#465](#465)) ([bc04be4](bc04be4))
    * Added documentation for running the platform ([#473](#473)) ([8b8386b](8b8386b))
    * Added missing mappings to pages ([5de9fd8](5de9fd8))
    * Fix Documentation Hyperlink and update expired Discord invite link ([#496](#496)) ([5a10e39](5a10e39))
    * Updated CLI docs ([#460](#460)) ([c7e0f13](c7e0f13))
    
    ### 🔧 Miscellaneous Chores
    
    * Add more logging to Sentry init ([#470](#470)) ([de4925d](de4925d))
    * **api:** Optimise API docker image size ([#360](#360)) ([ea40dc1](ea40dc1))
    * **api:** Updated lockfile ([a968e78](a968e78))
    * **CI:** Add [secure] scan validation ([f441262](f441262))
    * **cli:** Update controller invocation in environment commands ([#477](#477)) ([596bd1a](596bd1a))
    * Minor changes to variables ([fe01ca6](fe01ca6))
    * **[secure]-scan:** Failing lint issues ([#507](#507)) ([48f45df](48f45df))
    * **[secure]-scan:** Formatted files ([5884833](5884833))
    * Update .env.example ([70ad4f7](70ad4f7))
    * Updated scripts ([9eb76a7](9eb76a7))
    * **web:** email validation ([#487](#487)) ([e8e737a](e8e737a))
    @rajdip-b
    Copy link
    Member

    🎉 This PR is included in version 2.6.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
    Projects
    None yet
    Development

    Successfully merging this pull request may close these issues.

    WEB: suggesting changes while adding waitlist
    3 participants