[OTE-420]: Deprecate ONBOARD from Indexer #1728
Conversation
WalkthroughRecent updates to the Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant ComplianceController
participant WalletTable
participant ComplianceStatusDB
User ->> ComplianceController: Initiates CONNECT action
ComplianceController ->> WalletTable: Check wallet existence
WalletTable -->> ComplianceController: Responds with wallet info
ComplianceController ->> ComplianceStatusDB: upsertComplianceStatus
ComplianceStatusDB -->> ComplianceController: Compliance status updated
ComplianceController -->> User: Responds with updated status
Poem
Warning Review ran into problemsProblems (1)
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 as PR comments)
Additionally, you can add CodeRabbit Configration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
Outside diff range and nitpick comments (1)
indexer/services/comlink/src/controllers/api/v4/compliance-v2-controller.ts (1)
Line range hint
64-64: Decorators are incorrectly placed here. Decorators should only be used on class declarations, class expressions, and class methods. Please adjust the placement or configuration accordingly.- @Get('/screen/:address') + // Decorator needs to be moved or configured correctly.
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (2)
- indexer/services/comlink/tests/controllers/api/v4/compliance-v2-controller.test.ts (4 hunks)
- indexer/services/comlink/src/controllers/api/v4/compliance-v2-controller.ts (4 hunks)
Additional context used
Biome
indexer/services/comlink/src/controllers/api/v4/compliance-v2-controller.ts
[error] 64-64: Decorators are not valid here. (parse)
Decorators are only valid on class declarations, class expressions, and class methods.
You can enable parameter decorators by setting theunsafeParameterDecoratorsEnabledoption totruein your configuration file.
[error] 78-82: This else clause can be omitted because previous branches break early. (lint/style/noUselessElse)
Unsafe fix: Omit the else clause.
[error] 83-117: This else clause can be omitted because previous branches break early. (lint/style/noUselessElse)
Unsafe fix: Omit the else clause.
[error] 112-116: This else clause can be omitted because previous branches break early. (lint/style/noUselessElse)
Unsafe fix: Omit the else clause.
[error] 383-389: This else clause can be omitted because previous branches break early. (lint/style/noUselessElse)
Unsafe fix: Omit the else clause.
Additional comments not posted (5)
indexer/services/comlink/src/controllers/api/v4/compliance-v2-controller.ts (2)
12-13: The imports forWalletFromDatabaseandWalletTableare correctly added to handle wallet-related operations in compliance checks.
17-17: Import of lodash (_) is added to facilitate object and array manipulations which are common in the compliance logic.
[APROVED]indexer/services/comlink/__tests__/controllers/api/v4/compliance-v2-controller.test.ts (3)
253-253: The test correctly modifies theactiontoComplianceAction.CONNECTas part of the deprecation of theONBOARDaction. This aligns with changes in the main controller.
347-347: The addition ofawait dbHelpers.clearData()is a good practice to ensure a clean state before executing tests, particularly useful in compliance-related tests where state must be controlled.
369-369: This test case effectively checks the behavior when aCONNECTaction is received from a restricted country with no existing compliance status and a wallet. It ensures that the system correctly sets the status toFIRST_STRIKE_CLOSE_ONLY.
| /** | ||
| * If the address doesn't exist in the compliance table: | ||
| * - if the request is from a restricted country: | ||
| * - if the action is CONNECT and no wallet, set the status to BLOCKED | ||
| * - if the action is CONNECT and wallet exists, set the status to FIRST_STRIKE_CLOSE_ONLY | ||
| * - else if the request is from a non-restricted country: | ||
| * - set the status to COMPLIANT | ||
| * | ||
| * if the address is COMPLIANT: | ||
| * - the ONLY action should be CONNECT. VALID_SURVEY/INVALID_SURVEY are no-ops. | ||
| * - if the request is from a restricted country: | ||
| * - set the status to FIRST_STRIKE_CLOSE_ONLY | ||
| * | ||
| * if the address is FIRST_STRIKE_CLOSE_ONLY: | ||
| * - the ONLY actions should be VALID_SURVEY/INVALID_SURVEY/CONNECT. CONNECT | ||
| * are no-ops. | ||
| * - if the action is VALID_SURVEY: | ||
| * - set the status to FIRST_STRIKE | ||
| * - if the action is INVALID_SURVEY: | ||
| * - set the status to CLOSE_ONLY | ||
| * | ||
| * if the address is FIRST_STRIKE: | ||
| * - the ONLY action should be CONNECT. VALID_SURVEY/INVALID_SURVEY are no-ops. | ||
| * - if the request is from a restricted country: | ||
| * - set the status to CLOSE_ONLY | ||
| */ | ||
| async function upsertComplicanceStatus( | ||
| req: express.Request, | ||
| action: ComplianceAction, | ||
| address: string, | ||
| wallet: WalletFromDatabase | undefined, | ||
| complianceStatus: ComplianceStatusFromDatabase[], | ||
| updatedAt: string, | ||
| ): Promise<ComplianceStatusFromDatabase | undefined> { | ||
| if (complianceStatus.length === 0) { | ||
| if (!isRestrictedCountryHeaders(req.headers as CountryHeaders)) { | ||
| return ComplianceStatusTable.upsert({ | ||
| address, | ||
| status: ComplianceStatus.COMPLIANT, | ||
| updatedAt, | ||
| }); | ||
| } | ||
|
|
||
| // If address is restricted and is not onboarded then block | ||
| if (_.isUndefined(wallet)) { | ||
| return ComplianceStatusTable.upsert({ | ||
| address, | ||
| status: ComplianceStatus.BLOCKED, | ||
| reason: getGeoComplianceReason(req.headers as CountryHeaders)!, | ||
| updatedAt, | ||
| }); | ||
| } | ||
|
|
||
| return ComplianceStatusTable.upsert({ | ||
| address, | ||
| status: ComplianceStatus.FIRST_STRIKE_CLOSE_ONLY, | ||
| reason: getGeoComplianceReason(req.headers as CountryHeaders)!, | ||
| updatedAt, | ||
| }); | ||
| } | ||
|
|
||
| if ( | ||
| complianceStatus[0].status === ComplianceStatus.FIRST_STRIKE || | ||
| complianceStatus[0].status === ComplianceStatus.COMPLIANT | ||
| ) { | ||
| if ( | ||
| isRestrictedCountryHeaders(req.headers as CountryHeaders) && | ||
| action === ComplianceAction.CONNECT | ||
| ) { | ||
| return ComplianceStatusTable.update({ | ||
| address, | ||
| status: COMPLIANCE_PROGRESSION[complianceStatus[0].status], | ||
| reason: getGeoComplianceReason(req.headers as CountryHeaders)!, | ||
| updatedAt, | ||
| }); | ||
| } | ||
| } else if (complianceStatus[0].status === ComplianceStatus.FIRST_STRIKE_CLOSE_ONLY) { | ||
| if (action === ComplianceAction.VALID_SURVEY) { | ||
| return ComplianceStatusTable.update({ | ||
| address, | ||
| status: ComplianceStatus.FIRST_STRIKE, | ||
| updatedAt, | ||
| }); | ||
| } else if (action === ComplianceAction.INVALID_SURVEY) { | ||
| return ComplianceStatusTable.update({ | ||
| address, | ||
| status: ComplianceStatus.CLOSE_ONLY, | ||
| updatedAt, | ||
| }); | ||
| } | ||
| } | ||
|
|
||
| return complianceStatus[0]; | ||
| } |
There was a problem hiding this comment.
The function upsertComplianceStatus is well-structured to handle various compliance scenarios based on the country and wallet existence. However, the function name contains a typo. It should be upsertComplianceStatus.
- async function upsertComplicanceStatus(
+ async function upsertComplianceStatus(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.
| /** | |
| * If the address doesn't exist in the compliance table: | |
| * - if the request is from a restricted country: | |
| * - if the action is CONNECT and no wallet, set the status to BLOCKED | |
| * - if the action is CONNECT and wallet exists, set the status to FIRST_STRIKE_CLOSE_ONLY | |
| * - else if the request is from a non-restricted country: | |
| * - set the status to COMPLIANT | |
| * | |
| * if the address is COMPLIANT: | |
| * - the ONLY action should be CONNECT. VALID_SURVEY/INVALID_SURVEY are no-ops. | |
| * - if the request is from a restricted country: | |
| * - set the status to FIRST_STRIKE_CLOSE_ONLY | |
| * | |
| * if the address is FIRST_STRIKE_CLOSE_ONLY: | |
| * - the ONLY actions should be VALID_SURVEY/INVALID_SURVEY/CONNECT. CONNECT | |
| * are no-ops. | |
| * - if the action is VALID_SURVEY: | |
| * - set the status to FIRST_STRIKE | |
| * - if the action is INVALID_SURVEY: | |
| * - set the status to CLOSE_ONLY | |
| * | |
| * if the address is FIRST_STRIKE: | |
| * - the ONLY action should be CONNECT. VALID_SURVEY/INVALID_SURVEY are no-ops. | |
| * - if the request is from a restricted country: | |
| * - set the status to CLOSE_ONLY | |
| */ | |
| async function upsertComplicanceStatus( | |
| req: express.Request, | |
| action: ComplianceAction, | |
| address: string, | |
| wallet: WalletFromDatabase | undefined, | |
| complianceStatus: ComplianceStatusFromDatabase[], | |
| updatedAt: string, | |
| ): Promise<ComplianceStatusFromDatabase | undefined> { | |
| if (complianceStatus.length === 0) { | |
| if (!isRestrictedCountryHeaders(req.headers as CountryHeaders)) { | |
| return ComplianceStatusTable.upsert({ | |
| address, | |
| status: ComplianceStatus.COMPLIANT, | |
| updatedAt, | |
| }); | |
| } | |
| // If address is restricted and is not onboarded then block | |
| if (_.isUndefined(wallet)) { | |
| return ComplianceStatusTable.upsert({ | |
| address, | |
| status: ComplianceStatus.BLOCKED, | |
| reason: getGeoComplianceReason(req.headers as CountryHeaders)!, | |
| updatedAt, | |
| }); | |
| } | |
| return ComplianceStatusTable.upsert({ | |
| address, | |
| status: ComplianceStatus.FIRST_STRIKE_CLOSE_ONLY, | |
| reason: getGeoComplianceReason(req.headers as CountryHeaders)!, | |
| updatedAt, | |
| }); | |
| } | |
| if ( | |
| complianceStatus[0].status === ComplianceStatus.FIRST_STRIKE || | |
| complianceStatus[0].status === ComplianceStatus.COMPLIANT | |
| ) { | |
| if ( | |
| isRestrictedCountryHeaders(req.headers as CountryHeaders) && | |
| action === ComplianceAction.CONNECT | |
| ) { | |
| return ComplianceStatusTable.update({ | |
| address, | |
| status: COMPLIANCE_PROGRESSION[complianceStatus[0].status], | |
| reason: getGeoComplianceReason(req.headers as CountryHeaders)!, | |
| updatedAt, | |
| }); | |
| } | |
| } else if (complianceStatus[0].status === ComplianceStatus.FIRST_STRIKE_CLOSE_ONLY) { | |
| if (action === ComplianceAction.VALID_SURVEY) { | |
| return ComplianceStatusTable.update({ | |
| address, | |
| status: ComplianceStatus.FIRST_STRIKE, | |
| updatedAt, | |
| }); | |
| } else if (action === ComplianceAction.INVALID_SURVEY) { | |
| return ComplianceStatusTable.update({ | |
| address, | |
| status: ComplianceStatus.CLOSE_ONLY, | |
| updatedAt, | |
| }); | |
| } | |
| } | |
| return complianceStatus[0]; | |
| } | |
| async function upsertComplianceStatus( | |
| req: express.Request, | |
| action: ComplianceAction, | |
| address: string, | |
| wallet: WalletFromDatabase | undefined, | |
| complianceStatus: ComplianceStatusFromDatabase[], | |
| updatedAt: string, | |
| ): Promise<ComplianceStatusFromDatabase | undefined> { | |
| if (complianceStatus.length === 0) { | |
| if (!isRestrictedCountryHeaders(req.headers as CountryHeaders)) { | |
| return ComplianceStatusTable.upsert({ | |
| address, | |
| status: ComplianceStatus.COMPLIANT, | |
| updatedAt, | |
| }); | |
| } | |
| // If address is restricted and is not onboarded then block | |
| if (_.isUndefined(wallet)) { | |
| return ComplianceStatusTable.upsert({ | |
| address, | |
| status: ComplianceStatus.BLOCKED, | |
| reason: getGeoComplianceReason(req.headers as CountryHeaders)!, | |
| updatedAt, | |
| }); | |
| } | |
| return ComplianceStatusTable.upsert({ | |
| address, | |
| status: ComplianceStatus.FIRST_STRIKE_CLOSE_ONLY, | |
| reason: getGeoComplianceReason(req.headers as CountryHeaders)!, | |
| updatedAt, | |
| }); | |
| } | |
| if ( | |
| complianceStatus[0].status === ComplianceStatus.FIRST_STRIKE || | |
| complianceStatus[0].status === ComplianceStatus.COMPLIANT | |
| ) { | |
| if ( | |
| isRestrictedCountryHeaders(req.headers as CountryHeaders) && | |
| action === ComplianceAction.CONNECT | |
| ) { | |
| return ComplianceStatusTable.update({ | |
| address, | |
| status: COMPLIANCE_PROGRESSION[complianceStatus[0].status], | |
| reason: getGeoComplianceReason(req.headers as CountryHeaders)!, | |
| updatedAt, | |
| }); | |
| } | |
| } else if (complianceStatus[0].status === ComplianceStatus.FIRST_STRIKE_CLOSE_ONLY) { | |
| if (action === ComplianceAction.VALID_SURVEY) { | |
| return ComplianceStatusTable.update({ | |
| address, | |
| status: ComplianceStatus.FIRST_STRIKE, | |
| updatedAt, | |
| }); | |
| } else if (action === ComplianceAction.INVALID_SURVEY) { | |
| return ComplianceStatusTable.update({ | |
| address, | |
| status: ComplianceStatus.CLOSE_ONLY, | |
| updatedAt, | |
| }); | |
| } | |
| } | |
| return complianceStatus[0]; | |
| } |
Tools
Biome
[error] 383-389: This else clause can be omitted because previous branches break early. (lint/style/noUselessElse)
Unsafe fix: Omit the else clause.
There was a problem hiding this comment.
Actionable comments posted: 3
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (1)
- indexer/services/comlink/src/controllers/api/v4/compliance-v2-controller.ts (4 hunks)
Additional context used
Biome
indexer/services/comlink/src/controllers/api/v4/compliance-v2-controller.ts
[error] 64-64: Decorators are not valid here. (parse)
Decorators are only valid on class declarations, class expressions, and class methods.
You can enable parameter decorators by setting theunsafeParameterDecoratorsEnabledoption totruein your configuration file.
[error] 78-82: This else clause can be omitted because previous branches break early. (lint/style/noUselessElse)
Unsafe fix: Omit the else clause.
[error] 83-117: This else clause can be omitted because previous branches break early. (lint/style/noUselessElse)
Unsafe fix: Omit the else clause.
[error] 112-116: This else clause can be omitted because previous branches break early. (lint/style/noUselessElse)
Unsafe fix: Omit the else clause.
[error] 384-390: This else clause can be omitted because previous branches break early. (lint/style/noUselessElse)
Unsafe fix: Omit the else clause.
Additional comments not posted (2)
indexer/services/comlink/src/controllers/api/v4/compliance-v2-controller.ts (2)
12-13: Imports forWalletFromDatabaseandWalletTableadded successfully.
17-17: Import of lodash using_is correctly added.
| const [ | ||
| complianceStatus, | ||
| wallet, | ||
| ]: [ | ||
| ComplianceStatusFromDatabase[], | ||
| WalletFromDatabase | undefined, | ||
| ] = await Promise.all([ | ||
| ComplianceStatusTable.findAll( | ||
| { address: [address] }, | ||
| [], | ||
| ), | ||
| WalletTable.findById(address), | ||
| ]); | ||
|
|
||
| const updatedAt: string = DateTime.utc().toISO(); | ||
| if (complianceStatus.length === 0) { | ||
| if (isRestrictedCountryHeaders(req.headers as CountryHeaders)) { | ||
| if (action === ComplianceAction.ONBOARD) { | ||
| complianceStatusFromDatabase = await ComplianceStatusTable.upsert({ | ||
| address, | ||
| status: ComplianceStatus.BLOCKED, | ||
| reason: getGeoComplianceReason(req.headers as CountryHeaders)!, | ||
| updatedAt, | ||
| }); | ||
| } else if (action === ComplianceAction.CONNECT) { | ||
| complianceStatusFromDatabase = await ComplianceStatusTable.upsert({ | ||
| address, | ||
| status: ComplianceStatus.FIRST_STRIKE_CLOSE_ONLY, | ||
| reason: getGeoComplianceReason(req.headers as CountryHeaders)!, | ||
| updatedAt, | ||
| }); | ||
| } | ||
| } else { | ||
| complianceStatusFromDatabase = await ComplianceStatusTable.upsert({ | ||
| address, | ||
| status: ComplianceStatus.COMPLIANT, | ||
| updatedAt, | ||
| }); | ||
| } | ||
| } else { | ||
| complianceStatusFromDatabase = complianceStatus[0]; | ||
| if ( | ||
| complianceStatus[0].status === ComplianceStatus.FIRST_STRIKE || | ||
| complianceStatus[0].status === ComplianceStatus.COMPLIANT | ||
| ) { | ||
| if ( | ||
| isRestrictedCountryHeaders(req.headers as CountryHeaders) && | ||
| action === ComplianceAction.CONNECT | ||
| ) { | ||
| complianceStatusFromDatabase = await ComplianceStatusTable.update({ | ||
| address, | ||
| status: COMPLIANCE_PROGRESSION[complianceStatus[0].status], | ||
| reason: getGeoComplianceReason(req.headers as CountryHeaders)!, | ||
| updatedAt, | ||
| }); | ||
| } | ||
| } else if ( | ||
| complianceStatus[0].status === ComplianceStatus.FIRST_STRIKE_CLOSE_ONLY | ||
| ) { | ||
| if (action === ComplianceAction.VALID_SURVEY) { | ||
| complianceStatusFromDatabase = await ComplianceStatusTable.update({ | ||
| address, | ||
| status: ComplianceStatus.FIRST_STRIKE, | ||
| updatedAt, | ||
| }); | ||
| } else if (action === ComplianceAction.INVALID_SURVEY) { | ||
| complianceStatusFromDatabase = await ComplianceStatusTable.update({ | ||
| address, | ||
| status: ComplianceStatus.CLOSE_ONLY, | ||
| updatedAt, | ||
| }); | ||
| } | ||
| } | ||
| } | ||
| const complianceStatusFromDatabase: | ||
| ComplianceStatusFromDatabase | undefined = await upsertComplicanceStatus( | ||
| req, | ||
| action, | ||
| address, | ||
| wallet, | ||
| complianceStatus, | ||
| updatedAt, | ||
| ); | ||
|
|
There was a problem hiding this comment.
There is a typo in the function name upsertComplicanceStatus which should be corrected to upsertComplianceStatus.
- const complianceStatusFromDatabase: ComplianceStatusFromDatabase | undefined = await upsertComplicanceStatus(
+ const complianceStatusFromDatabase: ComplianceStatusFromDatabase | undefined = await upsertComplianceStatus(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.
| const [ | |
| complianceStatus, | |
| wallet, | |
| ]: [ | |
| ComplianceStatusFromDatabase[], | |
| WalletFromDatabase | undefined, | |
| ] = await Promise.all([ | |
| ComplianceStatusTable.findAll( | |
| { address: [address] }, | |
| [], | |
| ), | |
| WalletTable.findById(address), | |
| ]); | |
| const updatedAt: string = DateTime.utc().toISO(); | |
| if (complianceStatus.length === 0) { | |
| if (isRestrictedCountryHeaders(req.headers as CountryHeaders)) { | |
| if (action === ComplianceAction.ONBOARD) { | |
| complianceStatusFromDatabase = await ComplianceStatusTable.upsert({ | |
| address, | |
| status: ComplianceStatus.BLOCKED, | |
| reason: getGeoComplianceReason(req.headers as CountryHeaders)!, | |
| updatedAt, | |
| }); | |
| } else if (action === ComplianceAction.CONNECT) { | |
| complianceStatusFromDatabase = await ComplianceStatusTable.upsert({ | |
| address, | |
| status: ComplianceStatus.FIRST_STRIKE_CLOSE_ONLY, | |
| reason: getGeoComplianceReason(req.headers as CountryHeaders)!, | |
| updatedAt, | |
| }); | |
| } | |
| } else { | |
| complianceStatusFromDatabase = await ComplianceStatusTable.upsert({ | |
| address, | |
| status: ComplianceStatus.COMPLIANT, | |
| updatedAt, | |
| }); | |
| } | |
| } else { | |
| complianceStatusFromDatabase = complianceStatus[0]; | |
| if ( | |
| complianceStatus[0].status === ComplianceStatus.FIRST_STRIKE || | |
| complianceStatus[0].status === ComplianceStatus.COMPLIANT | |
| ) { | |
| if ( | |
| isRestrictedCountryHeaders(req.headers as CountryHeaders) && | |
| action === ComplianceAction.CONNECT | |
| ) { | |
| complianceStatusFromDatabase = await ComplianceStatusTable.update({ | |
| address, | |
| status: COMPLIANCE_PROGRESSION[complianceStatus[0].status], | |
| reason: getGeoComplianceReason(req.headers as CountryHeaders)!, | |
| updatedAt, | |
| }); | |
| } | |
| } else if ( | |
| complianceStatus[0].status === ComplianceStatus.FIRST_STRIKE_CLOSE_ONLY | |
| ) { | |
| if (action === ComplianceAction.VALID_SURVEY) { | |
| complianceStatusFromDatabase = await ComplianceStatusTable.update({ | |
| address, | |
| status: ComplianceStatus.FIRST_STRIKE, | |
| updatedAt, | |
| }); | |
| } else if (action === ComplianceAction.INVALID_SURVEY) { | |
| complianceStatusFromDatabase = await ComplianceStatusTable.update({ | |
| address, | |
| status: ComplianceStatus.CLOSE_ONLY, | |
| updatedAt, | |
| }); | |
| } | |
| } | |
| } | |
| const complianceStatusFromDatabase: | |
| ComplianceStatusFromDatabase | undefined = await upsertComplicanceStatus( | |
| req, | |
| action, | |
| address, | |
| wallet, | |
| complianceStatus, | |
| updatedAt, | |
| ); | |
| const [ | |
| complianceStatus, | |
| wallet, | |
| ]: [ | |
| ComplianceStatusFromDatabase[], | |
| WalletFromDatabase | undefined, | |
| ] = await Promise.all([ | |
| ComplianceStatusTable.findAll( | |
| { address: [address] }, | |
| [], | |
| ), | |
| WalletTable.findById(address), | |
| ]); | |
| const updatedAt: string = DateTime.utc().toISO(); | |
| const complianceStatusFromDatabase: | |
| ComplianceStatusFromDatabase | undefined = await upsertComplianceStatus( | |
| req, | |
| action, | |
| address, | |
| wallet, | |
| complianceStatus, | |
| updatedAt, | |
| ); |
| } else if (action === ComplianceAction.INVALID_SURVEY) { | ||
| return ComplianceStatusTable.update({ | ||
| address, | ||
| status: ComplianceStatus.CLOSE_ONLY, | ||
| updatedAt, | ||
| }); | ||
| } |
There was a problem hiding this comment.
This else clause can be omitted as previous branches break early, simplifying the control flow.
- } else if (action === ComplianceAction.INVALID_SURVEY) {
- return ComplianceStatusTable.update({
- address,
- status: ComplianceStatus.CLOSE_ONLY,
- updatedAt,
- });
- }
+ // Removed unnecessary else clauseCommittable suggestion was skipped due to low confidence.
Tools
Biome
[error] 384-390: This else clause can be omitted because previous branches break early. (lint/style/noUselessElse)
Unsafe fix: Omit the else clause.
| async function upsertComplicanceStatus( | ||
| req: express.Request, | ||
| action: ComplianceAction, | ||
| address: string, | ||
| wallet: WalletFromDatabase | undefined, | ||
| complianceStatus: ComplianceStatusFromDatabase[], | ||
| updatedAt: string, | ||
| ): Promise<ComplianceStatusFromDatabase | undefined> { | ||
| if (complianceStatus.length === 0) { | ||
| if (!isRestrictedCountryHeaders(req.headers as CountryHeaders)) { | ||
| return ComplianceStatusTable.upsert({ | ||
| address, | ||
| status: ComplianceStatus.COMPLIANT, | ||
| updatedAt, | ||
| }); | ||
| } | ||
|
|
||
| // If address is restricted and is not onboarded then block | ||
| if (_.isUndefined(wallet)) { | ||
| return ComplianceStatusTable.upsert({ | ||
| address, | ||
| status: ComplianceStatus.BLOCKED, | ||
| reason: getGeoComplianceReason(req.headers as CountryHeaders)!, | ||
| updatedAt, | ||
| }); | ||
| } | ||
|
|
||
| return ComplianceStatusTable.upsert({ | ||
| address, | ||
| status: ComplianceStatus.FIRST_STRIKE_CLOSE_ONLY, | ||
| reason: getGeoComplianceReason(req.headers as CountryHeaders)!, | ||
| updatedAt, | ||
| }); | ||
| } | ||
|
|
||
| if ( | ||
| complianceStatus[0].status === ComplianceStatus.FIRST_STRIKE || | ||
| complianceStatus[0].status === ComplianceStatus.COMPLIANT | ||
| ) { | ||
| if ( | ||
| isRestrictedCountryHeaders(req.headers as CountryHeaders) && | ||
| action === ComplianceAction.CONNECT | ||
| ) { | ||
| return ComplianceStatusTable.update({ | ||
| address, | ||
| status: COMPLIANCE_PROGRESSION[complianceStatus[0].status], | ||
| reason: getGeoComplianceReason(req.headers as CountryHeaders)!, | ||
| updatedAt, | ||
| }); | ||
| } | ||
| } else if (complianceStatus[0].status === ComplianceStatus.FIRST_STRIKE_CLOSE_ONLY) { | ||
| if (action === ComplianceAction.VALID_SURVEY) { | ||
| return ComplianceStatusTable.update({ | ||
| address, | ||
| status: ComplianceStatus.FIRST_STRIKE, | ||
| updatedAt, | ||
| }); | ||
| } else if (action === ComplianceAction.INVALID_SURVEY) { | ||
| return ComplianceStatusTable.update({ | ||
| address, | ||
| status: ComplianceStatus.CLOSE_ONLY, | ||
| updatedAt, | ||
| }); | ||
| } | ||
| } | ||
|
|
||
| return complianceStatus[0]; | ||
| } |
There was a problem hiding this comment.
The function upsertComplicanceStatus should be renamed to upsertComplianceStatus to correct the typo.
- async function upsertComplicanceStatus(
+ async function upsertComplianceStatus(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.
| async function upsertComplicanceStatus( | |
| req: express.Request, | |
| action: ComplianceAction, | |
| address: string, | |
| wallet: WalletFromDatabase | undefined, | |
| complianceStatus: ComplianceStatusFromDatabase[], | |
| updatedAt: string, | |
| ): Promise<ComplianceStatusFromDatabase | undefined> { | |
| if (complianceStatus.length === 0) { | |
| if (!isRestrictedCountryHeaders(req.headers as CountryHeaders)) { | |
| return ComplianceStatusTable.upsert({ | |
| address, | |
| status: ComplianceStatus.COMPLIANT, | |
| updatedAt, | |
| }); | |
| } | |
| // If address is restricted and is not onboarded then block | |
| if (_.isUndefined(wallet)) { | |
| return ComplianceStatusTable.upsert({ | |
| address, | |
| status: ComplianceStatus.BLOCKED, | |
| reason: getGeoComplianceReason(req.headers as CountryHeaders)!, | |
| updatedAt, | |
| }); | |
| } | |
| return ComplianceStatusTable.upsert({ | |
| address, | |
| status: ComplianceStatus.FIRST_STRIKE_CLOSE_ONLY, | |
| reason: getGeoComplianceReason(req.headers as CountryHeaders)!, | |
| updatedAt, | |
| }); | |
| } | |
| if ( | |
| complianceStatus[0].status === ComplianceStatus.FIRST_STRIKE || | |
| complianceStatus[0].status === ComplianceStatus.COMPLIANT | |
| ) { | |
| if ( | |
| isRestrictedCountryHeaders(req.headers as CountryHeaders) && | |
| action === ComplianceAction.CONNECT | |
| ) { | |
| return ComplianceStatusTable.update({ | |
| address, | |
| status: COMPLIANCE_PROGRESSION[complianceStatus[0].status], | |
| reason: getGeoComplianceReason(req.headers as CountryHeaders)!, | |
| updatedAt, | |
| }); | |
| } | |
| } else if (complianceStatus[0].status === ComplianceStatus.FIRST_STRIKE_CLOSE_ONLY) { | |
| if (action === ComplianceAction.VALID_SURVEY) { | |
| return ComplianceStatusTable.update({ | |
| address, | |
| status: ComplianceStatus.FIRST_STRIKE, | |
| updatedAt, | |
| }); | |
| } else if (action === ComplianceAction.INVALID_SURVEY) { | |
| return ComplianceStatusTable.update({ | |
| address, | |
| status: ComplianceStatus.CLOSE_ONLY, | |
| updatedAt, | |
| }); | |
| } | |
| } | |
| return complianceStatus[0]; | |
| } | |
| async function upsertComplianceStatus( | |
| req: express.Request, | |
| action: ComplianceAction, | |
| address: string, | |
| wallet: WalletFromDatabase | undefined, | |
| complianceStatus: ComplianceStatusFromDatabase[], | |
| updatedAt: string, | |
| ): Promise<ComplianceStatusFromDatabase | undefined> { | |
| if (complianceStatus.length === 0) { | |
| if (!isRestrictedCountryHeaders(req.headers as CountryHeaders)) { | |
| return ComplianceStatusTable.upsert({ | |
| address, | |
| status: ComplianceStatus.COMPLIANT, | |
| updatedAt, | |
| }); | |
| } | |
| // If address is restricted and is not onboarded then block | |
| if (_.isUndefined(wallet)) { | |
| return ComplianceStatusTable.upsert({ | |
| address, | |
| status: ComplianceStatus.BLOCKED, | |
| reason: getGeoComplianceReason(req.headers as CountryHeaders)!, | |
| updatedAt, | |
| }); | |
| } | |
| return ComplianceStatusTable.upsert({ | |
| address, | |
| status: ComplianceStatus.FIRST_STRIKE_CLOSE_ONLY, | |
| reason: getGeoComplianceReason(req.headers as CountryHeaders)!, | |
| updatedAt, | |
| }); | |
| } | |
| if ( | |
| complianceStatus[0].status === ComplianceStatus.FIRST_STRIKE || | |
| complianceStatus[0].status === ComplianceStatus.COMPLIANT | |
| ) { | |
| if ( | |
| isRestrictedCountryHeaders(req.headers as CountryHeaders) && | |
| action === ComplianceAction.CONNECT | |
| ) { | |
| return ComplianceStatusTable.update({ | |
| address, | |
| status: COMPLIANCE_PROGRESSION[complianceStatus[0].status], | |
| reason: getGeoComplianceReason(req.headers as CountryHeaders)!, | |
| updatedAt, | |
| }); | |
| } | |
| } else if (complianceStatus[0].status === ComplianceStatus.FIRST_STRIKE_CLOSE_ONLY) { | |
| if (action === ComplianceAction.VALID_SURVEY) { | |
| return ComplianceStatusTable.update({ | |
| address, | |
| status: ComplianceStatus.FIRST_STRIKE, | |
| updatedAt, | |
| }); | |
| } else if (action === ComplianceAction.INVALID_SURVEY) { | |
| return ComplianceStatusTable.update({ | |
| address, | |
| status: ComplianceStatus.CLOSE_ONLY, | |
| updatedAt, | |
| }); | |
| } | |
| } | |
| return complianceStatus[0]; | |
| } |
Tools
Biome
[error] 384-390: This else clause can be omitted because previous branches break early. (lint/style/noUselessElse)
Unsafe fix: Omit the else clause.
There was a problem hiding this comment.
@vincentwschau, thanks for the thumbs-up! If you need any more help or have further questions, feel free to ask. Happy coding! 🐰
Learnings added
User: vincentwschau
PR: dydxprotocol/v4-chain#1728
File: indexer/services/comlink/src/controllers/api/v4/compliance-v2-controller.ts:327-394
Timestamp: 2024-06-19T15:01:42.992Z
Learning: User vincentwschau acknowledged the correction suggestion about renaming the function `upsertComplicanceStatus` to `upsertComplianceStatus` with a thumbs-up emoji, indicating agreement or approval.
Note: Learnings are effective only in the context of specific code segments. To apply general coding guidelines, please configure review instructions.
There was a problem hiding this comment.
Actionable comments posted: 0
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (1)
- indexer/services/comlink/src/controllers/api/v4/compliance-v2-controller.ts (4 hunks)
Files skipped from review as they are similar to previous changes (1)
- indexer/services/comlink/src/controllers/api/v4/compliance-v2-controller.ts
|
@Mergifyio backport release/indexer/v5.x |
✅ Backports have been created
|
(cherry picked from commit 2039998)
Co-authored-by: Christopher-Li <[email protected]>
Changelist
Deprecate ONBOARD from Indexer
Test Plan
Unit tests
Author/Reviewer Checklist
state-breakinglabel.indexer-postgres-breakinglabel.PrepareProposalorProcessProposal, manually add the labelproposal-breaking.feature:[feature-name].backport/[branch-name].refactor,chore,bug.Summary by CodeRabbit
New Features
Refactor
Tests