feat(api): Update slug generation method

[Nil2000]

User description

Description

Updated the keyshade/apps/api/src/common/slug-generator.ts

Fixes #416

Dependencies

Mention any dependencies/packages used

Future Improvements

Mention any improvements to be done in future related to any file/feature

Mentions

Mention and tag the people

Screenshots of relevant screens

Add screenshots of relevant screens

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

Bug fix

---

Description

• Updated the generateSlug function to accept a counter parameter, improving the uniqueness of generated slugs.
• Replaced random alphanumeric characters with a counter-based approach for slug generation.
• Implemented a loop to increment the counter when a slug conflict is detected, ensuring unique slugs for each entity type.

---

Changes walkthrough 📝

	Relevant files
Bug fix	slug-generator.ts Improve slug generation with counter for uniqueness            apps/api/src/common/slug-generator.ts Modified generateSlug function to include a counter for uniqueness. Updated slug generation logic to use a counter instead of random characters. Implemented a loop to increment the counter if a slug already exists. Enhanced slug uniqueness across different entity types. +12/-3

---

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

[codiumai-pr-agent-free]

PR Reviewer Guide 🔍

⏱️ Estimated effort to review: 2 🔵🔵⚪⚪⚪

🧪 No relevant tests

🔒 No security concerns identified

⚡ Key issues to review Potential Performance Issue The while loop for generating unique slugs might run indefinitely if there's a high number of collisions.

[codiumai-pr-agent-free]

PR Code Suggestions ✨

