Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

add user chat status, set user chat status #4978

Merged
merged 5 commits into from
Feb 21, 2024
Merged

Conversation

nandinboldn
Copy link
Contributor

No description provided.

Copy link

@sourcery-ai sourcery-ai bot left a comment

Choose a reason for hiding this comment

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

PR Type: Enhancement

PR Summary: This pull request introduces the functionality to manage user chat statuses within the system. It includes the addition of a new field chatStatus to the user model, an enumeration to define possible chat statuses (online, offline), and a new mutation to set a user's chat status. The changes span across schema definitions, database model updates, and GraphQL mutations, aiming to enhance real-time user interaction capabilities by tracking and updating user availability.

Decision: Comment

📝 Type: 'Enhancement' - not supported yet.
  • Sourcery currently only approves 'Typo fix' PRs.
✅ Issue addressed: this change correctly addresses the issue or implements the desired feature.
No details provided.
✅ Small diff: the diff is small enough to approve with confidence.
No details provided.

General suggestions:

  • Consider aligning the chatStatus field type across all instances to use the IUserChatStatus enum. This ensures type safety and consistency within the codebase.
  • Review the implementation of the usersSetChatStatus mutation to ensure it properly handles the chat status updates, including error handling for invalid user IDs or chat statuses.
  • Ensure comprehensive testing around the new functionality, particularly focusing on edge cases such as attempting to set a chat status for a non-existent user or using an invalid chat status value.
packages/api-utils/src/definitions/users.ts: The code changes enhance clarity and maintainability by introducing an enum for predefined values without significantly increasing complexity.

After reviewing the changes, it's clear that the addition of the chatStatus field and the IUserChatStatus enum does not significantly increase the complexity of the code. On the contrary, these changes are in line with good practices for software development. The use of enums, in particular, enhances code readability and maintainability by ensuring that variables have one of only a few predefined values. This approach helps prevent errors and makes the codebase easier to understand. The modifications are well-contained, extending the functionality without altering the existing structure or logic significantly. Overall, these changes are a positive step towards keeping the codebase robust and maintainable.

packages/core/src/data/resolvers/mutations/users.ts: Consider simplifying the `usersSetChatStatus` function by combining user existence verification and update into a single operation to reduce complexity.

The addition of the usersSetChatStatus function introduces a higher level of complexity due to the asynchronous nature of the operation, explicit error handling for user existence, and multiple direct database interactions. While these elements are essential for the functionality, we might consider ways to streamline the implementation to align with our codebase's existing patterns and reduce complexity.

One approach could be to encapsulate the user existence check and the update operation into a single database interaction, if our database and ORM support such operations. This could potentially reduce the complexity by minimizing the number of asynchronous calls and simplifying error handling. For example:

async usersSetChatStatus(
  _root,
  { _id, status }: { _id: string; status: string },
  { models }: IContext,
) {
  try {
    const updatedUser = await models.Users.updateUserIfExist(_id, { chatStatus: status });
    if (!updatedUser) {
      throw new Error('User not found');
    }
    return updatedUser;
  } catch (error) {
    // Handle error based on the codebase's standard practice
    throw error;
  }
},

This hypothetical updateUserIfExist method would streamline the process by combining the check and update into a single step, assuming it returns null or a similar falsy value if the user does not exist. This change could help maintain the readability and maintainability of our codebase by keeping the asynchronous operations and error handling patterns consistent.

Thanks for using Sourcery. We offer it for free for open source projects and would be very grateful if you could help us grow. If you like it, would you consider sharing Sourcery on your favourite social media? ✨

Share Sourcery

Help me be more useful! Please click 👍 or 👎 on each comment to tell me if it was helpful.

@@ -29,6 +29,7 @@ const SALT_WORK_FACTOR = 10;
interface IEditProfile {
username?: string;
email?: string;
chatStatus?: string;
Copy link

Choose a reason for hiding this comment

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

suggestion (llm): The addition of chatStatus to the IEditProfile interface is a good move for extending user profile functionalities. However, considering the enum IUserChatStatus defined in packages/api-utils/src/definitions/users.ts, it might be more type-safe to use this enum here instead of a generic string. This ensures that only valid statuses can be assigned.

@@ -532,6 +532,20 @@ const userMutations = {
) {
return models.Users.configGetNotificationByEmail(user._id, isAllowed);
},

async usersSetChatStatus(
Copy link

Choose a reason for hiding this comment

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

suggestion (llm): The implementation of usersSetChatStatus mutation is a valuable addition for real-time user status updates. It's important to ensure that the status parameter aligns with the IUserChatStatus enum to maintain consistency and avoid potential mismatches.

Copy link

sonarcloud bot commented Feb 21, 2024

Quality Gate Failed Quality Gate failed

Failed conditions
C Reliability Rating on New Code (required ≥ A)

See analysis details on SonarCloud

idea Catch issues before they fail your Quality Gate with our IDE extension SonarLint SonarLint

@nandinboldn nandinboldn merged commit 7a5a05f into dev Feb 21, 2024
62 of 66 checks passed
enkhtuvshinD pushed a commit that referenced this pull request Feb 21, 2024
* add user chat status (online | offline ), set user chat status
* add chat message reaction, remove reaction
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.

None yet

1 participant