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(api): Create default workspace on user's creation #182

Merged
merged 1 commit into from
Apr 14, 2024

Conversation

rajdip-b
Copy link
Member

@rajdip-b rajdip-b commented Apr 14, 2024

User description

Description

Creates a default workspace when a user is created. Default workspaces have these features:

  • They have a isDefault flag set to true
  • Owners are not allowed to leave the workspace or transfer its ownership

Fixes #157


Type

enhancement


Description

  • Introduced functionality to create a default workspace when a new user is created.
  • Refactored user creation logic in AuthService and updated CreateUserDto to support optional fields.
  • Added a new function for workspace creation with default workspace support and detailed logging.
  • Enhanced user and workspace services to handle default workspaces, including preventing their deletion and ownership transfer.
  • Updated e2e tests to cover new functionality and added cleanup utilities.
  • Implemented a database migration to add an isDefault column to the Workspace table.
  • Simplified docker-compose test configuration by removing the version specification.

Changes walkthrough

Relevant files
Enhancement
9 files
auth.service.ts
Refactor User Creation Logic in AuthService                           

apps/api/src/auth/service/auth.service.ts

  • Refactored user creation logic by removing the createUser method and
    using the createUser function directly.
  • +8/-20   
    create-user.ts
    Implement Default Workspace Creation on New User Signup   

    apps/api/src/common/create-user.ts

  • Added functionality to create a default workspace when a new user is
    created.
  • Enhanced logging to include user and workspace creation details.
  • +26/-3   
    create-workspace.ts
    Add Functionality to Create Default Workspaces                     

    apps/api/src/common/create-workspace.ts

  • Introduced a new function to create workspaces with the ability to
    mark them as default.
  • Added detailed logging for workspace creation.
  • +95/-0   
    create.user.ts
    Update CreateUserDto to Support Optional Fields                   

    apps/api/src/user/dto/create.user/create.user.ts

  • Made name, profilePictureUrl, isActive, isOnboardingFinished, and
    isAdmin fields optional in CreateUserDto.
  • +5/-5     
    user.service.ts
    Enhance User Deletion to Remove Default Workspace               

    apps/api/src/user/service/user.service.ts

  • Added logic to delete a user's default workspace upon account
    deletion.
  • Enhanced logging to include user deletion details.
  • +10/-0   
    create.workspace.ts
    Update CreateWorkspace DTO to Support Optional Fields       

    apps/api/src/workspace/dto/create.workspace/create.workspace.ts

  • Made description and approvalEnabled fields optional in
    CreateWorkspace DTO.
  • +2/-2     
    workspace.service.ts
    Refactor Workspace Creation and Protect Default Workspaces

    apps/api/src/workspace/service/workspace.service.ts

  • Refactored workspace creation to use the createWorkspace function.
  • Added checks to prevent ownership transfer and deletion of default
    workspaces.
  • +17/-79 
    migration.sql
    Database Migration to Add isDefault Column to Workspace   

    apps/api/src/prisma/migrations/20240413155927_add_default_workspace/migration.sql

  • Added a migration to include an isDefault column in the Workspace
    table.
  • +2/-0     
    schema.prisma
    Update Prisma Schema to Include isDefault in Workspace Model

    apps/api/src/prisma/schema.prisma

  • Updated the Prisma schema to include an isDefault field in the
    Workspace model.
  • +1/-0     
    Tests
    2 files
    user.e2e.spec.ts
    Refactor User E2E Tests and Add Default Workspace Creation Test

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

  • Refactored user creation in tests to use the UserService.
  • Added cleanup utility to manage test data.
  • Implemented a test to verify default workspace creation for new users.

  • +70/-20 
    workspace.e2e.spec.ts
    Add E2E Tests for Default Workspace Restrictions                 

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

  • Added tests to ensure default workspaces cannot be deleted or have
    their ownership transferred.
  • +129/-37
    Configuration changes
    1 files
    docker-compose-test.yml
    Simplify docker-compose Test Configuration                             

    docker-compose-test.yml

  • Removed version specification from the docker-compose test
    configuration.
  • +0/-3     

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

    Copy link

    sonarcloud bot commented Apr 14, 2024

    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

    @codiumai-pr-agent-free codiumai-pr-agent-free bot added the type: enhancement New feature or request label Apr 14, 2024
    Copy link
    Contributor

    PR Description updated to latest commit (a94535b)

    Copy link
    Contributor

    PR Review

    ⏱️ Estimated effort to review [1-5]

    4, because the PR introduces significant changes across multiple files, including backend logic for user and workspace management, database schema updates, and comprehensive test coverage. The complexity of the changes and the importance of ensuring no regressions in user creation and workspace management workflows justify a thorough review.

    🧪 Relevant tests

    Yes

    🔍 Possible issues

    Missing Validation: The createWorkspace function might not validate the CreateWorkspace DTO fully, potentially leading to issues with incomplete or invalid workspace creation requests.

    Error Handling: The error handling in the new user and workspace creation flows could be improved. Specifically, there's no clear rollback mechanism or transaction handling if part of the process fails (e.g., user is created but default workspace creation fails).

    Performance Concerns: The deletion logic in UserService.deleteUserById method deletes default workspaces before deleting the user. This could lead to performance issues or inconsistencies if the workspace deletion fails but the user deletion proceeds.

    🔒 Security concerns

    No


    ✨ Review tool usage guide:

    Overview:
    The review tool scans the PR code changes, and generates a PR review which includes several types of feedbacks, such as possible PR issues, security threats and relevant test in the PR. More feedbacks can be added by configuring the tool.

    The tool can be triggered automatically every time a new PR is opened, or can be invoked manually by commenting on any PR.

    • When commenting, to edit configurations related to the review tool (pr_reviewer section), use the following template:
    /review --pr_reviewer.some_config1=... --pr_reviewer.some_config2=...
    
    [pr_reviewer]
    some_config1=...
    some_config2=...
    

    See the review usage page for a comprehensive guide on using this tool.

    Copy link
    Contributor

    PR Code Suggestions

    CategorySuggestions                                                                                                                                                       
    Enhancement
    Add a check for workspace name uniqueness in the workspace creation process.

    Consider implementing a check for the uniqueness of the workspace name within the
    createWorkspace function. This ensures that each workspace has a unique name, preventing
    potential conflicts or confusion.

    apps/api/src/workspace/service/workspace.service.ts [55]

    +// Ensure the workspace name is unique
    +const existingWorkspace = await this.prisma.workspace.findUnique({
    +  where: {
    +    name: dto.name,
    +  },
    +});
    +if (existingWorkspace) {
    +  throw new ConflictException('A workspace with this name already exists');
    +}
     return await createWorkspace(user, dto, this.prisma)
     
    Add more assertions for the properties of the default workspace in tests.

    To ensure the integrity of your tests, consider adding assertions to check the properties
    of the workspace created by default for a new user. This will help verify that the
    workspace is set up correctly and has the expected default values.

    apps/api/src/workspace/workspace.e2e.spec.ts [108-112]

     expect(createUserResponse.defaultWorkspace).toBeDefined()
     expect(createUserResponse.defaultWorkspace.name).toEqual('My Workspace')
     expect(createUserResponse.defaultWorkspace.isDefault).toEqual(true)
    +// Additional assertions for default workspace properties
    +expect(createUserResponse.defaultWorkspace.approvalEnabled).toEqual(false)
    +expect(createUserResponse.defaultWorkspace.isFreeTier).toEqual(true)
     
    Add error logging for workspace creation failures.

    Use the Logger instance for error logging as well, especially in critical operations like
    user and workspace creation, to ensure all errors are properly logged for debugging and
    monitoring.

    apps/api/src/common/create-user.ts [30-34]

    -const workspace = await createWorkspace(
    -    user,
    -    { name: 'My Workspace' },
    -    prisma,
    -    true
    -)
    +let workspace;
    +try {
    +    workspace = await createWorkspace(
    +        user,
    +        { name: 'My Workspace' },
    +        prisma,
    +        true
    +    )
    +} catch (error) {
    +    logger.error(`Failed to create default workspace for user ${user.id}: ${error.message}`);
    +    throw error; // Rethrow the error after logging
    +}
     
    Maintainability
    Refactor the check for default workspace into a private method to reduce code duplication.

    To improve the maintainability and readability of the code, consider refactoring the
    repeated logic for checking if a workspace is the default workspace and throwing a
    BadRequestException into a private method within the WorkspaceService.

    apps/api/src/workspace/service/workspace.service.ts [233-235]

    -if (workspace.isDefault) {
    -  throw new BadRequestException(
    -    `You cannot delete the default workspace ${workspace.name} (${workspace.id})`
    -  );
    +this.ensureNotDefaultWorkspace(workspace);
    +
    +// Inside WorkspaceService class
    +private ensureNotDefaultWorkspace(workspace: Workspace) {
    +  if (workspace.isDefault) {
    +    throw new BadRequestException(
    +      `Operation not allowed on the default workspace ${workspace.name} (${workspace.id})`
    +    );
    +  }
     }
     
    Reliability
    Add error handling around the workspace creation transaction for improved reliability.

    For better error handling and user feedback, consider adding a try-catch block around the
    workspace creation transaction. This way, you can catch any errors that occur during the
    creation process and return a more user-friendly error message.

    apps/api/src/common/create-workspace.ts [70-73]

    -const result = await prisma.$transaction([
    -  createNewWorkspace,
    -  assignOwnership
    -])
    +let result;
    +try {
    +  result = await prisma.$transaction([
    +    createNewWorkspace,
    +    assignOwnership
    +  ]);
    +} catch (error) {
    +  logger.error(`Failed to create workspace: ${error.message}`);
    +  throw new InternalServerErrorException('Failed to create workspace');
    +}
     
    Best practice
    Implement a comprehensive cleanup function for tests to ensure a clean state.

    To improve the cleanup process in your tests, consider implementing a more comprehensive
    cleanup function that not only deletes users but also associated workspaces and other
    related entities. This ensures a clean state for each test run and prevents potential
    interference between tests.

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

    -await cleanUp(prisma)
    +// Improved cleanup function that handles users, workspaces, and other entities
    +async function enhancedCleanUp(prisma: PrismaService) {
    +  await prisma.workspace.deleteMany({});
    +  await prisma.user.deleteMany({});
    +  // Add other necessary cleanup steps here
    +}
    +await enhancedCleanUp(prisma)
     
    Wrap user and workspace creation in a transaction for data consistency.

    Consider wrapping the user and workspace creation logic inside a transaction. This ensures
    that if either operation fails, the database remains in a consistent state without partial
    updates.

    apps/api/src/common/create-user.ts [18-35]

    -const user = await prisma.user.create({
    -    data: {
    -        email: dto.email,
    -        name: dto.name,
    -        isOnboardingFinished: dto.isOnboardingFinished ?? false
    -    }
    +const result = await prisma.$transaction(async (prisma) => {
    +    const user = await prisma.user.create({
    +        data: {
    +            email: dto.email,
    +            name: dto.name,
    +            isOnboardingFinished: dto.isOnboardingFinished ?? false
    +        }
    +    })
    +    const workspace = await createWorkspace(
    +        user,
    +        { name: 'My Workspace' },
    +        prisma,
    +        true
    +    )
    +    return { user, workspace };
     })
    -const workspace = await createWorkspace(
    -    user,
    -    { name: 'My Workspace' },
    -    prisma,
    -    true
    -)
     
    Add error handling for user and workspace deletion operations.

    Ensure to handle potential exceptions when deleting a user or their workspace, especially
    since these operations can fail due to foreign key constraints or other database rules.

    apps/api/src/user/service/user.service.ts [111-123]

    -await this.prisma.workspace.deleteMany({
    -    where: {
    -        ownerId: userId,
    -        isDefault: true
    -    }
    -})
    -await this.prisma.user.delete({
    -    where: {
    -        id: userId
    -    }
    -})
    +try {
    +    await this.prisma.workspace.deleteMany({
    +        where: {
    +            ownerId: userId,
    +            isDefault: true
    +        }
    +    })
    +    await this.prisma.user.delete({
    +        where: {
    +            id: userId
    +        }
    +    })
    +} catch (error) {
    +    this.log.error(`Failed to delete user ${userId}: ${error.message}`);
    +    throw new Error(`Deletion failed for user ${userId}`);
    +}
     
    Validate user creation input data to ensure data integrity.

    Validate the input data before creating a new user to prevent invalid data from being
    inserted into the database.

    apps/api/src/auth/service/auth.service.ts [160-166]

    +// Assume validateCreateUserInput is a function that validates the input data
    +const isValid = validateCreateUserInput({ email, name, profilePictureUrl });
    +if (!isValid) {
    +    throw new Error('Invalid input data');
    +}
     user = await createUser(
         {
             email,
             name,
             profilePictureUrl
         },
         this.prisma
     )
     
    Performance
    Add an index to the isDefault column for better query performance.

    Consider adding an index to the isDefault column in the Workspace table to improve query
    performance, especially if you frequently query for default workspaces.

    apps/api/src/prisma/schema.prisma [421]

    -isDefault       Boolean  @default(false)
    +isDefault       Boolean  @default(false) @index
     

    ✨ Improve tool usage guide:

    Overview:
    The improve tool scans the PR code changes, and automatically generates suggestions for improving the PR code. The tool can be triggered automatically every time a new PR is opened, or can be invoked manually by commenting on a PR.

    • When commenting, to edit configurations related to the improve tool (pr_code_suggestions section), use the following template:
    /improve --pr_code_suggestions.some_config1=... --pr_code_suggestions.some_config2=...
    
    [pr_code_suggestions]
    some_config1=...
    some_config2=...
    

    See the improve usage page for a comprehensive guide on using this tool.

    Copy link

    codecov bot commented Apr 14, 2024

    Codecov Report

    All modified and coverable lines are covered by tests ✅

    Project coverage is 93.15%. Comparing base (7bb3d21) to head (a94535b).
    Report is 45 commits behind head on develop.

    Additional details and impacted files
    @@             Coverage Diff              @@
    ##           develop     #182       +/-   ##
    ============================================
    + Coverage    62.20%   93.15%   +30.94%     
    ============================================
      Files           76       97       +21     
      Lines         1503     2148      +645     
      Branches       260      401      +141     
    ============================================
    + Hits           935     2001     +1066     
    + Misses         568      147      -421     
    Flag Coverage Δ
    api-e2e-tests 93.15% <100.00%> (+30.94%) ⬆️

    Flags with carried forward coverage won't be shown. Click here to find out more.

    ☔ View full report in Codecov by Sentry.
    📢 Have feedback on the report? Share it here.

    @rajdip-b rajdip-b merged commit 3dc0c4c into develop Apr 14, 2024
    9 checks passed
    @rajdip-b rajdip-b deleted the feat/default-workspace branch May 6, 2024 19:39
    rajdip-b pushed a commit that referenced this pull request May 12, 2024
    ## [1.3.0](v1.2.0...v1.3.0) (2024-05-12)
    
    ### 🚀 Features
    
    * Add approval support ([#158](#158)) ([e09ae60](e09ae60))
    * **api:** Add configuration live update support ([#181](#181)) ([f7d6684](f7d6684))
    * **api:** Add feature to export data of a workspace ([#152](#152)) ([46833aa](46833aa))
    * **api:** Add Integration support ([#203](#203)) ([f1ae87e](f1ae87e))
    * **api:** Add note to [secure] and variable ([#151](#151)) ([2e62351](2e62351))
    * **api:** Add OAuth redirection and polished authentication ([#212](#212)) ([d2968bc](d2968bc))
    * **api:** Add support for storing and managing variables ([#149](#149)) ([963a8ae](963a8ae))
    * **api:** Added GitLab OAuth ([#188](#188)) ([4d3bbe4](4d3bbe4))
    * **api:** Added validation for reason field ([#190](#190)) ([90b8ff2](90b8ff2))
    * **api:** Create default workspace on user's creation ([#182](#182)) ([3dc0c4c](3dc0c4c))
    * **api:** Reading `port` Dynamically ([#170](#170)) ([fd46e3e](fd46e3e))
    * **auth:** Add Google OAuth ([#156](#156)) ([cf387ea](cf387ea))
    * **web:** Added waitlist ([#168](#168)) ([1084c77](1084c77))
    * **web:** Landing revamp ([#165](#165)) ([0bc723b](0bc723b))
    
    ### 🐛 Bug Fixes
    
    * **web:** alignment issue in “Collaboration made easy” section ([#178](#178)) ([df5ca75](df5ca75))
    * **workspace:** delete duplicate tailwind config ([99d922a](99d922a))
    
    ### 📚 Documentation
    
    * add contributor list ([f37569a](f37569a))
    * Add integration docs ([#204](#204)) ([406ddb7](406ddb7))
    * Added integration docs to gitbook summary ([ab37530](ab37530))
    * **api:** Add swagger docs of API key controller ([#167](#167)) ([2910476](2910476))
    * **api:** Add swagger docs of User Controller ([#166](#166)) ([fd59522](fd59522))
    * fix typo in environment-variables.md ([#163](#163)) ([48294c9](48294c9))
    * Remove supabase from docs ([#169](#169)) ([eddbce8](eddbce8))
    * **setup:** replace NX with Turbo in setup instructions ([#175](#175)) ([af8a460](af8a460))
    * Update README.md ([b59f16b](b59f16b))
    * Update running-the-api.md ([177dbbf](177dbbf))
    * Update running-the-api.md ([#193](#193)) ([3d5bcac](3d5bcac))
    
    ### 🔧 Miscellaneous Chores
    
    * Added lockfile ([60a3b9b](60a3b9b))
    * Added lockfile ([6bb512c](6bb512c))
    * **api:** Added type inference and runtime validation to `process.env` ([#200](#200)) ([249e07d](249e07d))
    * **api:** Fixed prisma script env errors ([#209](#209)) ([8762354](8762354))
    * **API:** Refactor authority check functions in API ([#189](#189)) ([e9d710d](e9d710d))
    * **api:** Refactor user e2e tests ([b38d45a](b38d45a))
    * **ci:** Disabled api stage release ([97877c4](97877c4))
    * **ci:** Update stage deployment config ([868a6a1](868a6a1))
    * **codecov:** update api-e2e project coverage ([1e90d7e](1e90d7e))
    * **dockerfile:** Fixed web dockerfile ([6134bb2](6134bb2))
    * **docker:** Optimized web Dockerfile to reduct image size ([#173](#173)) ([444286a](444286a))
    * **release:** Downgraded package version ([c173fee](c173fee))
    * **release:** Fix failing release ([#213](#213)) ([40f64f3](40f64f3))
    * **release:** Install pnpm ([1081bea](1081bea))
    * **release:** Updated release commit ([b8958e7](b8958e7))
    * **release:** Updated release commit ([e270eb8](e270eb8))
    * Update deprecated husky Install command ([#202](#202)) ([e61102c](e61102c))
    * Upgrade @million/lint from 0.0.66 to 0.0.73 ([#172](#172)) ([dd43ed9](dd43ed9))
    * **web:** Updated fly memory config ([4debc66](4debc66))
    
    ### 🔨 Code Refactoring
    
    * **api:** Made events central to workspace ([#159](#159)) ([9bc00ae](9bc00ae))
    * **api:** Migrated to cookie based authentication ([#206](#206)) ([ad6911f](ad6911f))
    * **monorepo:** Migrate from nx to turbo ([#153](#153)) ([88b4b00](88b4b00))
    @rajdip-b
    Copy link
    Member Author

    🎉 This PR is included in version 1.3.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
    Labels
    type: enhancement New feature or request
    Projects
    None yet
    Development

    Successfully merging this pull request may close these issues.

    Create default workspace on user creation
    1 participant