fix: fix bugs #408
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,102 +1,17 @@ | ||
| import { Request, Response } from 'express'; | ||
| import { catchAsync } from '../utils'; | ||
| import parentLogger from '../config/logger'; | ||
| import { nftService } from '../services'; | ||
|
|
||
| const logger = parentLogger.child({ module: 'NftController' }); | ||
|
|
||
| const getReputationScore = catchAsync(async function (req: Request, res: Response) { | ||
| const { tokenId, address } = req.params; | ||
| logger.debug(tokenId, address); | ||
| const reputationScore = await nftService.getReputationScore(tokenId, address); | ||
| res.send(reputationScore); | ||
| }); | ||
|
|
||
| export default { | ||
| getReputationScore, | ||
| }; | ||
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change | ||||||
|---|---|---|---|---|---|---|---|---|
| @@ -1,35 +1,112 @@ | ||||||||
| import { HydratedDocument, Types } from 'mongoose'; | ||||||||
| import httpStatus from 'http-status'; | ||||||||
| import { IPlatform, IModule, PlatformNames } from '@togethercrew.dev/db'; | ||||||||
| import ApiError from '../utils/ApiError'; | ||||||||
| import * as Neo4j from '../neo4j'; | ||||||||
| import { NEO4J_PLATFORM_INFO } from '../constants/neo4j.constant'; | ||||||||
| import { SupportedNeo4jPlatforms } from '../types/neo4j.type'; | ||||||||
| import parentLogger from '../config/logger'; | ||||||||
| import moduleService from './module.service'; | ||||||||
| import platformService from './platform.service'; | ||||||||
| import ociService from './oci.service'; | ||||||||
|
|
||||||||
| const logger = parentLogger.child({ module: 'NftService' }); | ||||||||
|
|
||||||||
| const supportedPlatforms = [PlatformNames.Discord, PlatformNames.Discourse]; | ||||||||
|
|
||||||||
| /** | ||||||||
| * get reputation score | ||||||||
| * @param {IPlatform} PlatformBody | ||||||||
| * @returns {Promise<HydratedDocument<IPlatform>>} | ||||||||
| */ | ||||||||
| const getReputationScore = async (tokenId: string, address: string) => { | ||||||||
| const dynamicNftModule = await moduleService.getModuleByFilter({ 'options.platforms.0.metadata.tokenId': tokenId }); | ||||||||
| logger.debug(dynamicNftModule); | ||||||||
| throwErrorIfDynamicNftModuleDoesNotExist(dynamicNftModule); | ||||||||
|
|
||||||||
| const profiles: Array<any> = await getProfiles(address); | ||||||||
| logger.debug(profiles); | ||||||||
| throwErrorIfUserHasNoOnChainProfiles(profiles); | ||||||||
|
|
||||||||
| let reputationScore = 0; | ||||||||
| for (const profile of profiles) { | ||||||||
| if (supportedPlatforms.includes(profile.profile.provider)) { | ||||||||
| const platform = await platformService.getPlatformByFilter({ | ||||||||
| name: profile.profile.provider, | ||||||||
| community: dynamicNftModule?.community, | ||||||||
| }); | ||||||||
| if (platform) { | ||||||||
| reputationScore = (await calculateReputationScoreForProfile(profile, platform)) + reputationScore; | ||||||||
|
There was a problem hiding this comment. Fix reputation score calculation logic The current implementation adds reputation scores together without averaging, which could lead to inflated scores for users with multiple profiles. Apply this fix: - reputationScore = (await calculateReputationScoreForProfile(profile, platform)) + reputationScore;
+ const profileScore = await calculateReputationScoreForProfile(profile, platform);
+ reputationScore = Math.max(reputationScore, profileScore); // Take highest score across platforms📝 Committable suggestion
Suggested change
|
||||||||
| logger.debug(`Reputation Score: ${reputationScore}`); | ||||||||
| } | ||||||||
| } | ||||||||
| } | ||||||||
| return { | ||||||||
| reputationScore, | ||||||||
| communintyId: dynamicNftModule?.id, | ||||||||
|
There was a problem hiding this comment. Fix typo in property key 'communintyId' The property key Apply this diff to correct the typo: return {
reputationScore,
- communintyId: dynamicNftModule?.id,
+ communityId: dynamicNftModule?.id,
};
|
||||||||
| }; | ||||||||
| }; | ||||||||
|
|
||||||||
| async function getProfiles(address: string) { | ||||||||
| let profiles: Array<any> = []; | ||||||||
| const supportedChainIds = [11155111]; | ||||||||
|
There was a problem hiding this comment. 🛠️ Refactor suggestion Move chain ID to configuration The supported chain ID is hard-coded. This should be moved to a configuration file for better maintainability. Create a config file and move the chain IDs there: // src/config/blockchain.config.ts
export const SUPPORTED_CHAIN_IDS = {
SEPOLIA: 11155111,
// Add other chains as needed
}; |
||||||||
| for (let i = 0; i < supportedChainIds.length; i++) { | ||||||||
| const chainProfiles = await ociService.getProfiles(address, supportedChainIds[i]); | ||||||||
| profiles = profiles.concat(chainProfiles); | ||||||||
|
Comment on lines
+54
to
+55
There was a problem hiding this comment. 🛠️ Refactor suggestion Add error handling for OCI service calls Exceptions from Example implementation: for (let i = 0; i < supportedChainIds.length; i++) {
- const chainProfiles = await ociService.getProfiles(address, supportedChainIds[i]);
- profiles = profiles.concat(chainProfiles);
+ try {
+ const chainProfiles = await ociService.getProfiles(address, supportedChainIds[i]);
+ profiles = profiles.concat(chainProfiles);
+ } catch (error) {
+ logger.error(`Error fetching profiles for chain ID ${supportedChainIds[i]}: ${error.message}`);
+ // Optionally, handle the error or continue to the next iteration
+ }
}
|
||||||||
| } | ||||||||
| return profiles; | ||||||||
| } | ||||||||
|
|
||||||||
| async function calculateReputationScoreForProfile(profile: any, platform: any): Promise<number> { | ||||||||
| const platformName = platform.name as SupportedNeo4jPlatforms; | ||||||||
| const memberLabel = NEO4J_PLATFORM_INFO[platformName].member; | ||||||||
| const platformId = platform.id; | ||||||||
| const profileId = profile.profile.id; | ||||||||
|
|
||||||||
| const reputationScoreQuery = buildReputationScoreQuery(profileId, platformId, memberLabel); | ||||||||
| const neo4jData = await Neo4j.read(reputationScoreQuery); | ||||||||
|
|
||||||||
| return extractReputationScoreFromNeo4jData(neo4jData); | ||||||||
| } | ||||||||
|
|
||||||||
| function buildReputationScoreQuery(profileId: string, platformId: string, memberLabel: string): string { | ||||||||
| return ` | ||||||||
| MATCH (:${memberLabel} {id: "${profileId}"})-[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 | ||||||||
| `; | ||||||||
| } | ||||||||
|
Comment on lines
+72
to
+82
There was a problem hiding this comment. 🛠️ Refactor suggestion Prevent potential Cypher injection by using parameterized queries The Refactor the code to use parameters: -function buildReputationScoreQuery(profileId: string, platformId: string, memberLabel: string): string {
- return `
- MATCH (:${memberLabel} {id: "${profileId}"})-[r:HAVE_METRICS {platformId: "${platformId}"}]->(a)
+function buildReputationScoreQuery(memberLabel: string): string {
+ return `
+ MATCH (:${memberLabel} {id: $profileId})-[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)
+ 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
`;
}When executing the query, pass parameters separately: async function calculateReputationScoreForProfile(profile: any, platform: any): Promise<number> {
const platformName = platform.name as SupportedNeo4jPlatforms;
const memberLabel = NEO4J_PLATFORM_INFO[platformName].member;
const platformId = platform.id;
const profileId = profile.profile.id;
- const reputationScoreQuery = buildReputationScoreQuery(profileId, platformId, memberLabel);
- const neo4jData = await Neo4j.read(reputationScoreQuery);
+ const reputationScoreQuery = buildReputationScoreQuery(memberLabel);
+ const params = { profileId, platformId };
+ const neo4jData = await Neo4j.run(reputationScoreQuery, params);
return extractReputationScoreFromNeo4jData(neo4jData);
}Ensure your Neo4j driver supports parameterized queries. |
||||||||
|
|
||||||||
| function extractReputationScoreFromNeo4jData(neo4jData: any): number { | ||||||||
| const { records } = neo4jData; | ||||||||
| logger.debug(`Neo4j Records: ${JSON.stringify(records)}`); | ||||||||
|
|
||||||||
| if (records.length === 0) { | ||||||||
| return 0; | ||||||||
| } | ||||||||
|
|
||||||||
| const reputationScoreResponse = records[0]; | ||||||||
| const reputationScore = reputationScoreResponse.get('reputation_score'); | ||||||||
|
|
||||||||
| return reputationScore || 0; | ||||||||
| } | ||||||||
|
|
||||||||
| function throwErrorIfUserHasNoOnChainProfiles(profiles: Array<any>) { | ||||||||
| if (profiles.length === 0) { | ||||||||
| throw new ApiError(400, 'User does not have any on-chain profiles.'); | ||||||||
| } | ||||||||
| } | ||||||||
|
|
||||||||
| function throwErrorIfDynamicNftModuleDoesNotExist(dynamicNftModule: HydratedDocument<IModule> | null) { | ||||||||
| if (!dynamicNftModule) { | ||||||||
| throw new ApiError(400, 'There is no associated dynamic NFT module for the provided token ID.'); | ||||||||
| } | ||||||||
| } | ||||||||
|
|
||||||||
| export default { | ||||||||
| getReputationScore, | ||||||||
| }; | ||||||||
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🛠️ Refactor suggestion
Add input validation and error handling
The simplified implementation could benefit from some essential safeguards:
tokenIdandaddressConsider implementing these improvements:
const getReputationScore = catchAsync(async function (req: Request, res: Response) { const { tokenId, address } = req.params; - logger.debug(tokenId, address); + logger.debug({ tokenId, address }, 'Getting reputation score'); + + if (!tokenId || !address) { + throw new Error('Missing required parameters: tokenId and address'); + } + const reputationScore = await nftService.getReputationScore(tokenId, address); - res.send(reputationScore); + + if (reputationScore === null || reputationScore === undefined) { + throw new Error('Failed to retrieve reputation score'); + } + + logger.debug({ reputationScore }, 'Retrieved reputation score'); + res.json({ success: true, data: reputationScore }); });📝 Committable suggestion