Skip to content

Task #240139 Merge lateMarkingForSelfAttendance to release-1.0.1 branch of userservice#266

Open
Tusharmahajan12 wants to merge 65 commits intotekdi:release-1.0.1from
Tusharmahajan12:new_useroblf
Open

Task #240139 Merge lateMarkingForSelfAttendance to release-1.0.1 branch of userservice#266
Tusharmahajan12 wants to merge 65 commits intotekdi:release-1.0.1from
Tusharmahajan12:new_useroblf

Conversation

@Tusharmahajan12
Copy link
Collaborator

@Tusharmahajan12 Tusharmahajan12 commented May 5, 2025

Summary by CodeRabbit

  • New Features

    • Added a PATCH API endpoint to update user details by name.
    • Introduced automated deployment workflow for a specific server branch.
    • Added support for time and date field validation.
  • Enhancements

    • User creation now supports automatic username and password generation if not provided.
    • Improved cohort creation by making several fields optional and adding a flexible params field.
    • Cohort search offset is now optional in API documentation.
    • User creation DTO now allows optional username and password fields.
  • Bug Fixes

    • Adjusted cohort search to handle unlimited results when no limit is specified.
  • Refactor

    • Updated user update DTO structure and service interfaces for consistency and flexibility.
  • Chores

    • Removed unused deployment workflows and logging sidecar container from backend deployment.

AbhilashKD and others added 30 commits August 1, 2024 11:32
Issue #000 fix: validation to mark the attendance
…microservice into CreatedAndUpdatedSatusOblf
…atusOblf

Send createdAt and UpdatedAt fields value in responce
Set 'Not Found' Message if User is Not Assigned to a Cohort
Sourav-Tekdi and others added 26 commits September 30, 2024 17:15
Expand the API Limit for Cohort Searches
Show all schools in alphabetical order
In cohort name add pre fix 0 befoe tinning
corrected the limit in search cohort
@coderabbitai
Copy link

coderabbitai bot commented May 5, 2025

Walkthrough

This set of changes introduces new deployment automation for a specific server, removes two previous CI/CD workflows, and updates deployment configurations. In the backend, several DTOs and entity classes are modified to make fields optional, add new properties, and enhance validation. User management logic is extended with dynamic username/password generation and a new update-by-name feature, which is exposed via a new PATCH endpoint. Cohort-related classes gain new optional parameters and validation. Additionally, two new field validator classes for date and time are added. The codebase reflects a shift towards more flexible user and cohort creation, improved validation, and streamlined deployment processes.

Changes

File(s) Change Summary
.github/workflows/Prod-Deployment.yaml, .github/workflows/tekdi-server-deployment.yaml Deleted GitHub Actions workflows for production and Tekdi server deployments.
.github/workflows/oblf-deployment.yaml Added a new GitHub Actions workflow for automated deployment to the OBLF server via SSH.
manifest/backend.yaml Removed the cloudwatch-logs-agent container from the backend deployment manifest.
src/adapters/postgres/cohort-adapter.ts Modified logic for handling the limit parameter in cohort search, removing the default limit and making result slicing conditional.
src/adapters/postgres/user-adapter.ts Added dynamic username/password generation, new update-by-name method, and related helpers. Updated createUser and createUserInDatabase signatures and logic.
src/adapters/userservicelocator.ts Updated interface method signatures for user operations, added updateUserByName, and changed parameter types.
src/cohort/dto/cohort-create.dto.ts Made several properties optional, added enum and boolean validation, and introduced a new optional params property.
src/cohort/dto/cohort-search.dto.ts Changed offset property to be optional in API documentation.
src/cohort/entities/cohort.entity.ts Added an optional params JSONB column to the Cohort entity.
src/fields/fieldValidators/fieldTypeClasses.ts Added new field validator classes: TimeField and DateField for validating time and date formats.
src/user/dto/user-create.dto.ts Made username and password properties optional and removed required validation.
src/user/dto/user-update.dto.ts Replaced nested userData property with a simple name string in UserUpdateDTO.
src/user/user.controller.ts Added a new PATCH endpoint updateByName/:name for updating users by name.

Sequence Diagram(s)

sequenceDiagram
    participant Client
    participant UserController
    participant UserAdapter
    participant PostgresUserService

    Client->>UserController: PATCH /updateByName/:name (body: UserUpdateDTO)
    UserController->>UserAdapter: updateUserByName(userUpdateDto, response)
    UserAdapter->>PostgresUserService: updateUserByName(userUpdateDto, response)
    PostgresUserService->>PostgresUserService: Find users by name
    PostgresUserService->>PostgresUserService: Update editable custom fields
    PostgresUserService-->>UserAdapter: Return update result
    UserAdapter-->>UserController: Return result
    UserController-->>Client: Response
Loading
sequenceDiagram
    participant Client
    participant UserController
    participant UserAdapter
    participant PostgresUserService

    Client->>UserController: POST /users (body: UserCreateDto)
    UserController->>UserAdapter: createUser(request, userCreateDto, response)
    UserAdapter->>PostgresUserService: createUser(request, userCreateDto, response)
    PostgresUserService->>PostgresUserService: generateUserNamePassword(userCreateDto)
    PostgresUserService->>PostgresUserService: formatUsername(name)
    PostgresUserService->>PostgresUserService: createUserInDatabase(request, userCreateDto, response)
    PostgresUserService-->>UserAdapter: Return created user
    UserAdapter-->>UserController: Return result
    UserController-->>Client: Response
Loading
✨ Finishing Touches
  • 📝 Generate Docstrings

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share
🪧 Tips

Chat

There are 3 ways to chat with CodeRabbit:

  • Review comments: Directly reply to a review comment made by CodeRabbit. Example:
    • I pushed a fix in commit <commit_id>, please review it.
    • Generate unit testing code for this file.
    • Open a follow-up GitHub issue for this discussion.
  • Files and specific lines of code (under the "Files changed" tab): Tag @coderabbitai in a new review comment at the desired location with your query. Examples:
    • @coderabbitai generate unit testing code for this file.
    • @coderabbitai modularize this function.
  • PR comments: Tag @coderabbitai in a new PR comment to ask questions about the PR branch. For the best results, please provide a very specific query, as very limited context is provided in this mode. Examples:
    • @coderabbitai gather interesting stats about this repository and render them as a table. Additionally, render a pie chart showing the language distribution in the codebase.
    • @coderabbitai read src/utils.ts and generate unit testing code.
    • @coderabbitai read the files in the src/scheduler package and generate a class diagram using mermaid and a README in the markdown format.
    • @coderabbitai help me debug CodeRabbit configuration file.

Support

Need help? Create a ticket on our support page for assistance with any issues or questions.

Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments.

CodeRabbit Commands (Invoked using PR comments)

  • @coderabbitai pause to pause the reviews on a PR.
  • @coderabbitai resume to resume the paused reviews.
  • @coderabbitai review to trigger an incremental review. This is useful when automatic reviews are disabled for the repository.
  • @coderabbitai full review to do a full review from scratch and review all the files again.
  • @coderabbitai summary to regenerate the summary of the PR.
  • @coderabbitai generate docstrings to generate docstrings for this PR.
  • @coderabbitai generate sequence diagram to generate a sequence diagram of the changes in this PR.
  • @coderabbitai resolve resolve all the CodeRabbit review comments.
  • @coderabbitai configuration to show the current CodeRabbit configuration for the repository.
  • @coderabbitai help to get help.

Other keywords and placeholders

  • Add @coderabbitai ignore anywhere in the PR description to prevent this PR from being reviewed.
  • Add @coderabbitai summary to generate the high-level summary at a specific location in the PR description.
  • Add @coderabbitai anywhere in the PR title to generate the title automatically.

Documentation and Community

  • Visit our Documentation for detailed information on how to use CodeRabbit.
  • Join our Discord Community to get help, request features, and share feedback.
  • Follow us on X/Twitter for updates and announcements.

@sonarqubecloud
Copy link

sonarqubecloud bot commented May 5, 2025

Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 11

🔭 Outside diff range comments (1)
src/cohort/dto/cohort-search.dto.ts (1)

169-175: 🛠️ Refactor suggestion

Add @IsOptional() decorator to match @ApiPropertyOptional

While you've marked offset as optional in the API documentation with @ApiPropertyOptional, it's still required in validation because of @IsNumber() without @IsOptional(). For consistency, add the @IsOptional() decorator to make it truly optional.

  @ApiPropertyOptional({
    type: Number,
    description: "Offset",
  })
+ @IsOptional()
  @IsNumber()
  offset: number;
🧹 Nitpick comments (18)
.github/workflows/oblf-deployment.yaml (4)

1-5: Consider adding a manual trigger (workflow_dispatch) for on-demand deployments
Right now the workflow only runs on pushes to lateMarkingForSelfAttendance. It can be helpful to allow manual invocations for emergency or out-of-band deployments.

Example diff:

 on:
   push:
     branches:
       - lateMarkingForSelfAttendance
+  workflow_dispatch:

7-8: Cleanup or enable the commented conditional
The job-level #if: github.event.pull_request.merged == true line is currently disabled. Either remove it or re-enable it to prevent accidental deployments on non-merge pushes.


9-10: Add concurrency controls to avoid overlapping deployments
Without a concurrency group, multiple pushes in quick succession could trigger parallel deploys. Consider adding:

  concurrency: oblf-backend-deploy-${{ github.ref }}

under the job definition to cancel in-progress runs when a newer one starts.


19-26: Improve script robustness with fail-fast and safer file handling

  • Add set -euo pipefail for immediate failure on errors or undefined variables.
  • Use rm -f .env to simplify cleanup.
  • Use printf to preserve newlines if your secret contains multi-line values.
 script: |
-  cd ${{ secrets.TARGET_DIR_OBLF }}
-  if [ -f .env ]; then
-    rm .env
-  fi
-  echo '${{ secrets.OBLF_ENV }}' > .env
-  ls -ltra
-  ./deploy.sh
+  set -euo pipefail
+  cd "${{ secrets.TARGET_DIR_OBLF }}"
+  rm -f .env
+  printf '%s\n' "${{ secrets.OBLF_ENV }}" > .env
+  ls -ltra
+  ./deploy.sh
src/cohort/entities/cohort.entity.ts (3)