Category	Suggestion	Score
Best practice	✅ Improve code formatting for consistency Suggestion Impact: The suggestion to add spaces after commas in function parameters was implemented in the generateSlugName function. code diff: -const generateSlug = (name: string,counter:number): string => { +const generateSlugName = (name: string): string => { Add a space after the comma in the function parameters for consistent formatting and improved readability. apps/api/src/common/slug-generator.ts [11] -const generateSlug = (name: string,counter:number): string => { +const generateSlug = (name: string, counter: number): string => { Apply this suggestion Suggestion importance[1-10]: 8 Why: Adding a space after the comma in function parameters improves code readability and adheres to common formatting standards, making the code more consistent and easier to read.	8
	Improve variable naming for better code readability Consider using a more descriptive variable name instead of counter. For example, slugCounter or uniqueIdCounter would better convey its purpose in generating unique slugs. apps/api/src/common/slug-generator.ts [11] -const generateSlug = (name: string,counter:number): string => { +const generateSlug = (name: string, slugCounter: number): string => { Apply this suggestion Suggestion importance[1-10]: 7 Why: Using a more descriptive variable name like slugCounter enhances code readability and makes the purpose of the variable clearer, which is beneficial for maintainability.	7
Enhancement	Simplify the counter to string conversion Consider using a more efficient method to convert the counter to a string, such as counter.toString() instead of counter.toString(36). The base 36 conversion is unnecessary for a simple counter and may lead to confusion. apps/api/src/common/slug-generator.ts [23] -alphanumericName + '-' + counter.toString(36) +alphanumericName + '-' + counter.toString() Apply this suggestion Suggestion importance[1-10]: 6 Why: While converting the counter to a string using toString() is simpler and sufficient for this context, the base 36 conversion does not cause harm but may introduce unnecessary complexity. Simplifying it can reduce potential confusion.	6

[rajdip-b]

Hey man, I'm sorry but still this isn't what I wanted/meant. Please read carefully through the issue description. We don't want a while loop or any loops for that matter of fact. It will be a simple if else statement. Also, we don't want to generate slug names including the entity type. Entity types are used to assess the uniqueness of the slug in the table. You are just required to update the function listed in the issue, rest of the things are as per requirements.

[Nil2000]

Solution you mentioned

1. Convert it to lowercase
2. Replace spaces and underscores with hyphens
3. We query the database for this name using the regex pattern of ^port-[\d\w]*$, sorting the results in descending. The pattern translates to "any name that starts with port- and ends with a lowercase alphanumeric charset".
4. If one does not exist, we save the slug as port-0.
5. Although, if a name already exists, we increment the name and save it. Say the last name is port-44ab. Then the name of our new port variable would be port-44ac.

@rajdip-b Is this the one you are mentioning then? 👇

const generateSlug = async (name: string,entityType:string,prisma: PrismaService): Promise<string> => {
  // Convert to lowercase
  const lowerCaseName = name.trim().toLowerCase()

  // Replace spaces with hyphens
  const hyphenatedName = lowerCaseName.replace(/\s+/g, '-')

  // Replace all non-alphanumeric characters with hyphens
  const alphanumericName = hyphenatedName.replace(/[^a-zA-Z0-9-]/g, '-')

  // Returns the camelCase version of the entity type (WORKSPACE_ROLE -> workspaceRole)
  const entityTypeCamelCase = convertEntityTypeToCamelCase(entityType);
  
  //Querying the table to get slug (if available)
  const existingSlug=await prisma.entityTypeCamelCase.findMany({
      where: {
      slug: {
        startsWith: `${alphanumericName}-`
      }
    },
    orderBy: {
      slug: 'desc'
    }
  });

  if(existingSlug.length==0){
    return `${alphanumericName}-0`;
  }

  //Returns the slug with the highest ASCII value
  const lastSlug = existingSlug[0].slug;
  const suffix=lastSlug.substring(alphanumericName.length+1);
  const newSuffix=incrementSlugSuffix(suffix);

  return `${alphanumericName}-${newSuffix}`;
}

[rajdip-b]

1. You wont be including the entity type in the slug name. entity type would only be used to check for uniqueness of the slug in the particular table.
2. In your case you would need the entity name to query for the slug in the particular table. you can do so by modifying the check functions that are already present. right now they return a boolean value, you can update it to return the list of slug names.
3. The increment wont be made on the entire slug value, but just the appended part as mentioned in the issue.

Also, please go through the prisma tables first. there isnt no table named as entityTypeCamelCase

[Nil2000]

This function used for incrementing suffix received

const incrementSlugSuffix = (suffix: string): string => {
  const charset = '0123456789abcdefghijklmnopqrstuvwxyz'; 

  if (!suffix) {
    return '0';
  }

  let result = '';
  let carry = true;

  for (let i = suffix.length - 1; i >= 0; i--) {
    if (carry) {
      const currentChar = suffix[i];
      const index = charset.indexOf(currentChar);

      if (index === -1) {
        throw new Error(`Invalid character in slug suffix: ${currentChar}`);
      }
      const nextIndex = (index + 1) % charset.length;
      result = charset[nextIndex] + result;

      // Carry over if we wrapped around to '0'
      carry = nextIndex === 0;
    } else {
      // No carry, just append the remaining part of the suffix
      result = suffix[i] + result;
    }
  }

  if (carry) {
    result = '0' + result;
  }

  return result;
};

This is just to produce the slug name

const generateSlugName = (name: string): string => {
  // Convert to lowercase
  const lowerCaseName = name.trim().toLowerCase()

  // Replace spaces with hyphens
  const hyphenatedName = lowerCaseName.replace(/\s+/g, '-')

  // Replace all non-alphanumeric characters with hyphens
  const alphanumericName = hyphenatedName.replace(/[^a-zA-Z0-9-]/g, '-')

  return alphanumericName
}

The following part is to get list of slug containing the generatedSlug name. (Just showing the case for WorkspaceRole for now)

const getWorkspaceRoleIfSlugExists = async (
  slug: Workspace['slug'],
  prisma: PrismaService
): Promise<WorkspaceRole[]> => {
  const existingSlug=await prisma.workspaceRole.findMany({
      where: {
      slug: {
        startsWith: `${slug}-`
      }
    },
    orderBy: {
      slug: 'desc'
    }
  });
  return existingSlug;
}

Changes in generateEntitySlug function

export default async function generateEntitySlug(
  name: string,
  entityType:
    | 'WORKSPACE_ROLE'
    | 'WORKSPACE'
    | 'PROJECT'
    | 'VARIABLE'
    | 'SECRET'
    | 'INTEGRATION'
    | 'ENVIRONMENT'
    | 'API_KEY',
  prisma: PrismaService
): Promise<string> {
//Removing while loop
const baseSlug=generateSlugName(name)
switch (entityType) {
      case 'WORKSPACE_ROLE':
          const slugList=await getWorkspaceRoleIfSlugExists(baseSlug,prisma)
           let suffix=""
           if (slugList.length > 0) {
              //This takes only the suffix part (not the slug name part)
              suffix=slugList[0].slug.substring(baseSlug.length+1);
           }
           const newSuffix=incrementSlugSuffix(suffix)
           return `${baseSlug}-${newSuffix}`
...

Showing only for WORKSPACE_ROLE entity type only for now

Is this the one you are looking for?

[rajdip-b]

Yes, I think this will suffice. Although, do note that the current format of your code has a lot of redundancy. Ideally we would like to get rid of that. For example, instead of making the if check in the switch cases, you can have just one switch check in incrementSuffix.

[Nil2000]

Ah please don't push that now. I have not updated that thing in my code. I have just shown you my Ideation. Since things are fine, I will push the current change in the code. After that you can push/merge as per your needs.

[rajdip-b]

Ah boy, my bad! Although, there shouldn't be any conflicts with your code. I have made changes to other places.

[Nil2000]

instead of making the if check in the switch cases, you can have just one switch check in incrementSuffix

For this one I think this would do.

//Will define type or interface for the slugList types
const incrementSlugSuffix = (slugList:Workspace[]|WorkspaceRole[],baseSlug:string): string => {
  const charset = '0123456789abcdefghijklmnopqrstuvwxyz'; 

  let suffix = '';
  
  if(slugList.length>0){
    suffix = slugList[0].slug.substring(baseSlug.length + 1);
  }

  if (!suffix) {
    return `${baseSlug}-0`;
  }

  let result = '';
  let carry = true;

  for (let i = suffix.length - 1; i >= 0; i--) {
    if (carry) {
      const currentChar = suffix[i];
      const index = charset.indexOf(currentChar);

      if (index === -1) {
        throw new Error(`Invalid character in slug suffix: ${currentChar}`);
      }
      const nextIndex = (index + 1) % charset.length;
      result = charset[nextIndex] + result;

      // Carry over if we wrapped around to '0'
      carry = nextIndex === 0;
    } else {
      // No carry, just append the remaining part of the suffix
      result = suffix[i] + result;
    }
  }

  if (carry) {
    result = '0' + result;
  }

  return `${baseSlug}-${result}`;
};

Then the geenrateEntitySlug would be like this

export default async function generateEntitySlug(
  name: string,
  entityType:
    | 'WORKSPACE_ROLE'
    | 'WORKSPACE'
    | 'PROJECT'
    | 'VARIABLE'
    | 'SECRET'
    | 'INTEGRATION'
    | 'ENVIRONMENT'
    | 'API_KEY',
  prisma: PrismaService
): Promise<string> {
//Removing while loop
const baseSlug=generateSlugName(name)
switch (entityType) {
      case 'WORKSPACE_ROLE':
          const slugList=await getWorkspaceRoleIfSlugExists(baseSlug,prisma)
          return incrementSlugSuffix(slugList,baseSlug)
//other cases

[rajdip-b]

Yes, additionally, you can replace the type of slugList from slugList:Workspace[]|WorkspaceRole[] to slugList: string[].

  const existingSlug=await prisma.workspaceRole.findMany({
      where: {
      slug: {
        startsWith: `${slug}-`
      }
    },
    orderBy: {
      slug: 'desc'
    }
  });

And also, in this query, you can select only the first element using take. because thats the one that we need. this way we dont unnecessarily bloat the ram.

P.S. please take care of the spaces and newlines. Use pnpm format before you make the push. I feel prettier isnt configured to work in your IDE.

[rajdip-b]

all the tests are failing

[rajdip-b]

Can you also include a few tests? Unit tests would be fine. You would just need to mock the prisma client to send back dummy response. We would want to ensure that the changes in this file stay consistent.

[Nil2000]

Can you also include a few tests? Unit tests would be fine. You would just need to mock the prisma client to send back dummy response.

Ok let me try

[Nil2000]

describe('generateEntitySlug for each entity type', () => {
    it('should generate a unique slug for WORKSPACE_ROLE', async () => {
      const mockPrismaResult = [
        {
          id: 'role-id',
          slug: 'workspace-role-0',
          workspaceId: 'workspace-id'
        }
      ]

      prismaSingleton.workspaceRole.findMany.mockResolvedValue(mockPrismaResult)
      const slug = await generateEntitySlug(
        'Workspace Role',
        'WORKSPACE_ROLE',
        prisma
      )
      expect(slug).toBe('workspace-role-1')
    })

This always giving me error in mockPrismaResult. Why so? Do we require entire object will all the fields or few with slug will do

[rajdip-b]

You would need to override each of the prisma.<entity> functions that are being used by this function.

[Nil2000]

I tried singleton changing to any or partial. I was not able to do. I tried the following but was not able to fix prisma.workSpaceRole.findMany function. Everytime I get error. Can you tell me is it possible mock the data with just the slug field instead of entire field creation? Please share the any test file link if any created for this findMany

import { PrismaService } from '@/prisma/prisma.service'
import generateEntitySlug, {
  generateSlugName,
  incrementSlugSuffix
} from './slug-generator'
import { prismaSingleton } from '../../prisma-singleton'
import { Prisma, Workspace, WorkspaceRole } from '@prisma/client'
// Mock PrismaService
jest.mock('@/prisma/prisma.service')

describe('generateEntitySlug', () => {
  let prisma: PrismaService

  beforeEach(() => {
    prisma = new PrismaService()
  })

  describe('generateSlugName', () => {
    it('should convert name to slug format', () => {
      expect(generateSlugName('Hello World')).toBe('hello-world')
      expect(generateSlugName('Entity with 123')).toBe('entity-with-123')
      expect(generateSlugName('Special #Name!')).toBe('special--name-')
    })
  })

  describe('incrementSlugSuffix', () => {
    it('should return base slug with `-0` when no suffix is found', () => {
      const result = incrementSlugSuffix('', 'my-slug')
      expect(result).toBe('my-slug-0')
    })

    it('should increment suffix when found', () => {
      const result = incrementSlugSuffix('my-slug-0', 'my-slug')
      expect(result).toBe('my-slug-1')
    })

    it('should handle complex increment cases with carryover', () => {
      const result = incrementSlugSuffix('my-slug-z', 'my-slug')
      expect(result).toBe('my-slug-00')
    })
  })

  describe('generateEntitySlug for each entity type', () => {
    it('should generate a unique slug for WORKSPACE_ROLE', async () => {
      // Mocking the workspaceRole findMany method
      // ;(prisma.workspaceRole.findMany as jest.Mock).mockResolvedValue([
      //   {
      //     slug: 'workspace-role-0'
      //   }
      // ])
      jest
        .spyOn(prismaSingleton.workspaceRole, 'findMany')
        .mockResolvedValue([
          { slug: 'workspace-role-0' }
        ] as Partial<WorkspaceRole>[])

      const slug = await generateEntitySlug(
        'Workspace Role',
        'WORKSPACE_ROLE',
        prisma
      )
      expect(slug).toBe('workspace-role-1')
    })
  })
})

Will this do any help prisma/prisma#7084 (comment)

[rajdip-b]

I think you can use the mock-deep package. It's already added to our deps: https://github.com/marchaos/jest-mock-extended

[Nil2000]

I have been committing in a unconventional way. Everytime, I commit I get some errors using git cli. But I used the github in built features in vs code which also took some time. But I closed again and opened again and found that it's automatically committed. I ran Docker as well in the background. It says some localhost db auth error. Let me know if this is some major issue and if possible also tell How to fix this. So that I won't get any issues in future while commiting.

[rajdip-b]

I'm not sure what issues you are facing since I have never heard about them. We do have some issues with failing tests while comitting which you can bypass using the --no-verify flag while using git commit

[rajdip-b]

Looks good, I'll implement this once and then merge the code.

[rajdip-b]

@Nil2000 I have updated your branch to use psql raw regex. The tests are failing due to the lack of mock. Once you implement that, we would be good to go.

[rajdip-b]

🎉 This PR is included in version 2.5.0 🎉

The release is available on GitHub release

Your semantic-release bot 📦🚀