Implement request blocking for specific account ID; add tests for blo… #483
There are no files selected for viewing
This file contains hidden or 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 hidden or 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 hidden or 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 hidden or 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 |
|---|---|---|
| @@ -0,0 +1,84 @@ | ||
| import RequestManager from '../../../src/modules/request' | ||
| import Account from '../../../src/modules/account' | ||
|
|
||
| describe('RequestManager - Blocked Account ID', () => { | ||
| let requestManager | ||
| let mockLogger | ||
| let mockDevice | ||
| let mockSession | ||
| let mockIsPersonalisationActive | ||
|
|
||
| beforeEach(() => { | ||
| mockLogger = { | ||
| debug: jest.fn(), | ||
| error: jest.fn(), | ||
| wzrkError: {} | ||
| } | ||
| mockDevice = { | ||
| gcookie: 'test-gcookie-123' | ||
| } | ||
| mockSession = { | ||
| getSessionCookieObject: jest.fn(() => ({ s: 'session-123', p: 1 })) | ||
| } | ||
| mockIsPersonalisationActive = jest.fn(() => false) | ||
| }) | ||
|
|
||
| test('should drop post requests for blocked account ID 8R5-4K5-466Z', () => { | ||
| const blockedAccount = new Account({ id: '8R5-4K5-466Z' }) | ||
| requestManager = new RequestManager({ | ||
| logger: mockLogger, | ||
| account: blockedAccount, | ||
| device: mockDevice, | ||
| session: mockSession, | ||
| isPersonalisationActive: mockIsPersonalisationActive | ||
| }) | ||
|
|
||
| const testUrl = 'https://example.com/api/sync' | ||
| const testBody = JSON.stringify({ data: 'test' }) | ||
|
|
||
| return requestManager.post(testUrl, testBody) | ||
| .then(() => { | ||
| // If we reach here, the test should fail | ||
| expect(true).toBe(false) | ||
| }) | ||
| .catch((error) => { | ||
| // Verify that the request was blocked | ||
| expect(error.message).toBe('Account blocked') | ||
| expect(mockLogger.debug).toHaveBeenCalledWith('post req dropped for blocked account: 8R5-4K5-466Z') | ||
| }) | ||
| }) | ||
|
|
||
| test('should allow post requests for non-blocked account IDs', () => { | ||
| const normalAccount = new Account({ id: 'TEST-ACCOUNT-123' }) | ||
| requestManager = new RequestManager({ | ||
| logger: mockLogger, | ||
| account: normalAccount, | ||
| device: mockDevice, | ||
| session: mockSession, | ||
| isPersonalisationActive: mockIsPersonalisationActive | ||
| }) | ||
|
|
||
| const testUrl = 'https://example.com/api/sync' | ||
| const testBody = JSON.stringify({ data: 'test' }) | ||
|
|
||
| // Mock fetch | ||
| global.fetch = jest.fn(() => | ||
| Promise.resolve({ | ||
| ok: true, | ||
| json: () => Promise.resolve({ success: true }) | ||
| }) | ||
| ) | ||
|
|
||
| return requestManager.post(testUrl, testBody) | ||
| .then((result) => { | ||
| // Verify that the request was not blocked | ||
| expect(mockLogger.debug).not.toHaveBeenCalledWith(expect.stringContaining('post req dropped for blocked account')) | ||
| expect(result).toEqual({ success: true }) | ||
| expect(global.fetch).toHaveBeenCalledWith(testUrl, { | ||
| method: 'post', | ||
| headers: { 'Content-Type': 'application/json' }, | ||
| body: testBody | ||
| }) | ||
| }) | ||
| }) | ||
| }) |
Oops, something went wrong.
Oops, something went wrong.
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 | 🟠 Major
🧩 Analysis chain
Extract hardcoded account ID to a constant and centralize blocking logic.
The account ID
'8R5-4K5-466Z'is hardcoded as a magic string and duplicated across multiple locations (also appears in thepostmethod below and in other files per the AI summary). This violates the DRY principle and makes maintenance difficult.Additionally, logging specific account IDs may raise privacy or compliance concerns, depending on your data governance policies.
Consider the following refactor:
Run the following script to verify all occurrences of the hardcoded account ID across the codebase:
🏁 Script executed:
Length of output: 1202
Length of output: 386
🏁 Script executed:
Length of output: 380
🏁 Script executed:
Length of output: 1360
🏁 Script executed:
Length of output: 68
Length of output: 1035
🏁 Script executed:
Length of output: 3286
Extract hardcoded account ID to a constant in src/util/constants.js and centralize blocking logic across src/util/requestDispatcher.js and src/modules/request.js.
The account ID
'8R5-4K5-466Z'is duplicated across two source files with identical checks:src/util/requestDispatcher.js:23(fireRequest method)src/modules/request.js:280(post method)This violates the DRY principle and makes maintenance difficult. Additionally, logging specific account IDs may raise privacy or compliance concerns.
Recommended refactor:
src/util/requestDispatcher.js:23– Replace the hardcoded check with the helper functionsrc/modules/request.js:280– Replace the hardcoded check with the helper function'req dropped for blocked account')This approach scales for blocking multiple accounts and centralizes the logic for future maintenance.