428 move reputation score from nft to platform route#430
Conversation
WalkthroughThis pull request removes NFT-related functionality and associated environment variables from the codebase while introducing a new platform endpoint for reputation score retrieval. It reorganizes controller imports/exports, deletes obsolete NFT controllers, routes, services, and validations, and adds new functions and validation schemas for the platform reputation score. Additionally, related documentation and configuration changes have been made across multiple modules with some import reorganization in interfaces and middleware. Changes
Sequence Diagram(s)sequenceDiagram
participant C as Client
participant R as PlatformRoute
participant A as Auth Middleware
participant V as Validation Middleware
participant PC as PlatformController
participant PS as PlatformService
participant RS as ReputationScoreService
participant DB as Neo4j Database
C->>R: GET /:platformId/reputation-score
R->>A: Authenticate request
A-->>R: Authenticated
R->>V: Validate platformId
V-->>R: Validated
R->>PC: Call getReputationScore()
PC->>PS: Invoke getReputationScore(platform, userId)
PS->>RS: Calculate reputation score for user
RS->>DB: Execute reputation score query
DB-->>RS: Return query result
RS-->>PS: Return calculated score
PS-->>PC: Return reputation score
PC-->>R: Send response with score
R-->>C: Return reputation score in JSON
Possibly related PRs
Poem
✨ Finishing Touches
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
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)
Other keywords and placeholders
CodeRabbit Configuration File (
|
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (4)
src/services/reputationScore.service.ts (2)
12-24: Consider adding error handling for the Neo4j read operation.
Currently, ifNeo4j.readfails or throws an error, the function doesn’t handle the exception. Adding a try/catch block or higher-level error handling would make the service more robust.async function calculateReputationScoreForUser( platform: HydratedDocument<IPlatform>, userId: ObjectId, ): Promise<number> { const platformName = platform.name as SupportedNeo4jPlatforms; const memberLabel = NEO4J_PLATFORM_INFO[platformName].member; const platformId = platform.id; const reputationScoreQuery = buildReputationScoreQuery(userId, platformId, memberLabel); - const neo4jData = await Neo4j.read(reputationScoreQuery); + let neo4jData; + try { + neo4jData = await Neo4j.read(reputationScoreQuery); + } catch (err) { + logger.error('Error while reading from Neo4j', err); + throw err; + } return extractReputationScoreFromNeo4jData(neo4jData); }
38-50: Avoid logging full Neo4j records in debug mode to prevent potential PII/sensitive data leakage.
While it’s useful for debugging, consider filtering or redacting sensitive fields fromrecordsbefore logging. Also confirm the record set never contains user-sensitive data.logger.debug(`Neo4j Records: ${JSON.stringify(records)}`); - if (records.length === 0) { + if (!records || records.length === 0) { return 0; }src/docs/platform.doc.yml (2)
564-566: Refine Endpoint Summary Naming
The summary currently reads “Retrieve reputation scores for given platformId.” Since the response schema returns a single reputation score (a single integer field), consider changing the summary to “Retrieve reputation score for a given platformId” for clarity and consistency.
582-588: Ensure Correct YAML Indentation for the Response Schema
The success response’s schema block appears mis-indented. For proper YAML parsing and documentation rendering, the “type” and “properties” keys should be correctly nested under “schema:”. Please verify and adjust the indentation accordingly.Suggested diff:
- content: - application/json: - schema: - type: object - properties: - reputationScore: - type: integer - example: 1 + content: + application/json: + schema: + type: object + properties: + reputationScore: + type: integer + example: 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (19)
src/config/index.ts(0 hunks)src/controllers/index.ts(1 hunks)src/controllers/nft.controller.ts(0 hunks)src/controllers/platform.controller.ts(4 hunks)src/docs/nft.yml(0 hunks)src/docs/platform.doc.yml(2 hunks)src/interfaces/Request.interface.ts(1 hunks)src/middlewares/auth.ts(1 hunks)src/routes/v1/index.ts(1 hunks)src/routes/v1/nft.route.ts(0 hunks)src/routes/v1/platform.route.ts(2 hunks)src/services/index.ts(2 hunks)src/services/nft.service.ts(0 hunks)src/services/oci.service.ts(0 hunks)src/services/platform.service.ts(3 hunks)src/services/reputationScore.service.ts(1 hunks)src/validations/index.ts(1 hunks)src/validations/nft.validation.ts(0 hunks)src/validations/platform.validation.ts(3 hunks)
💤 Files with no reviewable changes (7)
- src/validations/nft.validation.ts
- src/config/index.ts
- src/docs/nft.yml
- src/controllers/nft.controller.ts
- src/routes/v1/nft.route.ts
- src/services/oci.service.ts
- src/services/nft.service.ts
✅ Files skipped from review due to trivial changes (2)
- src/interfaces/Request.interface.ts
- src/middlewares/auth.ts
🔇 Additional comments (15)
src/validations/index.ts (2)
1-11: LGTM! Import changes align with PR objectives.The changes correctly remove NFT-related imports while adding necessary validation imports for the platform route, aligning with the PR's goal of moving reputation score functionality.
13-25: LGTM! Export changes maintain consistency.The export section is well-organized:
- All imported validations are properly exported
- Exports maintain alphabetical order
- NFT validation is correctly removed
src/controllers/index.ts (2)
14-27: LGTM! Export structure is clean and consistent.The export block properly reflects the changes in imports, with NFT controller removed and all necessary controllers exported.
1-13:Details
❓ Verification inconclusive
LGTM! Verify complete removal of NFT references.
The import changes align with moving away from NFT functionality. The imports are well-organized and mostly follow alphabetical ordering.
Let's verify that all NFT-related code has been properly removed:
🏁 Script executed:
#!/bin/bash # Search for any remaining NFT-related files or references echo "Checking for NFT-related files..." fd -t f "nft" echo "Checking for NFT-related code references..." rg -i "nft|reputation.?score"Length of output: 4895
LGTM for src/controllers/index.ts – NFT Controller removed and imports correctly organized.
The file under review no longer references an NFT controller, and the import ordering is robust. However, our repository-wide search revealed lingering NFT/dynamicNft references in documentation, validations, and service files (e.g., in
src/docs/module.doc.yml,src/validations/module.validation.ts, and others). Please verify if these residual references are expected or if further cleanup is needed.src/services/reputationScore.service.ts (2)
1-10: Imports and logger initialization look good.
No issues identified with these initial lines. They correctly set up the logger context and import the required modules.
52-54: Export strategy looks fine.
Exporting only the primary function is good for encapsulation. If needed, you can expose helper functions or keep them private.src/services/index.ts (2)
1-18: Refactored imports appear correct.
The reintroduction of services and removal of NFT references aligns with the PR’s objectives. No issues found here.
38-38: Newly added export for reputationScoreService is consistent.
This ensures the service is globally accessible.src/routes/v1/index.ts (2)
2-16: New and updated imports for routes look consistent.
The removal of any NFT references and addition of relevant route imports for announcements, categories, communities, etc., align with the PR’s stated objectives.
19-73: Routes array updated to remove NFT endpoint.
Replacing NFT routes with platform-related routes is in line with the mission to move reputation score from NFT to platform. Implementation looks smooth.src/routes/v1/platform.route.ts (1)
43-48: LGTM! Well-structured route implementation.The route is properly secured with authentication middleware and includes validation. The endpoint follows RESTful conventions.
src/validations/platform.validation.ts (1)
174-178: LGTM! Well-structured validation schema.The validation schema correctly validates the platformId parameter using the custom objectId validation.
src/services/platform.service.ts (1)
289-305: LGTM! Well-structured validation function.The function correctly prevents updates during analysis and initial data fetch for Discord platforms.
src/controllers/platform.controller.ts (2)
326-326: LGTM! Good separation of concerns.The validation logic has been correctly moved to the service layer.
388-391: LGTM! Well-structured controller function.The function correctly uses catchAsync for error handling and follows the controller pattern.
| function buildReputationScoreQuery(userId: ObjectId, platformId: string, memberLabel: string): string { | ||
| return ` | ||
| MATCH (:${memberLabel} {id: "${userId}"})-[r:HAVE_METRICS {platformId: "${platformId}"}]->(a) | ||
| WITH r.date as metrics_date, r.closenessCentrality as memberScore | ||
| ORDER BY metrics_date DESC | ||
| LIMIT 1 | ||
| MATCH (user:${memberLabel})-[user_r:HAVE_METRICS {platformId: "${platformId}", date: metrics_date}]->(user) | ||
| WITH memberScore, MAX(user_r.closenessCentrality) as maxScore | ||
| RETURN memberScore / maxScore AS reputation_score | ||
| `; | ||
| } |
There was a problem hiding this comment.
Parameterize the dynamic Cypher query to mitigate potential injection attacks.
Relying on string interpolation for IDs can introduce security vulnerabilities if user-supplied identifiers are not controlled. Switching to parameterized queries provides a safer, cleaner approach.
function buildReputationScoreQuery(userId: ObjectId, platformId: string, memberLabel: string): string {
- return `
- MATCH (:${memberLabel} {id: "${userId}"})-[r:HAVE_METRICS {platformId: "${platformId}"}]->(a)
- WITH r.date as metrics_date, r.closenessCentrality as memberScore
- ORDER BY metrics_date DESC
- LIMIT 1
- MATCH (user:${memberLabel})-[user_r:HAVE_METRICS {platformId: "${platformId}", date: metrics_date}]->(user)
- WITH memberScore, MAX(user_r.closenessCentrality) as maxScore
- RETURN memberScore / maxScore AS reputation_score
- `;
+ return `
+ MATCH (m:${memberLabel} {id: $userId})-[r:HAVE_METRICS {platformId: $platformId}]->(a)
+ WITH r.date as metrics_date, r.closenessCentrality as memberScore
+ ORDER BY metrics_date DESC
+ LIMIT 1
+ MATCH (user:${memberLabel})-[user_r:HAVE_METRICS {platformId: $platformId, date: metrics_date}]->(user)
+ WITH memberScore, MAX(user_r.closenessCentrality) as maxScore
+ RETURN memberScore / maxScore AS reputation_score
+ `;
}In the calling code, pass parameters to Neo4j.read accordingly:
-const neo4jData = await Neo4j.read(reputationScoreQuery);
+const neo4jData = await Neo4j.read(reputationScoreQuery, {
+ userId: userId.toString(),
+ platformId,
+});📝 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.
| function buildReputationScoreQuery(userId: ObjectId, platformId: string, memberLabel: string): string { | |
| return ` | |
| MATCH (:${memberLabel} {id: "${userId}"})-[r:HAVE_METRICS {platformId: "${platformId}"}]->(a) | |
| WITH r.date as metrics_date, r.closenessCentrality as memberScore | |
| ORDER BY metrics_date DESC | |
| LIMIT 1 | |
| MATCH (user:${memberLabel})-[user_r:HAVE_METRICS {platformId: "${platformId}", date: metrics_date}]->(user) | |
| WITH memberScore, MAX(user_r.closenessCentrality) as maxScore | |
| RETURN memberScore / maxScore AS reputation_score | |
| `; | |
| } | |
| function buildReputationScoreQuery(userId: ObjectId, platformId: string, memberLabel: string): string { | |
| return ` | |
| MATCH (m:${memberLabel} {id: $userId})-[r:HAVE_METRICS {platformId: $platformId}]->(a) | |
| WITH r.date as metrics_date, r.closenessCentrality as memberScore | |
| ORDER BY metrics_date DESC | |
| LIMIT 1 | |
| MATCH (user:${memberLabel})-[user_r:HAVE_METRICS {platformId: $platformId, date: metrics_date}]->(user) | |
| WITH memberScore, MAX(user_r.closenessCentrality) as maxScore | |
| RETURN memberScore / maxScore AS reputation_score | |
| `; | |
| } | |
| // In the calling code, update the invocation of Neo4j.read: | |
| const neo4jData = await Neo4j.read(reputationScoreQuery, { | |
| userId: userId.toString(), | |
| platformId, | |
| }); |
| /** | ||
| * get reputation score | ||
| * @param {IPlatform} platform | ||
| * @param {IUser} user | ||
| */ | ||
| const getReputationScore = async (platform: HydratedDocument<IPlatform>, userId: ObjectId) => { | ||
| return { | ||
| reputationScore: (await reputationScoreService.calculateReputationScoreForUser(platform, userId)) * 100, | ||
| }; | ||
| }; |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Add error handling and improve documentation.
- Add error handling for null/undefined platform or userId.
- Complete the JSDoc by adding parameter types.
Apply this diff to improve the implementation:
/**
* get reputation score
- * @param {IPlatform} platform
- * @param {IUser} user
+ * @param {HydratedDocument<IPlatform>} platform - The platform document
+ * @param {ObjectId} userId - The user's ObjectId
+ * @throws {ApiError} - When platform or userId is null/undefined
*/
const getReputationScore = async (platform: HydratedDocument<IPlatform>, userId: ObjectId) => {
+ if (!platform || !userId) {
+ throw new ApiError(httpStatus.BAD_REQUEST, 'Platform and userId are required');
+ }
return {
reputationScore: (await reputationScoreService.calculateReputationScoreForUser(platform, userId)) * 100,
};
};📝 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.
| /** | |
| * get reputation score | |
| * @param {IPlatform} platform | |
| * @param {IUser} user | |
| */ | |
| const getReputationScore = async (platform: HydratedDocument<IPlatform>, userId: ObjectId) => { | |
| return { | |
| reputationScore: (await reputationScoreService.calculateReputationScoreForUser(platform, userId)) * 100, | |
| }; | |
| }; | |
| /** | |
| * get reputation score | |
| * @param {HydratedDocument<IPlatform>} platform - The platform document | |
| * @param {ObjectId} userId - The user's ObjectId | |
| * @throws {ApiError} - When platform or userId are null/undefined | |
| */ | |
| const getReputationScore = async (platform: HydratedDocument<IPlatform>, userId: ObjectId) => { | |
| if (!platform || !userId) { | |
| throw new ApiError(httpStatus.BAD_REQUEST, 'Platform and userId are required'); | |
| } | |
| return { | |
| reputationScore: (await reputationScoreService.calculateReputationScoreForUser(platform, userId)) * 100, | |
| }; | |
| }; |
| '200': | ||
| description: Platform created successfully. | ||
| content: |
There was a problem hiding this comment.
Update Response Description for Accuracy
The 200-response description currently states “Platform created successfully.” This is misleading for a GET request intended to retrieve a reputation score. Please update the description to “Reputation score retrieved successfully.”
Suggested change:
- description: Platform created successfully.
+ description: Reputation score retrieved successfully.📝 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.
| '200': | |
| description: Platform created successfully. | |
| content: | |
| '200': | |
| description: Reputation score retrieved successfully. | |
| content: |
Summary by CodeRabbit
New Features
Documentation
Refactor