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鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: Create new team #1925

Merged
merged 7 commits into from
Jan 23, 2024
Merged

Conversation

ksushant6566
Copy link
Contributor

What does this PR do?

  • Allow users to delete their only team
  • Redirect user to /create-first-team if they have no team

Fixes #1816

Loom Video: https://www.loom.com/share/843147c04785426eb667b5a2c664bc00?sid=e1a9b526-fa43-4536-b446-4c6d10bd9784

How should this be tested?

  • Login
  • Delete all teams (You should be able to delete the last team )
  • You should get redirected to create-first-team
  • Create a team
  • You should get redirected to environments/environmentId/surveys

Checklist

Required

  • Filled out the "How to test" section in this PR
  • Read How we Code at Formbricks
  • Self-reviewed my own code
  • Commented on my code in hard-to-understand bits
  • Ran pnpm build
  • Checked for warnings, there are none
  • Removed all console.logs
  • Merged the latest changes from main onto my branch with git pull origin main
  • My changes don't cause any responsiveness issues
  • First PR at Formbricks? Please sign the CLA! Without it we wont be able to merge it 馃檹

Appreciated

  • If a UI change was made: Added a screen recording or screenshots to this PR
  • Updated the Formbricks Docs if changes were necessary

Copy link

vercel bot commented Jan 18, 2024

@ksushant6566 is attempting to deploy a commit to the formbricks Team on Vercel.

A member of the Team first needs to authorize it.

@github-actions github-actions bot added the bug Something isn't working label Jan 18, 2024
Copy link
Contributor

github-actions bot commented Jan 18, 2024

Thank you for following the naming conventions for pull request titles! 馃檹

@ksushant6566 ksushant6566 changed the title Create new team feat: Create new team Jan 18, 2024
Copy link
Member

@mattinannt mattinannt left a comment

Choose a reason for hiding this comment

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

@ksushant6566 thank you very much for taking up this feature! 馃挭 Works great! 馃殌馃敟
There is only one issue I've noticed:

  • The /create-first-team is also accessible for users who are not signed in. Please add a session check to the page. You can see the onboarding page for reference.
    Also for this it would be great if the page.tsx would be a server component (instead of a client component) and rather render a client-only child component.

Can you please make the changes for this? 馃
After this we are ready to merge the feature 馃挭

@ksushant6566
Copy link
Contributor Author

Sure, i'll make these changes :)

@ksushant6566
Copy link
Contributor Author

Hey @mattinannt , just pushed the requested changes:

  • /create-first-team is now only accessible to logged in users
  • page.tsx is now a server component, which renders a client-only component

Also I noticed an issue in the delete-team dialog
On "Enter" key down the page is refreshed without actually doing anything (this is because the primary button is outside the form).

loom: https://www.loom.com/share/e6f8412761eb41c2a81bd9831a208ab1?sid=0ee2d535-bca4-4d26-9ab5-1daabb6030d5

FIX:
For now I have added a preventDefault to disable page-refresh on "Enter" key down.

<form onSubmit={(e) => e.preventDefault()}>
  <label htmlFor="deleteTeamConfirmation">
    Please enter <b>{teamData?.name}</b> in the following field to confirm the definitive deletion of
    this team:
  </label>
  <Input
    value={inputValue}
    onChange={handleInputChange}
    placeholder={teamData?.name}
    className="mt-5"
    type="text"
    id="deleteTeamConfirmation"
    name="deleteTeamConfirmation"
  />
</form>

Copy link
Member

@mattinannt mattinannt left a comment

Choose a reason for hiding this comment

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

@ksushant6566 thanks a lot for the changes 馃挭 Looks great now! Ready to merge this 馃槉馃敟

@mattinannt mattinannt added this pull request to the merge queue Jan 23, 2024
@ksushant6566
Copy link
Contributor Author

Happy to help :)

Merged via the queue into formbricks:main with commit 78c2cf4 Jan 23, 2024
7 of 9 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[BUG] After you remove a user from their only team, they can no longer log in
2 participants