64-66: Use double quotes for consistency with TypeORM decorators

The @Column decorator is using single quotes for the type 'jsonb', but all other decorators in this file use double quotes. For consistency with Google JavaScript style guide and the rest of the codebase, use double quotes instead.

- @Column('jsonb', { nullable: true })
+ @Column("jsonb", { nullable: true })
  params?: object;
🧰 Tools
🪛 ESLint

[error] 64-64: Replace 'jsonb' with "jsonb"

(prettier/prettier)


[error] 65-66: Delete

(prettier/prettier)


64-66: Consider adding a default value for the params object

Adding a default value for the optional params property would prevent potential null/undefined issues when this object is accessed in your application logic.

- @Column("jsonb", { nullable: true })
+ @Column("jsonb", { nullable: true, default: {} })
  params?: object;
🧰 Tools
🪛 ESLint

[error] 64-64: Replace 'jsonb' with "jsonb"

(prettier/prettier)


[error] 65-66: Delete

(prettier/prettier)


65-66: Remove extra line break after property definition

For consistent formatting throughout your codebase, remove the extra line break after the params property definition.

  @Column('jsonb', { nullable: true })
  params?: object;
-
🧰 Tools
🪛 ESLint

[error] 65-66: Delete

(prettier/prettier)

src/user/user.controller.ts (3)

199-199: Remove debug console.log statement

Debug console logs should not be left in production code. Remove this console.log statement.

- console.log(userUpdateDto);

201-201: Uncomment or remove commented code

This line appears to be an incomplete/commented implementation for setting the tenantId. Either implement this feature properly or remove the commented code to maintain clean codebase.

- // userDto.tenantId = headers["tenantid"];
+ // If needed:
+ // userUpdateDto.tenantId = headers["tenantid"];

195-197: Add Swagger documentation for path parameter

The name path parameter lacks Swagger documentation. Add @ApiParam decorator to document it properly for API consumers.

+ @ApiParam({
+   name: 'name',
+   description: 'User name to update',
+   required: true,
+   type: String
+ })
  public async updateUserByName(
    @Headers() headers,
    @Param("name") name: string,
src/adapters/postgres/cohort-adapter.ts (3)

811-813: Inconsistent formatting and conditional slicing implementation

The conditional slicing logic is correct, but the formatting is inconsistent with project style guidelines.

-         // const userExistCohortGroup = data.slice(offset, offset + limit);
-         const userExistCohortGroup = limit ? data.slice(offset, offset + limit) : data;
+         // Only slice the data if a limit is provided
+         const userExistCohortGroup = limit ? data.slice(offset, offset + limit) : data;
🧰 Tools
🪛 ESLint

[error] 811-811: Delete ·

(prettier/prettier)


[error] 812-812: Replace ·const·userExistCohortGroup·=·limit·?·data.slice(offset,·offset·+·limit)·:·data; with const·userExistCohortGroup·=·limit⏎··········?·data.slice(offset,·offset·+·limit)

(prettier/prettier)


[error] 813-813: Insert :·data;⏎

(prettier/prettier)


883-884: Inconsistent formatting in conditional slicing

The conditional slicing logic is correct, but the commented code and formatting is inconsistent.

-        const cohortData = limit ? data.slice(offset, offset + limit) : data;
-        //const cohortData = data.slice(offset, offset + limit);
+        // Only slice the data if a limit is provided
+        const cohortData = limit ? data.slice(offset, offset + limit) : data;

859-860: Spacing issue in conditional check

There's no space between the && operator and the ! operator.

-          getCohortIdUsingCustomFields.length > 0 && !whereClause['cohortId']
+          getCohortIdUsingCustomFields.length > 0 && !whereClause['cohortId']
🧰 Tools
🪛 ESLint

[error] 859-859: Replace ·!whereClause['cohortId' with ⏎··········!whereClause["cohortId"

(prettier/prettier)

src/adapters/userservicelocator.ts (1)

20-20: Method signature changed to use generic 'any' type

Changed the updateUser method to use 'any' types instead of specific DTOs, which reduces type safety.

Consider maintaining type safety by using specific types:

-  updateUser(userDto?: any, response?: any);
+  updateUser(userDto?: UserUpdateDTO, response?: Response);
src/cohort/dto/cohort-create.dto.ts (1)

131-137: Consider adding validation to the params object.

While you've added API documentation for the params object, it lacks validation. Consider adding @isObject() or another appropriate validation decorator to ensure the input is properly validated.

@ApiPropertyOptional({
  type: Object,
  description: "Cohort attendance params",
  default: {},
})
@Expose()
+@IsOptional()
+@IsObject()
params: object;
src/adapters/postgres/user-adapter.ts (3)

1667-1678: Improve username formatting for edge cases.

The formatUsername method doesn't handle edge cases like very short names or special characters.

async formatUsername(name: string) {
  // Remove prefixes (Dr., Mr., Mrs., etc.)
  const nameWithoutPrefix = name.replace(/^(Dr\.|Mr\.|Mrs\.)\s+/i, '');

  // Split the name by space
  const nameParts = nameWithoutPrefix.split(' ');

+ // Handle case when name is too short after splitting
+ if (nameParts.length === 0 || nameParts.some(part => part.length === 0)) {
+   return `user_${Date.now().toString(36)}`;
+ }

  // Convert the name to lowercase and join with an underscore
- const formattedName = nameParts.map(part => part.toLocaleLowerCase()).join('_');
+ const formattedName = nameParts
+   .map(part => part.toLocaleLowerCase())
+   .map(part => part.replace(/[^a-z0-9]/g, '')) // Remove special characters
+   .filter(part => part.length > 0) // Remove empty parts
+   .join('_');

+ // Ensure we still have a valid username
+ return formattedName.length > 0 ? formattedName : `user_${Date.now().toString(36)}`;
}
🧰 Tools
🪛 ESLint

[error] 1669-1669: Replace '' with ""

(prettier/prettier)


[error] 1672-1672: Replace '·' with "·"

(prettier/prettier)


[error] 1675-1675: Replace .map(part·=>·part.toLocaleLowerCase()).join('_' with ⏎······.map((part)·=>·part.toLocaleLowerCase())⏎······.join("_"

(prettier/prettier)


1680-1713: Clean up duplicated code in createUserInDatabase.

There is duplication in setting user properties and inconsistent style with some properties having commas and others not.

async createUserInDatabase(request: any, userCreateDto: UserCreateDto, response: Response) {
  const user = new User();
  
  // Consolidate all user property assignments
  Object.assign(user, {
    userId: userCreateDto?.userId,
    username: userCreateDto?.username,
    name: userCreateDto?.name,
    firstName: userCreateDto?.firstName,
    middleName: userCreateDto?.middleName,
    lastName: userCreateDto?.lastName,
    gender: userCreateDto?.gender,
    email: userCreateDto?.email,
    mobile: Number(userCreateDto?.mobile) || null,
    createdBy: userCreateDto?.createdBy,
    updatedBy: userCreateDto?.updatedBy
  });

  if (userCreateDto?.dob) {
    user.dob = new Date(userCreateDto.dob);
  }
  
  const result = await this.usersRepository.save(user);
  const createdBy = request.user?.userId || result.userId;

  // Rest of the method...
}
🧰 Tools
🪛 ESLint

[error] 1680-1680: Replace request:·any,·userCreateDto:·UserCreateDto,·response:·Response with ⏎····request:·any,⏎····userCreateDto:·UserCreateDto,⏎····response:·Response⏎··

(prettier/prettier)


[error] 1681-1682: Replace ⏎····const·user·=·new·User() with ····const·user·=·new·User();

(prettier/prettier)


[error] 1683-1683: Insert ;

(prettier/prettier)


[error] 1684-1684: Insert ;

(prettier/prettier)


[error] 1685-1685: Insert ;

(prettier/prettier)


[error] 1686-1686: Replace user.mobile·=·Number(userCreateDto?.mobile)·||·null with (user.mobile·=·Number(userCreateDto?.mobile)·||·null)

(prettier/prettier)


[error] 1687-1687: Replace user.createdBy·=·userCreateDto?.createdBy with (user.createdBy·=·userCreateDto?.createdBy);

(prettier/prettier)


[error] 1688-1688: Insert ;

(prettier/prettier)


[error] 1689-1689: Replace user.userId·=·userCreateDto?.userId with (user.userId·=·userCreateDto?.userId)

(prettier/prettier)


[error] 1690-1690: Replace user.username·=·userCreateDto?.username with (user.username·=·userCreateDto?.username)

(prettier/prettier)


[error] 1691-1691: Replace user.firstName·=·userCreateDto?.firstName with (user.firstName·=·userCreateDto?.firstName)

(prettier/prettier)


[error] 1692-1692: Replace user.middleName·=·userCreateDto?.middleName with (user.middleName·=·userCreateDto?.middleName)

(prettier/prettier)


[error] 1693-1693: Replace user.lastName·=·userCreateDto?.lastName with (user.lastName·=·userCreateDto?.lastName)

(prettier/prettier)


[error] 1694-1694: Replace user.gender·=·userCreateDto?.gender with (user.gender·=·userCreateDto?.gender)

(prettier/prettier)


[error] 1695-1695: Replace user.email·=·userCreateDto?.email with (user.email·=·userCreateDto?.email)

(prettier/prettier)


[error] 1696-1696: Replace user.mobile·=·Number(userCreateDto?.mobile)·||·null with (user.mobile·=·Number(userCreateDto?.mobile)·||·null)

(prettier/prettier)


[error] 1697-1697: Replace user.createdBy·=·userCreateDto?.createdBy·||·userCreateDto?.createdBy with (user.createdBy·=·userCreateDto?.createdBy·||·userCreateDto?.createdBy)

(prettier/prettier)


[error] 1706-1706: Replace userCreateDto.automaticMember·&&·userCreateDto?.automaticMember?.value·===·true with ⏎········userCreateDto.automaticMember·&&⏎········userCreateDto?.automaticMember?.value·===·true⏎······

(prettier/prettier)


[error] 1707-1707: Replace userCreateDto.automaticMember,·userCreateDto.customFields,·userCreateDto.tenantCohortRoleMapping,·result.userId,·createdBy) with ⏎··········userCreateDto.automaticMember,⏎··········userCreateDto.customFields,⏎··········userCreateDto.tenantCohortRoleMapping,⏎··········result.userId,⏎··········createdBy⏎········);

(prettier/prettier)


[error] 1709-1709: Replace userCreateDto.tenantCohortRoleMapping,·academicYearId,·result.userId,·createdBy with ⏎··········userCreateDto.tenantCohortRoleMapping,⏎··········academicYearId,⏎··········result.userId,⏎··········createdBy⏎········

(prettier/prettier)


839-876: Consider using Promise.all for efficient batch processing.

When updating multiple users with the same name, the code processes them sequentially, which could be optimized.

const getUser = await this.usersRepository.find({where: { name: userDto.name }});

+ // If we have custom fields to update and multiple users, process them in parallel
+ if (userDto?.customFields?.length > 0 && getUser.length > 1) {
+   const getFieldsAttributes = await this.fieldsService.getEditableFieldsAttributes();
+   const fieldIdAndAttributes = {};
+   const isEditableFieldId = getFieldsAttributes.map(fieldDetails => {
+     fieldIdAndAttributes[fieldDetails.fieldId] = fieldDetails;
+     return fieldDetails.fieldId;
+   });
+ 
+   const updatePromises = getUser.map(async (user) => {
+     const userUpdateResults = {
+       customFields: [],
+       errors: {
+         uneditableFields: [],
+         editFailures: []
+       }
+     };
+ 
+     await Promise.all(userDto.customFields.map(async (data) => {
+       if (isEditableFieldId.includes(data.fieldId)) {
+         const result = await this.fieldsService.updateCustomFields(
+           user.userId, data, fieldIdAndAttributes[data.fieldId]
+         );
+         if (result.correctValue) {
+           userUpdateResults.customFields.push(result);
+         } else {
+           userUpdateResults.errors.editFailures.push(
+             `${data.fieldId}: ${result?.valueIssue} - ${result.fieldName}`
+           );
+         }
+       } else {
+         userUpdateResults.errors.uneditableFields.push(data.fieldId);
+       }
+     }));
+ 
+     return userUpdateResults;
+   });
+ 
+   const results = await Promise.all(updatePromises);
+   
+   // Combine results
+   const updatedData = { customFields: [] };
+   const editIssues = { uneditableFields: [], editFieldsFailure: [] };
+   
+   results.forEach(result => {
+     if (result.customFields.length > 0) {
+       updatedData.customFields = [...updatedData.customFields, ...result.customFields];
+     }
+     if (result.errors.uneditableFields.length > 0) {
+       editIssues.uneditableFields = [...editIssues.uneditableFields, ...result.errors.uneditableFields];
+     }
+     if (result.errors.editFailures.length > 0) {
+       editIssues.editFieldsFailure = [...editIssues.editFieldsFailure, ...result.errors.editFailures];
+     }
+   });
+ 
+   return await APIResponse.success(
+     response, apiId, { ...updatedData, editIssues },
+     HttpStatus.OK, "User has been updated successfully."
+   );
+ }
+ 
+ // Original sequential processing for single user or when no custom fields
for(const user of getUser){
  // ...existing code
}
🧰 Tools
🪛 ESLint

[error] 839-839: 'getUser' is never reassigned. Use 'const' instead.

(prefer-const)


[error] 839-839: Replace where:{name:·userDto.name}}) with ⏎········where:·{·name:·userDto.name·},⏎······});

