-
Notifications
You must be signed in to change notification settings - Fork 4
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
404 add reputation score api endpoint #405
Changes from 2 commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change | ||||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
@@ -0,0 +1,109 @@ | ||||||||||||||
import { Request, Response } from 'express'; | ||||||||||||||
import { catchAsync } from '../utils'; | ||||||||||||||
import parentLogger from '../config/logger'; | ||||||||||||||
import { moduleService, platformService, ociService } from '../services'; | ||||||||||||||
import { ApiError } from '../utils'; | ||||||||||||||
import { IModule, IPlatform } from '@togethercrew.dev/db'; | ||||||||||||||
import { HydratedDocument } from 'mongoose'; | ||||||||||||||
import * as Neo4j from '../neo4j'; | ||||||||||||||
import { NEO4J_PLATFORM_INFO } from '../constants/neo4j.constant'; | ||||||||||||||
import { SupportedNeo4jPlatforms } from '../types/neo4j.type'; | ||||||||||||||
|
||||||||||||||
const logger = parentLogger.child({ module: 'NftController' }); | ||||||||||||||
|
||||||||||||||
const getReputationScore = catchAsync(async function (req: Request, res: Response) { | ||||||||||||||
const { tokenId, address } = req.params; | ||||||||||||||
const supportedPlatforms = ['discord', 'discourse']; | ||||||||||||||
|
||||||||||||||
let repuationScore; | ||||||||||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Handle potential undefined 'repuationScore' before returning If none of the profiles match the supported platforms, Apply the following diff to initialize - let repuationScore;
+ let repuationScore = 0;
...
- return repuationScore;
+ res.json({ reputationScore: repuationScore }); Also applies to: 64-64 |
||||||||||||||
logger.debug(tokenId, address); | ||||||||||||||
const profiles: Array<any> = await getProfilesOnAllSupportedChains(address); | ||||||||||||||
logger.debug(profiles); | ||||||||||||||
Comment on lines
+20
to
+21
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Validate profiles after retrieval After retrieving profiles, you should validate that profiles exist before proceeding to prevent errors when profiles are empty. Apply the following diff: const profiles: Array<any> = await getProfilesOnAllSupportedChains(address);
+ shouldProfilesExist(profiles);
logger.debug(profiles); Ensure that the
|
||||||||||||||
const dynamicNftModule = await moduleService.getModuleByFilter({ 'options.platforms.0.metadata.tokenId': tokenId }); | ||||||||||||||
logger.debug(dynamicNftModule); | ||||||||||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Validate the existence of the dynamic NFT module After retrieving Apply the following diff: const dynamicNftModule = await moduleService.getModuleByFilter({ 'options.platforms.0.metadata.tokenId': tokenId });
+ shouldDynamicNftModuleExist(dynamicNftModule);
logger.debug(dynamicNftModule); Ensure that the
|
||||||||||||||
|
||||||||||||||
for (let i = 0; i < supportedPlatforms.length; i++) { | ||||||||||||||
const platform = await platformService.getPlatformByFilter({ | ||||||||||||||
name: supportedPlatforms[i], | ||||||||||||||
community: dynamicNftModule?.community, | ||||||||||||||
}); | ||||||||||||||
logger.debug({ i, platform, supportedPlatforms: supportedPlatforms[i] }); | ||||||||||||||
// shouldPlatformExist(platform); | ||||||||||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Uncomment the platform existence validation The call to Apply the following diff: - // shouldPlatformExist(platform);
+ shouldPlatformExist(platform); Ensure that
|
||||||||||||||
for (let j = 0; j < profiles.length; j++) { | ||||||||||||||
const profile = profiles[j]; | ||||||||||||||
logger.debug({ i, j, profile, supportedPlatforms: supportedPlatforms[i] }); | ||||||||||||||
if (profile.profile.provider === supportedPlatforms[i]) { | ||||||||||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Validate the existence of the user profile After retrieving Apply the following diff: const profile = profiles[j];
+ shouldProfileExist(profile);
logger.debug({ i, j, profile, supportedPlatforms: supportedPlatforms[i] });
if (profile.profile.provider === supportedPlatforms[i]) { Ensure that the
|
||||||||||||||
const reputationScoreQuery = ` | ||||||||||||||
MATCH (:${NEO4J_PLATFORM_INFO[platform?.name as SupportedNeo4jPlatforms].member} {id: "${profile.profile.id}"})-[r:HAVE_METRICS {platformId: "${platform?.id}"}]->(a) | ||||||||||||||
WITH r.date as metrics_date, r.closenessCentrality as memberScore | ||||||||||||||
ORDER BY metrics_date DESC | ||||||||||||||
LIMIT 1 | ||||||||||||||
MATCH (user:${NEO4J_PLATFORM_INFO[platform?.name as SupportedNeo4jPlatforms].member})-[user_r:HAVE_METRICS {platformId: "${platform?.id}", date: metrics_date}]->(user) | ||||||||||||||
WITH memberScore, MAX(user_r.closenessCentrality) as maxScore | ||||||||||||||
RETURN memberScore / maxScore AS reputation_score | ||||||||||||||
`; | ||||||||||||||
|
||||||||||||||
const neo4jData = await Neo4j.read(reputationScoreQuery); | ||||||||||||||
const { records } = neo4jData; | ||||||||||||||
logger.debug(records); | ||||||||||||||
|
||||||||||||||
const reputationScoreResponse = records[0]; | ||||||||||||||
|
||||||||||||||
logger.debug(reputationScoreResponse); | ||||||||||||||
|
||||||||||||||
const { _fieldLookup, _fields } = reputationScoreResponse as unknown as { | ||||||||||||||
_fieldLookup: Record<string, number>; | ||||||||||||||
_fields: number[]; | ||||||||||||||
}; | ||||||||||||||
Comment on lines
+54
to
+57
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🛠️ Refactor suggestion Avoid using internal properties of Neo4j response objects Accessing internal properties like Refactor the code to use public methods: - const { _fieldLookup, _fields } = reputationScoreResponse as unknown as {
- _fieldLookup: Record<string, number>;
- _fields: number[];
- };
- repuationScore = _fields[_fieldLookup['reputation_score']];
+ const reputationScoreRecord = reputationScoreResponse.get('reputation_score');
+ repuationScore = reputationScoreRecord; This approach uses the 📝 Committable suggestion
Suggested change
|
||||||||||||||
|
||||||||||||||
repuationScore = _fields[_fieldLookup['reputation_score']]; | ||||||||||||||
logger.debug(repuationScore); | ||||||||||||||
} | ||||||||||||||
} | ||||||||||||||
} | ||||||||||||||
return repuationScore; | ||||||||||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Send the reputation score in the HTTP response The controller function currently returns the reputation score but does not send it in the HTTP response. In an Express controller, you should use Apply the following diff to send the reputation score in the response: - return reputationScore;
+ res.json({ reputationScore });
|
||||||||||||||
}); | ||||||||||||||
|
||||||||||||||
async function getProfilesOnAllSupportedChains(address: string) { | ||||||||||||||
let profiles: Array<any> = []; | ||||||||||||||
const supportedChainIds = [11155111]; | ||||||||||||||
for (let i = 0; i < supportedChainIds.length; i++) { | ||||||||||||||
const chainProfiles = await ociService.getProfiles(address, supportedChainIds[i]); | ||||||||||||||
profiles = profiles.concat(chainProfiles); | ||||||||||||||
Comment on lines
+71
to
+72
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Handle potential errors from OCI service calls The call to Add error handling for the OCI service call: for (let i = 0; i < supportedChainIds.length; i++) {
- const chainProfiles = await ociService.getProfiles(address, supportedChainIds[i]);
+ let chainProfiles;
+ try {
+ chainProfiles = await ociService.getProfiles(address, supportedChainIds[i]);
+ } catch (error) {
+ logger.error(`Failed to get profiles for chain ID ${supportedChainIds[i]}:`, error);
+ continue;
+ }
profiles = profiles.concat(chainProfiles);
} This ensures that a failure in one chain does not halt the entire process.
|
||||||||||||||
} | ||||||||||||||
return profiles; | ||||||||||||||
} | ||||||||||||||
|
||||||||||||||
function shouldProfilesExist(profiles: Array<any>) { | ||||||||||||||
if (profiles.length < 0) { | ||||||||||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Fix incorrect condition in profiles existence check The condition Apply the following diff: function shouldProfilesExist(profiles: Array<any>) {
- if (profiles.length < 0) {
+ if (profiles.length === 0) {
throw new ApiError(400, 'User has no on-chain profiles');
}
} |
||||||||||||||
throw new ApiError(400, 'User has no any onchain profiles'); | ||||||||||||||
} | ||||||||||||||
} | ||||||||||||||
|
||||||||||||||
function shouldDynamicNftModuleExist(dynamicNftModule: HydratedDocument<IModule> | null) { | ||||||||||||||
if (!dynamicNftModule) { | ||||||||||||||
throw new ApiError(400, "There's not any assoicated dynamic nft module to the token Id"); | ||||||||||||||
} | ||||||||||||||
} | ||||||||||||||
|
||||||||||||||
function shouldPlatformExist(platform: HydratedDocument<IPlatform> | null) { | ||||||||||||||
if (!platform) { | ||||||||||||||
throw new ApiError(400, "There's not any platform connected for requested platform"); | ||||||||||||||
} | ||||||||||||||
} | ||||||||||||||
|
||||||||||||||
function shouldProfileExist(profile: any) { | ||||||||||||||
if (!profile) { | ||||||||||||||
throw new ApiError(400, "There's not any user oncahin profile for requested platform"); | ||||||||||||||
} | ||||||||||||||
} | ||||||||||||||
export default { | ||||||||||||||
getReputationScore, | ||||||||||||||
}; | ||||||||||||||
|
||||||||||||||
// const platform = await platformService.getPlatformByFilter({ | ||||||||||||||
// name: platforms[i], | ||||||||||||||
// community: dynamicNftModule?.community, | ||||||||||||||
// }); | ||||||||||||||
// const profile = profiles.find((p: any) => p.profile.provider === platform); | ||||||||||||||
// // Do the Cypher || N/A ?? || drop-down for platforms |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,14 @@ | ||
import express from 'express'; | ||
import { nftController } from '../../controllers'; | ||
import { nftValidation } from '../../validations'; | ||
import { validate } from '../../middlewares'; | ||
const router = express.Router(); | ||
|
||
// Routes | ||
router.post( | ||
'/:tokenId/:address/reputation-score', | ||
validate(nftValidation.getReputationScore), | ||
nftController.getReputationScore, | ||
); | ||
Comment on lines
+8
to
+12
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 💡 Codebase verification Consider adding authentication middleware to protect NFT reputation endpoint The codebase has authentication middleware (
router.post(
'/:tokenId/:address/reputation-score',
auth('admin', 'view'), // Add this line
validate(nftValidation.getReputationScore),
nftController.getReputationScore,
); The validation schema correctly validates:
🔗 Analysis chainConsider adding authentication and rate limiting middleware This endpoint processes NFT reputation scores based on user input. Consider implementing:
Let's verify the validation schema and any existing auth middleware: 🏁 Scripts executedThe following scripts were executed for the analysis: Script: #!/bin/bash
# Check for validation schema implementation
echo "Checking validation schema..."
rg -A 10 "getReputationScore" --glob "src/validations/**/*.ts"
# Check for existing auth middleware in the project
echo "Checking auth middleware..."
rg -l "auth.*middleware" --glob "src/middlewares/**/*.ts"
Length of output: 894 Script: #!/bin/bash
# Check for any auth or rate limiting middleware in the route file
echo "Checking route middleware..."
rg -B 5 -A 5 "router\.(get|post|put|delete)" src/routes/v1/nft.route.ts
# Check for any auth middleware implementations
echo "Checking auth implementations..."
fd -e ts -e js --exec rg -l "auth" {} \; -X rg -A 5 "(middleware|auth)" {}
# Check for rate limiting implementations
echo "Checking rate limiting..."
fd -e ts -e js --exec rg -l "limit" {} \; -X rg -A 5 "(rateLimit|rateLimiter)"
Length of output: 1083 Script: #!/bin/bash
# Check for auth implementations with corrected syntax
echo "Checking auth implementations..."
rg -l "auth" --type ts --type js | xargs rg -A 5 "(middleware|auth)"
# Check for rate limiting with corrected syntax
echo "Checking rate limiting..."
rg -l "limit" --type ts --type js | xargs rg -A 5 "(rateLimit|rateLimiter)"
# Check middleware imports in the main app setup
echo "Checking app middleware setup..."
rg -A 10 "express\(\)" --type ts --type js
Length of output: 51320 |
||
|
||
export default router; |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,35 @@ | ||
import { HydratedDocument, Types } from 'mongoose'; | ||
import httpStatus from 'http-status'; | ||
import { Platform, IPlatform } from '@togethercrew.dev/db'; | ||
import ApiError from '../utils/ApiError'; | ||
import sagaService from './saga.service'; | ||
import discourseService from './discourse'; | ||
import { Snowflake } from 'discord.js'; | ||
import { analyzerAction, analyzerWindow } from '../config/analyzer.statics'; | ||
import { PlatformNames } from '@togethercrew.dev/db'; | ||
|
||
/** | ||
* get reputation score | ||
* @param {IPlatform} PlatformBody | ||
* @returns {Promise<HydratedDocument<IPlatform>>} | ||
*/ | ||
const getReputationScore = async (PlatformBody: IPlatform): Promise<HydratedDocument<IPlatform>> => { | ||
if (PlatformBody.name === PlatformNames.Discord || PlatformBody.name === PlatformNames.Discourse) { | ||
if (PlatformBody.metadata) { | ||
PlatformBody.metadata = { | ||
action: analyzerAction, | ||
window: analyzerWindow, | ||
...PlatformBody.metadata, | ||
}; | ||
} | ||
} | ||
const platform = await Platform.create(PlatformBody); | ||
Comment on lines
+16
to
+26
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Add error handling and input validation. The function lacks proper error handling and input validation:
const getReputationScore = async (PlatformBody: IPlatform): Promise<HydratedDocument<IPlatform>> => {
+ if (!PlatformBody || !PlatformBody.name) {
+ throw new ApiError(httpStatus.BAD_REQUEST, 'Invalid platform data');
+ }
+
if (PlatformBody.name === PlatformNames.Discord || PlatformBody.name === PlatformNames.Discourse) {
if (PlatformBody.metadata) {
- PlatformBody.metadata = {
+ const enrichedMetadata = {
action: analyzerAction,
window: analyzerWindow,
...PlatformBody.metadata,
};
+ PlatformBody = { ...PlatformBody, metadata: enrichedMetadata };
}
}
- const platform = await Platform.create(PlatformBody);
+ try {
+ const platform = await Platform.create(PlatformBody);
+ } catch (error) {
+ throw new ApiError(httpStatus.INTERNAL_SERVER_ERROR, 'Failed to create platform');
+ }
|
||
if (PlatformBody.name === PlatformNames.Discord) { | ||
await sagaService.createAndStartFetchMemberSaga(platform._id); | ||
} | ||
return platform; | ||
}; | ||
|
||
export default { | ||
getReputationScore, | ||
}; | ||
Comment on lines
+1
to
+35
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 💡 Codebase verification Integration verification reveals mismatched implementation The service implementation doesn't align with its usage in the controller:
Locations needing attention:
🔗 Analysis chainVerify integration with related components. Let's verify the integration with the controller, routes, and validation components mentioned in the summary. Let me gather more information about the integration by checking the actual implementation in the route and controller files. Let me check the contents of the controller and validation files to complete the verification. 🏁 Scripts executedThe following scripts were executed for the analysis: Script: #!/bin/bash
# Description: Verify integration with related components
# Check for controller implementation
echo "Checking NFT controller implementation..."
rg -l "getReputationScore.*nft\.service" src/controllers/
# Check for route definition
echo "Checking route definition..."
rg -l "/:tokenId/:address/reputation-score" src/routes/
# Check for validation schema
echo "Checking validation schema..."
rg -l "reputation.*score.*validation" src/validations/
# Verify OCI_BACKEND_URL usage
echo "Checking OCI_BACKEND_URL usage..."
rg "OCI_BACKEND_URL" src/config/
Length of output: 697 Script: #!/bin/bash
# Check the route implementation
echo "Route implementation:"
cat src/routes/v1/nft.route.ts
# Check for controller file
echo -e "\nSearching for NFT controller:"
fd "nft.controller" src/
# Check validation schema files
echo -e "\nLooking for validation files:"
fd "nft.*validation" src/
Length of output: 742 Script: #!/bin/bash
# Check the controller implementation
echo "Controller implementation:"
cat src/controllers/nft.controller.ts
# Check the validation schema
echo -e "\nValidation schema:"
cat src/validations/nft.validation.ts
Length of output: 4749 |
Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
@@ -0,0 +1,29 @@ | ||||||||||||||||||||||||||
import fetch from 'node-fetch'; | ||||||||||||||||||||||||||
import config from '../config'; | ||||||||||||||||||||||||||
import { ApiError } from '../utils'; | ||||||||||||||||||||||||||
import parentLogger from '../config/logger'; | ||||||||||||||||||||||||||
|
||||||||||||||||||||||||||
const logger = parentLogger.child({ module: 'OciService' }); | ||||||||||||||||||||||||||
|
||||||||||||||||||||||||||
async function getProfiles(address: string, chainId: number) { | ||||||||||||||||||||||||||
try { | ||||||||||||||||||||||||||
logger.debug(`${config.ociBackendURL}/oci/profiles/${chainId}/${address}`); | ||||||||||||||||||||||||||
const response = await fetch(`${config.ociBackendURL}/api/v1/oci/profiles/${chainId}/${address}`, { | ||||||||||||||||||||||||||
method: 'GET', | ||||||||||||||||||||||||||
headers: { 'Content-Type': 'application/json' }, | ||||||||||||||||||||||||||
}); | ||||||||||||||||||||||||||
Comment on lines
+10
to
+14
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🛠️ Refactor suggestion Enhance URL construction and request configuration Several improvements can be made to the request setup:
- logger.debug(`${config.ociBackendURL}/oci/profiles/${chainId}/${address}`);
- const response = await fetch(`${config.ociBackendURL}/api/v1/oci/profiles/${chainId}/${address}`, {
+ const url = new URL(`${config.ociBackendURL}/api/v1/oci/profiles/${chainId}/${encodeURIComponent(address)}`);
+ logger.debug(`Fetching profiles from: ${url.toString()}`);
+ const response = await fetch(url, {
method: 'GET',
headers: { 'Content-Type': 'application/json' },
+ timeout: 5000, // Add reasonable timeout
}); 📝 Committable suggestion
Suggested change
|
||||||||||||||||||||||||||
if (response.ok) { | ||||||||||||||||||||||||||
return await response.json(); | ||||||||||||||||||||||||||
} else { | ||||||||||||||||||||||||||
const errorResponse = await response.text(); | ||||||||||||||||||||||||||
throw new Error(errorResponse); | ||||||||||||||||||||||||||
} | ||||||||||||||||||||||||||
} catch (error: any) { | ||||||||||||||||||||||||||
logger.error(error, 'Failed to get profiles from oci backend'); | ||||||||||||||||||||||||||
throw new ApiError(590, 'Failed to get profiles from oci backend '); | ||||||||||||||||||||||||||
} | ||||||||||||||||||||||||||
Comment on lines
+21
to
+24
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🛠️ Refactor suggestion Improve error handling specificity The error handling could be more specific and informative:
- } catch (error: any) {
- logger.error(error, 'Failed to get profiles from oci backend');
- throw new ApiError(590, 'Failed to get profiles from oci backend ');
+ } catch (error) {
+ const errorMessage = `Failed to get profiles for address ${address} on chain ${chainId}`;
+ logger.error({ error, address, chainId }, errorMessage);
+ if (error instanceof Error) {
+ throw new ApiError(
+ OCI_ERROR_CODES.PROFILE_FETCH_FAILED,
+ `${errorMessage}: ${error.message}`
+ );
+ }
+ throw new ApiError(OCI_ERROR_CODES.PROFILE_FETCH_FAILED, errorMessage); Consider adding an error codes enum: enum OCI_ERROR_CODES {
PROFILE_FETCH_FAILED = 590,
// Add other OCI-related error codes here
} |
||||||||||||||||||||||||||
} | ||||||||||||||||||||||||||
|
||||||||||||||||||||||||||
export default { | ||||||||||||||||||||||||||
getProfiles, | ||||||||||||||||||||||||||
}; |
Original file line number | Diff line number | Diff line change | ||||||
---|---|---|---|---|---|---|---|---|
@@ -0,0 +1,15 @@ | ||||||||
import Joi from 'joi'; | ||||||||
import { PlatformNames } from '@togethercrew.dev/db'; | ||||||||
|
||||||||
Comment on lines
+1
to
+3
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🛠️ Refactor suggestion Remove unused import The import Joi from 'joi';
-import { PlatformNames } from '@togethercrew.dev/db'; 📝 Committable suggestion
Suggested change
|
||||||||
const getReputationScore = { | ||||||||
params: Joi.object().keys({ | ||||||||
tokenId: Joi.string().required(), | ||||||||
address: Joi.string() | ||||||||
.regex(/^0x[a-fA-F0-9]{40}$/) | ||||||||
.required(), | ||||||||
}), | ||||||||
}; | ||||||||
|
||||||||
export default { | ||||||||
getReputationScore, | ||||||||
}; |
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
Correct the misspelled variable name "repuationScore"
The variable
repuationScore
is misspelled in multiple places. It should bereputationScore
for clarity and consistency.Apply the following diff to fix the variable name:
Also applies to: 59-59, 60-60, 64-64