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.

Sign up for GitHub

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

Jump to bottom

refactor: Fetch user profile API #368

Merged
merged 7 commits into from
Dec 26, 2023
Merged

refactor: Fetch user profile API #368

merged 7 commits into from
Dec 26, 2023

Conversation

pranalidhanavade
Copy link
Contributor

@pranalidhanavade pranalidhanavade commented Dec 21, 2023 •
edited
Loading

  • Refactored the GET API /users/profileand GET API /users/activity

What

  1. Implemented a well-defined interface by replacing previous existing Promise<object>.
  2. Ensured consistency of API status codes.
  3. Modified the NATS function.
  4. Tested with both positive and negative scenarios.

How

  1. Replaced the return type with a well-defined interface to enhance code clarity.
  2. Checked and standardized API status codes to ensure a uniform response.
  3. Modified the NATS function to accommodate the adjustments made in the API.

Why

  1. The implementation of a well-defined interface enhances code readability and makes the structure of the API response more explicit.
  2. Ensuring consistent API status codes improves communication between different components of the system and enhances overall reliability.

@pranalidhanavade
refactor:fetch user profile api
781884a 
Signed-off-by: pranalidhanavade <pranali.dhanavade@ayanworks.com>
@pranalidhanavade pranalidhanavade self-assigned this Dec 21, 2023
@pranalidhanavade
refactor:users activity api
23f6ed7 
Signed-off-by: pranalidhanavade <pranali.dhanavade@ayanworks.com>
vivekayanworks
vivekayanworks reviewed Dec 21, 2023
View reviewed changes
apps/api-gateway/src/user/user.controller.ts Outdated
@@ -69,6 +70,7 @@ export class UserController {
* @returns Users list of organization
*/
@Get('/public-profiles')
@ApiExcludeEndpoint()
@ApiResponse({ status: 200, description: 'Success', type: ApiResponseDto })
Copy link
Contributor

Choose a reason for hiding this comment

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

Let's get status code from "HttpStatus" for example, for 200 we can use "HttpStatus.OK"

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Resolved

apps/api-gateway/src/user/user.service.ts Outdated
@@ -9,16 +9,18 @@ import { UpdateUserProfileDto } from './dto/update-user-profile.dto';
import { AddPasskeyDetails } from './dto/add-user.dto';
import { UpdatePlatformSettingsDto } from './dto/update-platform-settings.dto';
import { CreateUserCertificateDto } from './dto/share-certificate.dto';
import { UsersProfile } from 'apps/user/interfaces/user.interface';
Copy link
Contributor

Choose a reason for hiding this comment

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

Let's have consistency in interface names. We have decided to use "I" prefix and then interface name.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Resolved.

apps/user/interfaces/user.interface.ts Outdated
@@ -23,14 +23,35 @@ export interface Organisation {
}

export interface UsersProfile {
Copy link
Contributor

Choose a reason for hiding this comment

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

At least one field should be mandatory. If there is an requirement for having all fields optional, let's discuss and finalize.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Resolved.

apps/user/interfaces/user.interface.ts Outdated
userId: string;
orgRoleId: string;
orgId: string;
orgRole: {
Copy link
Contributor

Choose a reason for hiding this comment

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

Let's have OrgRole as an interface instead of object.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Resolved

apps/user/interfaces/user.interface.ts Outdated
name: string;
description: string;
};
organisation: {
Copy link
Contributor

Choose a reason for hiding this comment

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

organization can be an interface.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Resolved

apps/user/interfaces/user.interface.ts Outdated
@@ -23,14 +23,35 @@ export interface Organisation {
}

export interface UsersProfile {
Copy link
Contributor

Choose a reason for hiding this comment

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

Let's follow naming convention for interfaces.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Resolved

apps/user/interfaces/user.interface.ts Outdated
userOrgRoles?: UserOrgRole[];
}

interface UserOrgRole {
Copy link
Contributor

Choose a reason for hiding this comment

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

Let's follow naming convention for interfaces.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Resolved

apps/user/interfaces/user.interface.ts Outdated
@@ -48,7 +69,7 @@ export interface UsersProfile {
username?: string;
}

export interface userInfo {
export interface UserInfo {
Copy link
Contributor

Choose a reason for hiding this comment

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

Let's follow naming convention for interfaces.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Resolved

apps/user/repositories/user.repository.ts Outdated
@@ -254,7 +254,7 @@ export class UserRepository {
});
}

async findUserForPublicProfile(queryOptions: UserQueryOptions): Promise<UsersProfile> {
async findUserForPublicProfile(queryOptions: UserQueryOptions): Promise<object> {
Copy link
Contributor

Choose a reason for hiding this comment

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

Why we have replaced interface with object?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I have resolved this.

@pranalidhanavade
fix:comments on user profile api
71ea39a 
Signed-off-by: pranalidhanavade <pranali.dhanavade@ayanworks.com>
@pranalidhanavade
Merge branch 'nats-refactor' of https://github.com/credebl/platform i…
2ff012f 
…nto error-handling-of-user-module
@pranalidhanavade
fix:resolved comments of get user profile api
a55992d 
Signed-off-by: pranalidhanavade <pranali.dhanavade@ayanworks.com>
@pranalidhanavade
fix:resolved comments of get user profile api
89baf0e 
Signed-off-by: pranalidhanavade <pranali.dhanavade@ayanworks.com>
@pranalidhanavade
refactor:user activity api summary and description
4b8a8a5 
Signed-off-by: pranalidhanavade <pranali.dhanavade@ayanworks.com>
@sonarqubecloud SonarQubeCloud
Copy link

Quality Gate Passed Quality Gate passed

Kudos, no new issues were introduced!

0 New issues
0 Security Hotspots
No data about Coverage
0.0% Duplication on New Code

See analysis details on SonarCloud

@pranalidhanavade pranalidhanavade linked an issue Dec 23, 2023 that may be closed by this pull request
Improve error handling for APIs of User, Auth modules #353
Closed
14 tasks
ankita-p17
ankita-p17 reviewed Dec 26, 2023
View reviewed changes
apps/api-gateway/src/user/user.service.ts
const payload = { userId, limit };
return this.sendNats(this.serviceProxy, 'get-user-activity', payload);
return this.sendNatsMessage(this.serviceProxy, 'get-user-activity', payload);
Copy link
Contributor

Choose a reason for hiding this comment

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

Please keep all the constants at one place and use in the application, enum can be used.

Copy link
Contributor

Choose a reason for hiding this comment

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

As discussed with team, we can take this activity in upcoming sprint as this is a good to have practice.

@vivekayanworks vivekayanworks merged commit d560ce4 into nats-refactor Dec 26, 2023
2 checks passed
@vivekayanworks vivekayanworks deleted the error-handling-of-user-module branch December 26, 2023 11:06
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@ankita-p17 ankita-p17 ankita-p17 left review comments

@vivekayanworks vivekayanworks vivekayanworks approved these changes

Assignees

@pranalidhanavade pranalidhanavade

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Improve error handling for APIs of User, Auth modules
3 participants
@pranalidhanavade @ankita-p17 @vivekayanworks