(prettier/prettier)


[error] 840-841: Replace (let·user·of·getUser){⏎ with ·(let·user·of·getUser)·{

(prettier/prettier)


[error] 840-840: 'user' is never reassigned. Use 'const' instead.

(prefer-const)


[error] 843-843: Insert ⏎···········

(prettier/prettier)


[error] 844-844: Delete ··

(prettier/prettier)


[error] 845-845: 'isEditableFieldId' is never reassigned. Use 'const' instead.

(prefer-const)


[error] 847-847: 'fieldDetails' is never reassigned. Use 'const' instead.

(prefer-const)


[error] 851-851: Delete ··

(prettier/prettier)


[error] 852-852: 'unEditableIdes' is never reassigned. Use 'const' instead.

(prefer-const)


[error] 853-853: 'editFailures' is never reassigned. Use 'const' instead.

(prefer-const)


[error] 854-854: 'data' is never reassigned. Use 'const' instead.

(prefer-const)


[error] 856-856: Replace user.userId,·data,·fieldIdAndAttributes[data.fieldId] with ⏎················user.userId,⏎················data,⏎················fieldIdAndAttributes[data.fieldId]⏎··············

(prettier/prettier)


[error] 858-858: Replace 'customFields' with "customFields"

(prettier/prettier)


[error] 859-859: Replace 'customFields' with "customFields"

(prettier/prettier)


[error] 860-860: Replace 'customFields' with "customFields"

(prettier/prettier)


[error] 862-862: Replace ``${data.fieldId}:·${result?.valueIssue}·-·${result.fieldName}) with `⏎··················`${data.fieldId}:·${result?.valueIssue}·-·${result.fieldName}`⏎················);`

(prettier/prettier)


[error] 865-865: Insert ;

(prettier/prettier)


[error] 869-869: Insert ;

(prettier/prettier)


[error] 872-872: Insert ;

(prettier/prettier)

📜 Review details

Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 106baa4 and c3fe923.

📒 Files selected for processing (14)
  • .github/workflows/Prod-Deployment.yaml (0 hunks)
  • .github/workflows/oblf-deployment.yaml (1 hunks)
  • .github/workflows/tekdi-server-deployment.yaml (0 hunks)
  • manifest/backend.yaml (0 hunks)
  • src/adapters/postgres/cohort-adapter.ts (3 hunks)
  • src/adapters/postgres/user-adapter.ts (4 hunks)
  • src/adapters/userservicelocator.ts (1 hunks)
  • src/cohort/dto/cohort-create.dto.ts (4 hunks)
  • src/cohort/dto/cohort-search.dto.ts (1 hunks)
  • src/cohort/entities/cohort.entity.ts (1 hunks)
  • src/fields/fieldValidators/fieldTypeClasses.ts (1 hunks)
  • src/user/dto/user-create.dto.ts (2 hunks)
  • src/user/dto/user-update.dto.ts (1 hunks)
  • src/user/user.controller.ts (1 hunks)
💤 Files with no reviewable changes (3)
  • manifest/backend.yaml
  • .github/workflows/tekdi-server-deployment.yaml
  • .github/workflows/Prod-Deployment.yaml
🧰 Additional context used
📓 Path-based instructions (1)
`**/*.ts`: "Review the JavaScript code for conformity with the Google JavaScript style guide, highlighting any deviations. Ensure that: - The code adheres to best practices associa...

**/*.ts: "Review the JavaScript code for conformity with the Google JavaScript style guide, highlighting any deviations. Ensure that:

  • The code adheres to best practices associated with nodejs.
  • The code adheres to best practices associated with nestjs framework.
  • The code adheres to best practices recommended for performance.
  • The code adheres to similar naming conventions for controllers, models, services, methods, variables."
  • src/cohort/dto/cohort-search.dto.ts
  • src/cohort/entities/cohort.entity.ts
  • src/user/dto/user-update.dto.ts
  • src/adapters/postgres/cohort-adapter.ts
  • src/user/user.controller.ts
  • src/fields/fieldValidators/fieldTypeClasses.ts
  • src/user/dto/user-create.dto.ts
  • src/adapters/userservicelocator.ts
  • src/cohort/dto/cohort-create.dto.ts
  • src/adapters/postgres/user-adapter.ts
🧬 Code Graph Analysis (2)
src/adapters/userservicelocator.ts (1)
src/user/dto/user-create.dto.ts (1)
  • UserCreateDto (81-213)
src/cohort/dto/cohort-create.dto.ts (1)
src/user/dto/user-create.dto.ts (1)
  • FieldValuesOptionDto (48-63)
🪛 ESLint
src/cohort/entities/cohort.entity.ts

[error] 64-64: Replace 'jsonb' with "jsonb"

(prettier/prettier)


[error] 65-66: Delete

(prettier/prettier)

src/adapters/postgres/cohort-adapter.ts

[error] 661-661: Insert ·

(prettier/prettier)


[error] 662-662: Insert ·

(prettier/prettier)


[error] 668-668: Delete ·····

(prettier/prettier)


[error] 670-671: Delete ⏎········

(prettier/prettier)


[error] 808-809: Delete

(prettier/prettier)


[error] 810-810: Delete ··········

(prettier/prettier)


[error] 811-811: Delete ·

(prettier/prettier)


[error] 812-812: Replace ·const·userExistCohortGroup·=·limit·?·data.slice(offset,·offset·+·limit)·:·data; with const·userExistCohortGroup·=·limit⏎··········?·data.slice(offset,·offset·+·limit)

(prettier/prettier)


[error] 813-813: Insert :·data;⏎

(prettier/prettier)

src/user/user.controller.ts

[error] 180-181: Delete

(prettier/prettier)


[error] 200-200: Delete ····

(prettier/prettier)


[error] 203-203: Replace .buildUserAdapter() with ⏎······.buildUserAdapter()⏎······

(prettier/prettier)


[error] 204-205: Delete

(prettier/prettier)

src/fields/fieldValidators/fieldTypeClasses.ts

[error] 49-49: Delete ··

(prettier/prettier)


[error] 50-50: Replace ········if·(!(typeof·value·===·'string' with ····if·(!(typeof·value·===·"string"

(prettier/prettier)


[error] 51-51: Replace ······throw·new·Error('Value·must·be·a·valid·time·format·(HH:mm·or·HH:mm:ss).' with throw·new·Error("Value·must·be·a·valid·time·format·(HH:mm·or·HH:mm:ss)."

(prettier/prettier)


[error] 52-52: Replace ········ with ····

(prettier/prettier)


[error] 53-53: Replace ········ with ····

(prettier/prettier)


[error] 54-54: Delete ··

(prettier/prettier)


[error] 56-56: Delete ··

(prettier/prettier)


[error] 57-57: Delete ····

(prettier/prettier)


[error] 58-58: Delete ····

(prettier/prettier)


[error] 59-59: Delete ····

(prettier/prettier)


[error] 60-60: Replace ···· with ··

(prettier/prettier)


[error] 64-64: Delete ··

(prettier/prettier)


[error] 65-65: Replace ········if·(typeof·value·!==·'string' with ····if·(typeof·value·!==·"string"

(prettier/prettier)


[error] 66-66: Replace ······throw·new·Error('Value·must·be·a·valid·date·format·(YYYY-MM-DD).' with throw·new·Error("Value·must·be·a·valid·date·format·(YYYY-MM-DD)."

(prettier/prettier)


[error] 67-67: Replace ········ with ····

(prettier/prettier)


[error] 68-68: Replace ········ with ····

(prettier/prettier)


[error] 69-69: Delete ··

(prettier/prettier)


[error] 71-71: Delete ··

(prettier/prettier)


[error] 72-72: Replace ········ with ····

(prettier/prettier)


[error] 73-73: Delete ····

(prettier/prettier)


[error] 75-75: Delete ····

(prettier/prettier)


[error] 76-76: Delete ····

(prettier/prettier)


[error] 77-77: Replace ············ with ······

(prettier/prettier)


[error] 78-78: Delete ····

(prettier/prettier)


[error] 80-80: Delete ····

(prettier/prettier)


[error] 81-81: Delete ····

(prettier/prettier)


[error] 82-82: Replace ····return·date·instanceof·Date·&&·!isNaN(date.getTime())·&&·input·===·date.toISOString().slice(0,·10); with return·(⏎······date·instanceof·Date·&&⏎······!isNaN(date.getTime())·&&⏎······input·===·date.toISOString().slice(0,·10)

(prettier/prettier)


[error] 83-83: Insert );⏎··

(prettier/prettier)


[error] 84-85: Delete

(prettier/prettier)

src/adapters/userservicelocator.ts

[error] 23-23: Don't use String as a type. Use string instead

(@typescript-eslint/ban-types)


[error] 23-23: Replace tenantId?:·string) with ·tenantId?:·string);

(prettier/prettier)

src/adapters/postgres/user-adapter.ts

[error] 833-833: Insert ·

(prettier/prettier)


[error] 836-836: 'updatedData' is never reassigned. Use 'const' instead.

(prefer-const)


[error] 837-837: 'editIssues' is never reassigned. Use 'const' instead.

(prefer-const)


[error] 839-839: 'getUser' is never reassigned. Use 'const' instead.

(prefer-const)


[error] 839-839: Replace where:{name:·userDto.name}}) with ⏎········where:·{·name:·userDto.name·},⏎······});

(prettier/prettier)


[error] 840-841: Replace (let·user·of·getUser){⏎ with ·(let·user·of·getUser)·{

(prettier/prettier)


[error] 840-840: 'user' is never reassigned. Use 'const' instead.

(prefer-const)


[error] 843-843: Insert ⏎···········

(prettier/prettier)


[error] 844-844: Delete ··

(prettier/prettier)


[error] 845-845: 'isEditableFieldId' is never reassigned. Use 'const' instead.

(prefer-const)


[error] 847-847: 'fieldDetails' is never reassigned. Use 'const' instead.

(prefer-const)


[error] 851-851: Delete ··

(prettier/prettier)


[error] 852-852: 'unEditableIdes' is never reassigned. Use 'const' instead.

(prefer-const)


[error] 853-853: 'editFailures' is never reassigned. Use 'const' instead.

(prefer-const)


[error] 854-854: 'data' is never reassigned. Use 'const' instead.

(prefer-const)


[error] 856-856: Replace user.userId,·data,·fieldIdAndAttributes[data.fieldId] with ⏎················user.userId,⏎················data,⏎················fieldIdAndAttributes[data.fieldId]⏎··············

(prettier/prettier)


[error] 858-858: Replace 'customFields' with "customFields"

(prettier/prettier)


[error] 859-859: Replace 'customFields' with "customFields"

(prettier/prettier)


[error] 860-860: Replace 'customFields' with "customFields"

(prettier/prettier)


[error] 862-862: Replace ``${data.fieldId}:·${result?.valueIssue}·-·${result.fieldName}) with `⏎··················`${data.fieldId}:·${result?.valueIssue}·-·${result.fieldName}`⏎················);`

(prettier/prettier)


[error] 865-865: Insert ;

(prettier/prettier)


[error] 869-869: Insert ;

(prettier/prettier)


[error] 872-872: Insert ;

(prettier/prettier)


[error] 877-877: Replace response,·apiId, with ⏎········response,⏎········apiId,⏎·······

(prettier/prettier)


[error] 878-878: Replace ·"User·has·been·updated·successfully.") with ⏎········"User·has·been·updated·successfully."⏎······);

(prettier/prettier)


[error] 880-880: Replace response,·apiId,·"Internal·Server·Error",·${e},·HttpStatus.INTERNAL_SERVER_ERROR with ⏎········response,⏎········apiId,⏎········"Internal·Server·Error",⏎········${e},⏎········HttpStatus.INTERNAL_SERVER_ERROR⏎······

(prettier/prettier)


[error] 1270-1270: Replace userCreateDto with ⏎········userCreateDto⏎······

(prettier/prettier)


[error] 1272-1272: Replace ·?·await·this.formatUsername(userCreateDto.username)·:·getUsernameAndPassword['username' with ⏎········?·await·this.formatUsername(userCreateDto.username)⏎········:·getUsernameAndPassword["username"

(prettier/prettier)


[error] 1274-1274: Replace ·?·userCreateDto.password·:·getUsernameAndPassword['password' with ⏎········?·userCreateDto.password⏎········:·getUsernameAndPassword["password"

(prettier/prettier)


[error] 1600-1600: 'tenantCohortRoleMap' is never reassigned. Use 'const' instead.

(prefer-const)


[error] 1600-1600: Replace ·?·userCreateDto.tenantCohortRoleMapping with ⏎········?·userCreateDto.tenantCohortRoleMapping⏎·······

(prettier/prettier)


[error] 1605-1605: 'getRoleName' is never reassigned. Use 'const' instead.

(prefer-const)


[error] 1606-1606: Replace "roleId" with roleId

(prettier/prettier)


[error] 1607-1607: Insert ,

(prettier/prettier)


[error] 1608-1608: Insert ;

(prettier/prettier)


[error] 1610-1610: 'getTenantName' is never reassigned. Use 'const' instead.

(prefer-const)


[error] 1611-1611: Replace "tenantId" with tenantId

(prettier/prettier)


[error] 1612-1612: Insert ,

(prettier/prettier)


[error] 1613-1613: Insert ;

(prettier/prettier)


[error] 1614-1614: Replace .map(role·=>·role?.title.toUpperCase()).join(',·') with ⏎············.map((role)·=>·role?.title.toUpperCase())⏎············.join(",·");

(prettier/prettier)


[error] 1615-1615: Replace role·=>·role?.name).join(',·') with (role)·=>·role?.name).join(",·");

(prettier/prettier)


[error] 1623-1623: Replace 'STUDENT' with "STUDENT"

(prettier/prettier)


[error] 1624-1624: 'fieldValues' is never reassigned. Use 'const' instead.

(prefer-const)


[error] 1628-1628: 'getFieldName' is never reassigned. Use 'const' instead.

(prefer-const)


[error] 1628-1628: Replace fieldsData?.fieldId with ⏎··············fieldsData?.fieldId⏎············

(prettier/prettier)


[error] 1629-1629: Replace 'name']·===·'program' with "name"]·===·"program"

(prettier/prettier)


[error] 1630-1630: Insert ;

(prettier/prettier)


[error] 1637-1637: Replace 'user' with "user"

(prettier/prettier)


[error] 1638-1638: Replace "MAX(CAST(REGEXP_REPLACE(user.username,·'[^0-9]',·'',·'g')·AS·INTEGER))",·'max' with ⏎············"MAX(CAST(REGEXP_REPLACE(user.username,·'[^0-9]',·'',·'g')·AS·INTEGER))",⏎············"max"⏎··········

(prettier/prettier)


[error] 1639-1639: Replace ·usernamePattern:·${userTenant}%`` with ⏎············usernamePattern:·${userTenant}%`,⏎·········`

(prettier/prettier)


[error] 1646-1646: Replace suffix?.[0]?.toUpperCase()·||·'' with ⏎··········suffix?.[0]?.toUpperCase()·||·""⏎········

(prettier/prettier)


[error] 1647-1647: Replace userTenant.toLowerCase()·||·''}`` with ⏎··········userTenant.toLowerCase()·||·""⏎········};

(prettier/prettier)


[error] 1648-1649: Delete

(prettier/prettier)


[error] 1651-1651: Replace 'TEACHER' with "TEACHER"

(prettier/prettier)


[error] 1653-1653: Replace userTenant.toLowerCase()·||·''}`` with ⏎··········userTenant.toLowerCase()·||·""⏎········};

(prettier/prettier)


[error] 1656-1656: 'loginCredintial' is never reassigned. Use 'const' instead.

(prefer-const)


[error] 1657-1657: Replace "username" with username

(prettier/prettier)


[error] 1658-1658: Replace "password":·generatePassword with password:·generatePassword,

(prettier/prettier)


[error] 1661-1662: Delete

(prettier/prettier)


[error] 1669-1669: Replace '' with ""

(prettier/prettier)


[error] 1672-1672: Replace '·' with "·"

(prettier/prettier)


[error] 1675-1675: Replace .map(part·=>·part.toLocaleLowerCase()).join('_' with ⏎······.map((part)·=>·part.toLocaleLowerCase())⏎······.join("_"

(prettier/prettier)


[error] 1680-1680: Replace request:·any,·userCreateDto:·UserCreateDto,·response:·Response with ⏎····request:·any,⏎····userCreateDto:·UserCreateDto,⏎····response:·Response⏎··

(prettier/prettier)


[error] 1681-1682: Replace ⏎····const·user·=·new·User() with ····const·user·=·new·User();

(prettier/prettier)


[error] 1683-1683: Insert ;

(prettier/prettier)


[error] 1684-1684: Insert ;

(prettier/prettier)


[error] 1685-1685: Insert ;

(prettier/prettier)


[error] 1686-1686: Replace user.mobile·=·Number(userCreateDto?.mobile)·||·null with (user.mobile·=·Number(userCreateDto?.mobile)·||·null)

(prettier/prettier)


[error] 1687-1687: Replace user.createdBy·=·userCreateDto?.createdBy with (user.createdBy·=·userCreateDto?.createdBy);

(prettier/prettier)


[error] 1688-1688: Insert ;

(prettier/prettier)

🪛 Biome (1.9.4)
src/user/dto/user-update.dto.ts

[error] 169-170: Expected an identifier, a string literal, a number literal, a private field name, or a computed name but instead found '==='.

Expected an identifier, a string literal, a number literal, a private field name, or a computed name here.

(parse)


[error] 172-173: Expected an identifier, a string literal, a number literal, a private field name, or a computed name but instead found '>'.

Expected an identifier, a string literal, a number literal, a private field name, or a computed name here.

(parse)


[error] 173-173: expected a semicolon to end the class property, but found none

(parse)


[error] 173-173: Expected an identifier, a string literal, a number literal, a private field name, or a computed name but instead found '/'.

Expected an identifier, a string literal, a number literal, a private field name, or a computed name here.

(parse)


[error] 171-171: Duplicate class member name "userId"

(lint/suspicious/noDuplicateClassMembers)

src/adapters/userservicelocator.ts

[error] 23-23: Don't use 'String' as a type.

Use lowercase primitives for consistency.
Safe fix: Use 'string' instead

(lint/complexity/noBannedTypes)

🔇 Additional comments (16)
src/adapters/postgres/cohort-adapter.ts (1)

670-672: Improved limit validation

Good improvement to check if limit is undefined before comparing it to MAX_LIMIT.

🧰 Tools
🪛 ESLint

[error] 670-671: Delete ⏎········

(prettier/prettier)

src/fields/fieldValidators/fieldTypeClasses.ts (2)

48-61: Well-implemented TimeField validation class

The TimeField class provides a robust validation mechanism for time strings using a regular expression pattern that matches both HH:mm and HH:mm:ss formats.

However, the code formatting differs from the project style. Consider updating the double quotes consistency and indentation:

 export class TimeField extends Field {
-    validate(value: any): boolean {
-        if (!(typeof value === 'string' && this.isValidTime(value))) {
-            throw new Error('Value must be a valid time format (HH:mm or HH:mm:ss).');
-        }
-        return true;
-    }
+  validate(value: any): boolean {
+    if (!(typeof value === "string" && this.isValidTime(value))) {
+      throw new Error("Value must be a valid time format (HH:mm or HH:mm:ss).");
+    }
+    return true;
+  }

-    isValidTime(input: string): boolean {
-        // Regular expression to match HH:mm or HH:mm:ss format
-        const timeRegex = /^([01]\d|2[0-3]):([0-5]\d)(:[0-5]\d)?$/;
-        return timeRegex.test(input);
-    }
+  isValidTime(input: string): boolean {
+    // Regular expression to match HH:mm or HH:mm:ss format
+    const timeRegex = /^([01]\d|2[0-3]):([0-5]\d)(:[0-5]\d)?$/;
+    return timeRegex.test(input);
+  }
 }
🧰 Tools
🪛 ESLint

[error] 49-49: Delete ··

(prettier/prettier)


[error] 50-50: Replace ········if·(!(typeof·value·===·'string' with ····if·(!(typeof·value·===·"string"

(prettier/prettier)


[error] 51-51: Replace ······throw·new·Error('Value·must·be·a·valid·time·format·(HH:mm·or·HH:mm:ss).' with throw·new·Error("Value·must·be·a·valid·time·format·(HH:mm·or·HH:mm:ss)."

(prettier/prettier)


[error] 52-52: Replace ········ with ····

(prettier/prettier)


[error] 53-53: Replace ········ with ····

(prettier/prettier)


[error] 54-54: Delete ··

(prettier/prettier)


[error] 56-56: Delete ··

(prettier/prettier)


[error] 57-57: Delete ····

(prettier/prettier)


[error] 58-58: Delete ····

(prettier/prettier)


[error] 59-59: Delete ····

(prettier/prettier)


[error] 60-60: Replace ···· with ··

(prettier/prettier)


63-84: Well-implemented DateField validation with comprehensive checks

The DateField class provides thorough validation with both regex pattern matching and actual date validity verification, which is an excellent approach.

The code formatting should be updated to match project style:

 export class DateField extends Field {
-    validate(value: any): boolean {
-        if (typeof value !== 'string' || !this.isValidDate(value)) {
-            throw new Error('Value must be a valid date format (YYYY-MM-DD).');
-        }
-        return true;
-    }
+  validate(value: any): boolean {
+    if (typeof value !== "string" || !this.isValidDate(value)) {
+      throw new Error("Value must be a valid date format (YYYY-MM-DD).");
+    }
+    return true;
+  }

-    isValidDate(input: string): boolean {
-        // Regular expression to match YYYY-MM-DD format
-        const dateRegex = /^\d{4}-(0[1-9]|1[0-2])-(0[1-9]|[12]\d|3[01])$/;
+  isValidDate(input: string): boolean {
+    // Regular expression to match YYYY-MM-DD format
+    const dateRegex = /^\d{4}-(0[1-9]|1[0-2])-(0[1-9]|[12]\d|3[01])$/;

-        // Check if the date matches the format
-        if (!dateRegex.test(input)) {
-            return false;
-        }
+    // Check if the date matches the format
+    if (!dateRegex.test(input)) {
+      return false;
+    }

-        // Additional check for valid month and day combinations (like no February 30th)
-        const date = new Date(input);
-        return date instanceof Date && !isNaN(date.getTime()) && input === date.toISOString().slice(0, 10);
+    // Additional check for valid month and day combinations (like no February 30th)
+    const date = new Date(input);
+    return (
+      date instanceof Date && 
+      !isNaN(date.getTime()) && 
+      input === date.toISOString().slice(0, 10)
+    );
+  }
 }
🧰 Tools
🪛 ESLint

[error] 64-64: Delete ··

(prettier/prettier)


[error] 65-65: Replace ········if·(typeof·value·!==·'string' with ····if·(typeof·value·!==·"string"

(prettier/prettier)


[error] 66-66: Replace ······throw·new·Error('Value·must·be·a·valid·date·format·(YYYY-MM-DD).' with throw·new·Error("Value·must·be·a·valid·date·format·(YYYY-MM-DD)."

(prettier/prettier)


[error] 67-67: Replace ········ with ····

(prettier/prettier)


[error] 68-68: Replace ········ with ····

(prettier/prettier)


[error] 69-69: Delete ··

(prettier/prettier)


[error] 71-71: Delete ··

(prettier/prettier)


[error] 72-72: Replace ········ with ····

(prettier/prettier)


[error] 73-73: Delete ····

(prettier/prettier)


[error] 75-75: Delete ····

(prettier/prettier)


[error] 76-76: Delete ····

(prettier/prettier)


[error] 77-77: Replace ············ with ······

(prettier/prettier)


[error] 78-78: Delete ····

(prettier/prettier)


[error] 80-80: Delete ····

(prettier/prettier)


[error] 81-81: Delete ····

(prettier/prettier)


[error] 82-82: Replace ····return·date·instanceof·Date·&&·!isNaN(date.getTime())·&&·input·===·date.toISOString().slice(0,·10); with return·(⏎······date·instanceof·Date·&&⏎······!isNaN(date.getTime())·&&⏎······input·===·date.toISOString().slice(0,·10)

(prettier/prettier)


[error] 83-83: Insert );⏎··

(prettier/prettier)

src/user/dto/user-create.dto.ts (2)

85-87: Username made optional in UserCreateDto

The username field is now correctly marked as optional, which aligns with the automatic username generation feature mentioned in the summary.


142-147: Password field made optional

The password field is now correctly marked as optional using ApiPropertyOptional, aligning with the automatic password generation feature.

src/adapters/userservicelocator.ts (2)

21-21: New method added for updating users by name

Good addition of a new method to support the feature for updating users by name.

However, using generic 'any' types reduces type safety. Consider:

-  updateUserByName(userDto?: any, response?: any);
+  updateUserByName(name: string, userDto?: Partial<UserUpdateDTO>, response?: Response);

22-22: Method signature updated to remove academicYearId parameter

The createUser method signature has been simplified, which aligns with changes in the implementation.

src/cohort/dto/cohort-create.dto.ts (8)

23-28: LGTM! Making createdAt and updatedAt optional is appropriate.

These timestamp fields are typically managed by the system rather than being required inputs from users.


30-38: LGTM! Making programId optional with proper documentation.

The implementation includes complete API documentation and appropriate validation.


40-48: LGTM! Making parentId optional with proper documentation.

The implementation includes complete API documentation and appropriate validation.


74-83: Appropriate enum validation for status field.

Good addition of enum validation to ensure status values are restricted to the allowed set.


85-94: Good addition of boolean validation for attendanceCaptureImage.

The field now has proper type validation with @isboolean() and clear API documentation with default value.


103-107: LGTM! Adding string validation for metadata.

The implementation correctly adds string validation and makes the field optional.


109-119: LGTM! Adding string validation for createdBy and updatedBy.

The implementation correctly adds string validation and makes the fields optional.


121-129: LGTM! Making customFields optional is appropriate.

The field has proper validation with ValidateNested and Type decorators.

src/adapters/postgres/user-adapter.ts (1)

1270-1275: Good approach for dynamic username/password generation.

The code now dynamically generates username and password if they're not provided, which improves user creation flexibility.

🧰 Tools
🪛 ESLint

[error] 1270-1270: Replace userCreateDto with ⏎········userCreateDto⏎······

(prettier/prettier)


[error] 1272-1272: Replace ·?·await·this.formatUsername(userCreateDto.username)·:·getUsernameAndPassword['username' with ⏎········?·await·this.formatUsername(userCreateDto.username)⏎········:·getUsernameAndPassword["username"

(prettier/prettier)


[error] 1274-1274: Replace ·?·userCreateDto.password·:·getUsernameAndPassword['password' with ⏎········?·userCreateDto.password⏎········:·getUsernameAndPassword["password"

(prettier/prettier)

timeout-minutes: 15
steps:
- name: Deploy Stack
uses: appleboy/ssh-action@master
Copy link

Choose a reason for hiding this comment

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

🛠️ Refactor suggestion

Pin the SSH action to a specific version
Using @master can introduce breaking changes unexpectedly. Pin to a known stable release, e.g.:

- uses: appleboy/ssh-action@master
+ uses: appleboy/ssh-action@v0.1.8
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
uses: appleboy/ssh-action@master
uses: appleboy/ssh-action@v0.1.8

Comment on lines 161 to +172
userId: string;

<<<<<<< HEAD
@ApiProperty({ type: () => [UserDataDTO] })
@Expose()
@ValidateNested()
@IsNotEmpty()
@Type(() => UserDataDTO)
userData: UserDataDTO;
=======
userId: string;
name:string;
Copy link

Choose a reason for hiding this comment

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

⚠️ Potential issue

Remove duplicate userId property

The userId property is declared twice in the class (lines 161 and 171), which will cause issues. Remove one of the declarations.

🧰 Tools
🪛 Biome (1.9.4)

[error] 162-163: Expected an identifier, a string literal, a number literal, a private field name, or a computed name but instead found '<<'.

Expected an identifier, a string literal, a number literal, a private field name, or a computed name here.

(parse)


[error] 169-170: Expected an identifier, a string literal, a number literal, a private field name, or a computed name but instead found '==='.

Expected an identifier, a string literal, a number literal, a private field name, or a computed name here.

(parse)


[error] 171-171: Duplicate class member name "userId"

(lint/suspicious/noDuplicateClassMembers)

Comment on lines +163 to +173
<<<<<<< HEAD
@ApiProperty({ type: () => [UserDataDTO] })
@Expose()
@ValidateNested()
@IsNotEmpty()
@Type(() => UserDataDTO)
userData: UserDataDTO;
=======
userId: string;
name:string;
>>>>>>> upstream/lateMarkingForSelfAttendance
Copy link

Choose a reason for hiding this comment

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

⚠️ Potential issue

Resolve merge conflict in UserUpdateDTO

There's an unresolved merge conflict in this file. The git merge markers need to be removed and a proper resolution implemented. Based on the surrounding code and the AI summary, you need to decide whether to keep the nested userData object or replace it with a simpler name property.

- <<<<<<< HEAD
-  @ApiProperty({ type: () => [UserDataDTO] })
-  @Expose()
-  @ValidateNested()
-  @IsNotEmpty()
-  @Type(() => UserDataDTO)
-  userData: UserDataDTO;
- =======
-    userId: string;
-    name:string;
- >>>>>>> upstream/lateMarkingForSelfAttendance
+ // If choosing the simpler approach:
+ @ApiProperty({ type: String, description: 'User name for identification' })
+ @IsString()
+ @IsNotEmpty()
+ name: string;
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
<<<<<<< HEAD
@ApiProperty({ type: () => [UserDataDTO] })
@Expose()
@ValidateNested()
@IsNotEmpty()
@Type(() => UserDataDTO)
userData: UserDataDTO;
=======
userId: string;
name:string;
>>>>>>> upstream/lateMarkingForSelfAttendance
// If choosing the simpler approach:
@ApiProperty({ type: String, description: 'User name for identification' })
@IsString()
@IsNotEmpty()
name: string;
🧰 Tools
🪛 Biome (1.9.4)

[error] 169-170: Expected an identifier, a string literal, a number literal, a private field name, or a computed name but instead found '==='.

Expected an identifier, a string literal, a number literal, a private field name, or a computed name here.

(parse)


[error] 172-173: Expected an identifier, a string literal, a number literal, a private field name, or a computed name but instead found '>'.

Expected an identifier, a string literal, a number literal, a private field name, or a computed name here.

(parse)


[error] 173-173: expected a semicolon to end the class property, but found none

(parse)


[error] 173-173: Expected an identifier, a string literal, a number literal, a private field name, or a computed name but instead found '/'.

Expected an identifier, a string literal, a number literal, a private field name, or a computed name here.

(parse)


[error] 171-171: Duplicate class member name "userId"

(lint/suspicious/noDuplicateClassMembers)

@UseGuards(JwtAuthGuard)
@ApiBasicAuth("access-token")
@ApiBody({ type: UserUpdateDTO })
@ApiCreatedResponse({ description: "User has been updated successfully." })
Copy link

Choose a reason for hiding this comment

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

🛠️ Refactor suggestion

Use ApiOkResponse instead of ApiCreatedResponse for PATCH operations

PATCH operations typically update existing resources and should return a 200 OK response, not a 201 Created. Use @ApiOkResponse instead of @ApiCreatedResponse to accurately reflect the HTTP semantics.

- @ApiCreatedResponse({ description: "User has been updated successfully." })
+ @ApiOkResponse({ description: "User has been updated successfully." })

Comment on lines +181 to +204
@UseFilters(new AllExceptionsFilter(APIID.USER_UPDATE))
@Patch("updateByName/:name")
@UsePipes(new ValidationPipe())
@UseGuards(JwtAuthGuard)
@ApiBasicAuth("access-token")
@ApiBody({ type: UserUpdateDTO })
@ApiCreatedResponse({ description: "User has been updated successfully." })
@ApiForbiddenResponse({ description: "Forbidden" })
@ApiHeader({
name: "tenantid",
})
public async updateUserByName(
@Headers() headers,
@Param("name") name: string,
@Req() request: Request,
@Body() userUpdateDto: UserUpdateDTO,
@Res() response: Response
) {
console.log(userUpdateDto);

// userDto.tenantId = headers["tenantid"];
userUpdateDto.name = name;
return await this.userAdapter.buildUserAdapter().updateUserByName(userUpdateDto, response);
}
Copy link

Choose a reason for hiding this comment

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

🛠️ Refactor suggestion

Add error handling for user not found

The current implementation doesn't include proper API documentation or handling for the case when a user with the given name doesn't exist. Consider adding @ApiNotFoundResponse decorator and ensuring the adapter method handles this case appropriately.

  @ApiBasicAuth("access-token")
  @ApiBody({ type: UserUpdateDTO })
  @ApiOkResponse({ description: "User has been updated successfully." })
  @ApiForbiddenResponse({ description: "Forbidden" })
+ @ApiNotFoundResponse({ description: "User not found" })
  @ApiHeader({
    name: "tenantid",
  })

Committable suggestion skipped: line range outside the PR's diff.

🧰 Tools
🪛 ESLint

[error] 200-200: Delete ····

(prettier/prettier)


[error] 203-203: Replace .buildUserAdapter() with ⏎······.buildUserAdapter()⏎······

(prettier/prettier)

Comment on lines +661 to +662
// limit = limit || 200;
limit = limit || null;
Copy link

Choose a reason for hiding this comment

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

🛠️ Refactor suggestion

Modified default limit behavior

Changing from a default limit of 200 to null effectively removes pagination by default, which could impact performance if you have large datasets. This change allows returning all results when no limit is specified.

Consider adding a safeguard to prevent potentially large result sets from impacting performance:

-     // limit = limit || 200;
-     limit = limit || null;
+     // If no limit is provided, use a sensible default or set to null for all results
+     limit = limit || (process.env.DEFAULT_QUERY_LIMIT ? parseInt(process.env.DEFAULT_QUERY_LIMIT) : null);
+     
+     // Log when unlimited results are requested to monitor usage
+     if (limit === null) {
+       LoggerUtil.warn(`Unlimited results requested for cohort search`, apiId);
+     }
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
// limit = limit || 200;
limit = limit || null;
// If no limit is provided, use a sensible default or set to null for all results
limit = limit || (process.env.DEFAULT_QUERY_LIMIT ? parseInt(process.env.DEFAULT_QUERY_LIMIT, 10) : null);
// Log when unlimited results are requested to monitor usage
if (limit === null) {
LoggerUtil.warn(`Unlimited results requested for cohort search`, apiId);
}
🧰 Tools
🪛 ESLint

[error] 661-661: Insert ·

(prettier/prettier)


[error] 662-662: Insert ·

(prettier/prettier)

updateUser(userDto?: any, response?: any);
updateUserByName(userDto?: any, response?: any);
createUser(request: any, userDto: UserCreateDto, response: Response);
findUserDetails(userID: any, username: String,tenantId?: string)
Copy link

Choose a reason for hiding this comment

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

🛠️ Refactor suggestion

Use lowercase 'string' primitive type instead of 'String'

TypeScript best practices recommend using lowercase primitive types instead of constructor types.

-  findUserDetails(userID: any, username: String,tenantId?: string)
+  findUserDetails(userID: any, username: string, tenantId?: string);

Also note the missing semicolon and space after the comma.

📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
findUserDetails(userID: any, username: String,tenantId?: string)
findUserDetails(userID: any, username: string, tenantId?: string);
🧰 Tools
🪛 Biome (1.9.4)

[error] 23-23: Don't use 'String' as a type.

Use lowercase primitives for consistency.
Safe fix: Use 'string' instead

(lint/complexity/noBannedTypes)

🪛 ESLint

[error] 23-23: Don't use String as a type. Use string instead

(@typescript-eslint/ban-types)


[error] 23-23: Replace tenantId?:·string) with ·tenantId?:·string);

(prettier/prettier)

Comment on lines +1598 to +1665
async generateUserNamePassword(userCreateDto: UserCreateDto) {
try {
let tenantCohortRoleMap = userCreateDto ? userCreateDto.tenantCohortRoleMapping : [];
let userRole;
let userTenant;
for (const tenantCohortRole of tenantCohortRoleMap) {
if (tenantCohortRole.roleId) {
let getRoleName = await this.roleRepository.find({
where: { "roleId": tenantCohortRole?.roleId },
select: ["title"]
})

let getTenantName = await this.tenantsRepository.find({
where: { "tenantId": tenantCohortRole?.tenantId },
select: ["name"]
})
userRole = getRoleName.map(role => role?.title.toUpperCase()).join(', ')
userTenant = getTenantName.map(role => role?.name).join(', ')
}
}

let generatedUsername;
let generatePassword;

// Generate the username based on the role and the program
if (userRole && userRole === 'STUDENT') {
let fieldValues = userCreateDto ? userCreateDto?.customFields : [];
let suffix;
if (fieldValues) {
for (const fieldsData of fieldValues) {
let getFieldName = await this.fieldsService.getFieldByIdes(fieldsData?.fieldId);
if (getFieldName['name'] === 'program') {
suffix = fieldsData?.value
}
}
}

// Retrieve the maximum sequence number from existing usernames
const maxUsername = await this.usersRepository
.createQueryBuilder('user')
.select("MAX(CAST(REGEXP_REPLACE(user.username, '[^0-9]', '', 'g') AS INTEGER))", 'max')
.where("user.username ILIKE :usernamePattern", { usernamePattern: `${userTenant}%` })
.andWhere("REGEXP_REPLACE(user.username, '[^0-9]', '', 'g') <> ''")
.getRawOne();

const maxNumber = maxUsername?.max;
const newSequenceNumber = maxNumber + 1;

generatedUsername = `${userTenant}${newSequenceNumber}${suffix?.[0]?.toUpperCase() || ''}`;
generatePassword = `${generatedUsername}@${userTenant.toLowerCase() || ''}`
}


if (userRole && userRole === 'TEACHER') {
generatedUsername = await this.formatUsername(userCreateDto?.name);
generatePassword = `${generatedUsername}@${userTenant.toLowerCase() || ''}`
}

let loginCredintial = {
"username": generatedUsername,
"password": generatePassword
};
return loginCredintial;

} catch (error) {
return false;
}
}
Copy link

Choose a reason for hiding this comment

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

🛠️ Refactor suggestion

Improve security and efficiency in username/password generation.

The current implementation has several issues:

  1. Predictable username/password pattern could be a security risk
  2. Database queries inside loops may impact performance
  3. Minimal error handling
async generateUserNamePassword(userCreateDto: UserCreateDto) {
  try {
    const tenantCohortRoleMap = userCreateDto ? userCreateDto.tenantCohortRoleMapping : [];
    let userRole;
    let userTenant;
    
    // Get all roleIds and tenantIds first to reduce database calls
+   const roleIds = tenantCohortRoleMap.map(tcr => tcr.roleId).filter(Boolean);
+   const tenantIds = tenantCohortRoleMap.map(tcr => tcr.tenantId).filter(Boolean);
+   
+   if (roleIds.length === 0 || tenantIds.length === 0) {
+     throw new Error('Missing role or tenant information');
+   }
+   
+   // Fetch all roles and tenants in batch queries
+   const [roleNames, tenantNames] = await Promise.all([
+     this.roleRepository.find({
+       where: { roleId: In(roleIds) },
+       select: ["title"]
+     }),
+     this.tenantsRepository.find({
+       where: { tenantId: In(tenantIds) },
+       select: ["name"]
+     })
+   ]);
+   
+   userRole = roleNames.map(role => role?.title.toUpperCase()).join(', ');
+   userTenant = tenantNames.map(tenant => tenant?.name).join(', ');

    let generatedUsername;
    let generatePassword;

    // Add more randomness to password creation
+   const generateSecurePassword = (base) => {
+     const randomChars = [...Array(3)].map(() => String.fromCharCode(65 + Math.floor(Math.random() * 26))).join('');
+     return `${base}@${randomChars}${Math.floor(Math.random() * 100)}`;
+   };

    // Rest of the method with improved security...

Committable suggestion skipped: line range outside the PR's diff.

🧰 Tools
🪛 ESLint

[error] 1600-1600: 'tenantCohortRoleMap' is never reassigned. Use 'const' instead.

(prefer-const)


[error] 1600-1600: Replace ·?·userCreateDto.tenantCohortRoleMapping with ⏎········?·userCreateDto.tenantCohortRoleMapping⏎·······

(prettier/prettier)


[error] 1605-1605: 'getRoleName' is never reassigned. Use 'const' instead.

(prefer-const)


[error] 1606-1606: Replace "roleId" with roleId

(prettier/prettier)


[error] 1607-1607: Insert ,

(prettier/prettier)


[error] 1608-1608: Insert ;

(prettier/prettier)


[error] 1610-1610: 'getTenantName' is never reassigned. Use 'const' instead.

(prefer-const)


[error] 1611-1611: Replace "tenantId" with tenantId

(prettier/prettier)


[error] 1612-1612: Insert ,

(prettier/prettier)


[error] 1613-1613: Insert ;

(prettier/prettier)


[error] 1614-1614: Replace .map(role·=>·role?.title.toUpperCase()).join(',·') with ⏎············.map((role)·=>·role?.title.toUpperCase())⏎············.join(",·");

(prettier/prettier)


[error] 1615-1615: Replace role·=>·role?.name).join(',·') with (role)·=>·role?.name).join(",·");

(prettier/prettier)


[error] 1623-1623: Replace 'STUDENT' with "STUDENT"

(prettier/prettier)


[error] 1624-1624: 'fieldValues' is never reassigned. Use 'const' instead.

(prefer-const)


[error] 1628-1628: 'getFieldName' is never reassigned. Use 'const' instead.

(prefer-const)


[error] 1628-1628: Replace fieldsData?.fieldId with ⏎··············fieldsData?.fieldId⏎············

(prettier/prettier)


[error] 1629-1629: Replace 'name']·===·'program' with "name"]·===·"program"

(prettier/prettier)


[error] 1630-1630: Insert ;

(prettier/prettier)


[error] 1637-1637: Replace 'user' with "user"

(prettier/prettier)


[error] 1638-1638: Replace "MAX(CAST(REGEXP_REPLACE(user.username,·'[^0-9]',·'',·'g')·AS·INTEGER))",·'max' with ⏎············"MAX(CAST(REGEXP_REPLACE(user.username,·'[^0-9]',·'',·'g')·AS·INTEGER))",⏎············"max"⏎··········

(prettier/prettier)


[error] 1639-1639: Replace ·usernamePattern:·${userTenant}%`` with ⏎············usernamePattern:·${userTenant}%`,⏎·········`

(prettier/prettier)


[error] 1646-1646: Replace suffix?.[0]?.toUpperCase()·||·'' with ⏎··········suffix?.[0]?.toUpperCase()·||·""⏎········

(prettier/prettier)


[error] 1647-1647: Replace userTenant.toLowerCase()·||·''}`` with ⏎··········userTenant.toLowerCase()·||·""⏎········};

(prettier/prettier)


[error] 1648-1649: Delete

(prettier/prettier)


[error] 1651-1651: Replace 'TEACHER' with "TEACHER"

(prettier/prettier)


[error] 1653-1653: Replace userTenant.toLowerCase()·||·''}`` with ⏎··········userTenant.toLowerCase()·||·""⏎········};

(prettier/prettier)


[error] 1656-1656: 'loginCredintial' is never reassigned. Use 'const' instead.

(prefer-const)


[error] 1657-1657: Replace "username" with username

(prettier/prettier)


[error] 1658-1658: Replace "password":·generatePassword with password:·generatePassword,

(prettier/prettier)


[error] 1661-1662: Delete

(prettier/prettier)

Comment on lines +1639 to +1647
.where("user.username ILIKE :usernamePattern", { usernamePattern: `${userTenant}%` })
.andWhere("REGEXP_REPLACE(user.username, '[^0-9]', '', 'g') <> ''")
.getRawOne();

const maxNumber = maxUsername?.max;
const newSequenceNumber = maxNumber + 1;

generatedUsername = `${userTenant}${newSequenceNumber}${suffix?.[0]?.toUpperCase() || ''}`;
generatePassword = `${generatedUsername}@${userTenant.toLowerCase() || ''}`
Copy link

Choose a reason for hiding this comment

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

🛠️ Refactor suggestion

Add error handling for database queries.

The database query to find the maximum sequence number could fail, but there's no error handling for this case.

try {
  // Retrieve the maximum sequence number from existing usernames
  const maxUsername = await this.usersRepository
    .createQueryBuilder('user')
    .select("MAX(CAST(REGEXP_REPLACE(user.username, '[^0-9]', '', 'g') AS INTEGER))", 'max')
    .where("user.username ILIKE :usernamePattern", { usernamePattern: `${userTenant}%` })
    .andWhere("REGEXP_REPLACE(user.username, '[^0-9]', '', 'g') <> ''")
    .getRawOne();

-  const maxNumber = maxUsername?.max;
+  const maxNumber = maxUsername?.max || 0;
  const newSequenceNumber = maxNumber + 1;

  generatedUsername = `${userTenant}${newSequenceNumber}${suffix?.[0]?.toUpperCase() || ''}`;
  generatePassword = `${generatedUsername}@${userTenant.toLowerCase() || ''}`
+ } catch (error) {
+   // Fallback to a timestamp-based username if query fails
+   LoggerUtil.error(
+     'Error retrieving max username sequence',
+     error.message
+   );
+   generatedUsername = `${userTenant}${Date.now().toString(36)}`;
+   generatePassword = `${generatedUsername}@${userTenant.toLowerCase() || ''}`
}
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
.where("user.username ILIKE :usernamePattern", { usernamePattern: `${userTenant}%` })
.andWhere("REGEXP_REPLACE(user.username, '[^0-9]', '', 'g') <> ''")
.getRawOne();
const maxNumber = maxUsername?.max;
const newSequenceNumber = maxNumber + 1;
generatedUsername = `${userTenant}${newSequenceNumber}${suffix?.[0]?.toUpperCase() || ''}`;
generatePassword = `${generatedUsername}@${userTenant.toLowerCase() || ''}`
try {
// Retrieve the maximum sequence number from existing usernames
const maxUsername = await this.usersRepository
.createQueryBuilder('user')
.select(
"MAX(CAST(REGEXP_REPLACE(user.username, '[^0-9]', '', 'g') AS INTEGER))",
'max',
)
.where("user.username ILIKE :usernamePattern", {
usernamePattern: `${userTenant}%`,
})
.andWhere("REGEXP_REPLACE(user.username, '[^0-9]', '', 'g') <> ''")
.getRawOne();
const maxNumber = maxUsername?.max || 0;
const newSequenceNumber = maxNumber + 1;
generatedUsername = `${userTenant}${newSequenceNumber}${suffix?.[0]?.toUpperCase() || ''}`;
generatePassword = `${generatedUsername}@${userTenant.toLowerCase() || ''}`;
} catch (error) {
// Fallback to a timestamp-based username if query fails
LoggerUtil.error(
'Error retrieving max username sequence',
error.message,
);
generatedUsername = `${userTenant}${Date.now().toString(36)}`;
generatePassword = `${generatedUsername}@${userTenant.toLowerCase() || ''}`;
}
🧰 Tools
🪛 ESLint

[error] 1639-1639: Replace ·usernamePattern:·${userTenant}%`` with ⏎············usernamePattern:·${userTenant}%`,⏎·········`

(prettier/prettier)


[error] 1646-1646: Replace suffix?.[0]?.toUpperCase()·||·'' with ⏎··········suffix?.[0]?.toUpperCase()·||·""⏎········

(prettier/prettier)


[error] 1647-1647: Replace userTenant.toLowerCase()·||·''}`` with ⏎··········userTenant.toLowerCase()·||·""⏎········};

(prettier/prettier)

Comment on lines +833 to +882
async updateUserByName(userDto, response: Response){
const apiId = APIID.USER_UPDATE;
try {
let updatedData = {};
let editIssues = {};

let getUser = await this.usersRepository.find({where:{name: userDto.name}})
for(let user of getUser){

if (userDto?.customFields?.length > 0) {
const getFieldsAttributes = await this.fieldsService.getEditableFieldsAttributes();

let isEditableFieldId = [];
const fieldIdAndAttributes = {};
for (let fieldDetails of getFieldsAttributes) {
isEditableFieldId.push(fieldDetails.fieldId);
fieldIdAndAttributes[`${fieldDetails.fieldId}`] = fieldDetails;
}

let unEditableIdes = [];
let editFailures = [];
for (let data of userDto.customFields) {
if (isEditableFieldId.includes(data.fieldId)) {
const result = await this.fieldsService.updateCustomFields(user.userId, data, fieldIdAndAttributes[data.fieldId]);
if (result.correctValue) {
if (!updatedData['customFields'])
updatedData['customFields'] = [];
updatedData['customFields'].push(result);
} else {
editFailures.push(`${data.fieldId}: ${result?.valueIssue} - ${result.fieldName}`)
}
} else {
unEditableIdes.push(data.fieldId)
}
}
if (unEditableIdes.length > 0) {
editIssues["uneditableFields"] = unEditableIdes
}
if (editFailures.length > 0) {
editIssues["editFieldsFailure"] = editFailures
}
}
}

return await APIResponse.success(response, apiId, { ...updatedData, editIssues },
HttpStatus.OK, "User has been updated successfully.")
} catch (e) {
return APIResponse.error(response, apiId, "Internal Server Error", `${e}`, HttpStatus.INTERNAL_SERVER_ERROR);
}
}
Copy link

Choose a reason for hiding this comment

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

🛠️ Refactor suggestion

Add error handling for when no user is found.

The updateUserByName method doesn't handle the case where no users are found with the given name. If getUser is empty, the function will still return a success response but may not have updated anything.

async updateUserByName(userDto, response: Response){
  const apiId = APIID.USER_UPDATE;
  try {
    const updatedData = {};
    const editIssues = {};

    const getUser = await this.usersRepository.find({where: { name: userDto.name }});
+   if (getUser.length === 0) {
+     return APIResponse.error(
+       response,
+       apiId,
+       API_RESPONSES.NOT_FOUND,
+       `No users found with name: ${userDto.name}`,
+       HttpStatus.NOT_FOUND
+     );
+   }
    
    for(const user of getUser){
      // Existing code...
    }

    return await APIResponse.success(
      response, 
      apiId, 
      { ...updatedData, editIssues },
      HttpStatus.OK, 
      "User has been updated successfully."
    );
  } catch (e) {
    return APIResponse.error(
      response, 
      apiId, 
-     "Internal Server Error", 
+     API_RESPONSES.INTERNAL_SERVER_ERROR,
      `${e}`, 
      HttpStatus.INTERNAL_SERVER_ERROR
    );
  }
}
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
async updateUserByName(userDto, response: Response){
const apiId = APIID.USER_UPDATE;
try {
let updatedData = {};
let editIssues = {};
let getUser = await this.usersRepository.find({where:{name: userDto.name}})
for(let user of getUser){
if (userDto?.customFields?.length > 0) {
const getFieldsAttributes = await this.fieldsService.getEditableFieldsAttributes();
let isEditableFieldId = [];
const fieldIdAndAttributes = {};
for (let fieldDetails of getFieldsAttributes) {
isEditableFieldId.push(fieldDetails.fieldId);
fieldIdAndAttributes[`${fieldDetails.fieldId}`] = fieldDetails;
}
let unEditableIdes = [];
let editFailures = [];
for (let data of userDto.customFields) {
if (isEditableFieldId.includes(data.fieldId)) {
const result = await this.fieldsService.updateCustomFields(user.userId, data, fieldIdAndAttributes[data.fieldId]);
if (result.correctValue) {
if (!updatedData['customFields'])
updatedData['customFields'] = [];
updatedData['customFields'].push(result);
} else {
editFailures.push(`${data.fieldId}: ${result?.valueIssue} - ${result.fieldName}`)
}
} else {
unEditableIdes.push(data.fieldId)
}
}
if (unEditableIdes.length > 0) {
editIssues["uneditableFields"] = unEditableIdes
}
if (editFailures.length > 0) {
editIssues["editFieldsFailure"] = editFailures
}
}
}
return await APIResponse.success(response, apiId, { ...updatedData, editIssues },
HttpStatus.OK, "User has been updated successfully.")
} catch (e) {
return APIResponse.error(response, apiId, "Internal Server Error", `${e}`, HttpStatus.INTERNAL_SERVER_ERROR);
}
}
async updateUserByName(userDto, response: Response){
const apiId = APIID.USER_UPDATE;
try {
let updatedData = {};
let editIssues = {};
let getUser = await this.usersRepository.find({ where: { name: userDto.name } });
if (getUser.length === 0) {
return APIResponse.error(
response,
apiId,
API_RESPONSES.NOT_FOUND,
`No users found with name: ${userDto.name}`,
HttpStatus.NOT_FOUND
);
}
for (let user of getUser) {
// Existing code...
}
return await APIResponse.success(
response,
apiId,
{ ...updatedData, editIssues },
HttpStatus.OK,
"User has been updated successfully."
);
} catch (e) {
return APIResponse.error(
response,
apiId,
API_RESPONSES.INTERNAL_SERVER_ERROR,
`${e}`,
HttpStatus.INTERNAL_SERVER_ERROR
);
}
}
🧰 Tools
🪛 ESLint

[error] 833-833: Insert ·

(prettier/prettier)


[error] 836-836: 'updatedData' is never reassigned. Use 'const' instead.

(prefer-const)


[error] 837-837: 'editIssues' is never reassigned. Use 'const' instead.

(prefer-const)


[error] 839-839: 'getUser' is never reassigned. Use 'const' instead.

(prefer-const)


[error] 839-839: Replace where:{name:·userDto.name}}) with ⏎········where:·{·name:·userDto.name·},⏎······});

(prettier/prettier)


[error] 840-841: Replace (let·user·of·getUser){⏎ with ·(let·user·of·getUser)·{

(prettier/prettier)


[error] 840-840: 'user' is never reassigned. Use 'const' instead.

(prefer-const)


[error] 843-843: Insert ⏎···········

(prettier/prettier)


[error] 844-844: Delete ··

(prettier/prettier)


[error] 845-845: 'isEditableFieldId' is never reassigned. Use 'const' instead.

(prefer-const)


[error] 847-847: 'fieldDetails' is never reassigned. Use 'const' instead.

(prefer-const)


[error] 851-851: Delete ··

(prettier/prettier)


[error] 852-852: 'unEditableIdes' is never reassigned. Use 'const' instead.

(prefer-const)


[error] 853-853: 'editFailures' is never reassigned. Use 'const' instead.

(prefer-const)


[error] 854-854: 'data' is never reassigned. Use 'const' instead.

(prefer-const)


[error] 856-856: Replace user.userId,·data,·fieldIdAndAttributes[data.fieldId] with ⏎················user.userId,⏎················data,⏎················fieldIdAndAttributes[data.fieldId]⏎··············

(prettier/prettier)


[error] 858-858: Replace 'customFields' with "customFields"

(prettier/prettier)


[error] 859-859: Replace 'customFields' with "customFields"

(prettier/prettier)


[error] 860-860: Replace 'customFields' with "customFields"

(prettier/prettier)


[error] 862-862: Replace ``${data.fieldId}:·${result?.valueIssue}·-·${result.fieldName}) with `⏎··················`${data.fieldId}:·${result?.valueIssue}·-·${result.fieldName}`⏎················);`

(prettier/prettier)


[error] 865-865: Insert ;

(prettier/prettier)


[error] 869-869: Insert ;

(prettier/prettier)


[error] 872-872: Insert ;

(prettier/prettier)


[error] 877-877: Replace response,·apiId, with ⏎········response,⏎········apiId,⏎·······

(prettier/prettier)


[error] 878-878: Replace ·"User·has·been·updated·successfully.") with ⏎········"User·has·been·updated·successfully."⏎······);

(prettier/prettier)


[error] 880-880: Replace response,·apiId,·"Internal·Server·Error",·${e},·HttpStatus.INTERNAL_SERVER_ERROR with ⏎········response,⏎········apiId,⏎········"Internal·Server·Error",⏎········${e},⏎········HttpStatus.INTERNAL_SERVER_ERROR⏎······

(prettier/prettier)

@Tusharmahajan12 Tusharmahajan12 changed the title New useroblf Task #240139 Merge lateMarkingForSelfAttendance to release-1.0.1 branch of userservice May 5, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants