Added Default User Login in Connect Flow And Environment based Cache Suffix

[Raj-Shah1]

Summary by CodeRabbit

• New Feature: Added support for differentiating between test and production users in Salesforce authentication flow.
• Refactor: Updated cache constants and keys to ensure consistency across the application. Cache keys now include a suffix based on the environment.
• Test: Modified test cases to accommodate changes in method signatures and behavior.
• Chore: Introduced new methods to retrieve default test user credentials and determine the current environment.

[coderabbitai]

Walkthrough

The changes primarily focus on enhancing the application's environment detection, cache management, and test user handling. The code now differentiates between staging and production environments, modifies cache keys based on the environment, and introduces a new authentication flow for test users.

Changes

File(s)	Summary
.../api/config/CoreConstants.java .../globalConstants/SecretConstants.java	Introduced methods to detect environment and retrieve default test user credentials.
.../api/config/MemcachedConfig.java .../globalConstants/CacheConstants.java .../repositories/SalesforceOauthTokenRepository.java .../repositories/SalesforceUserRepository.java	Renamed cache constants and added a cache suffix based on the environment to ensure consistent cache keys.
.../lib/globalConstants/SalesforceConstants.java	Added a function to return the password grant type string.
.../lib/salesforce/wrappers/SalesforceTokens.java .../services/salesforce/AuthService.java	Modified the getTokens method to handle different authentication flows for test users and regular users.
.../functional/controllers/authController/PostDisconnectTest.java .../controllers/authController/PostSalesforceConnectTest.java .../unit/lib/salesforce/wrappers/SalesforceTokensTest.java	Updated test methods to accommodate the changes in the getTokens method.

"In the land of code, where logic is king, 🐇👑

Changes were made, oh what joy they bring! 🎉

Environments detected, caches refined, 🌍💾

Test users handled, with ease of mind. 🧪👥

Celebrate, dear rabbit, for the code is bright, 🎊💡

Dancing in the glow of the monitor's light." 💃🖥️

---

Tips

Chat with CodeRabbit Bot (@coderabbitai)

• Mention @coderabbitai in any review comment for bot assistance.
• Note: Review comments are made on code diffs or files, not on the PR overview.

Pause Incremental Reviews

• Insert @coderabbitai: ignore in the PR description to halt the bot's ongoing reviews. Remove the line to resume.

[coderabbitai]

Review Status

Actionable comments generated: 8

Commits

Files that changed from the base of the PR and between 39190a6 and 7dbee79 commits.

Files ignored due to filter (1)

• sample.secrets.json

Files selected for processing (12)

• src/main/java/com/salessparrow/api/config/CoreConstants.java (2 hunks)
• src/main/java/com/salessparrow/api/config/MemcachedConfig.java (1 hunks)
• src/main/java/com/salessparrow/api/lib/globalConstants/CacheConstants.java (1 hunks)
• src/main/java/com/salessparrow/api/lib/globalConstants/SalesforceConstants.java (1 hunks)
• src/main/java/com/salessparrow/api/lib/globalConstants/SecretConstants.java (1 hunks)
• src/main/java/com/salessparrow/api/lib/salesforce/wrappers/SalesforceTokens.java (1 hunks)
• src/main/java/com/salessparrow/api/repositories/SalesforceOauthTokenRepository.java (4 hunks)
• src/main/java/com/salessparrow/api/repositories/SalesforceUserRepository.java (4 hunks)
• src/main/java/com/salessparrow/api/services/salesforce/AuthService.java (3 hunks)
• src/test/java/com/salessparrow/api/functional/controllers/authController/PostDisconnectTest.java (3 hunks)
• src/test/java/com/salessparrow/api/functional/controllers/authController/PostSalesforceConnectTest.java (5 hunks)
• src/test/java/com/salessparrow/api/unit/lib/salesforce/wrappers/SalesforceTokensTest.java (2 hunks)

Files skipped from review due to trivial changes (5)

• src/main/java/com/salessparrow/api/config/CoreConstants.java
• src/main/java/com/salessparrow/api/config/MemcachedConfig.java
• src/main/java/com/salessparrow/api/lib/globalConstants/SalesforceConstants.java
• src/main/java/com/salessparrow/api/lib/globalConstants/SecretConstants.java
• src/main/java/com/salessparrow/api/repositories/SalesforceOauthTokenRepository.java

Additional comments (Suppressed): 14

src/main/java/com/salessparrow/api/repositories/SalesforceUserRepository.java (4)

• 33-34: The cache eviction key has been updated to include a suffix based on the environment. Ensure that this change is reflected in all places where the cache is accessed.

• 53-54: The cache eviction key has been updated to include a suffix based on the environment. Ensure that this change is reflected in all places where the cache is accessed.

• 71-72: The cacheable key has been updated to include a suffix based on the environment. Ensure that this change is reflected in all places where the cache is accessed.

• 82-83: The cache eviction key has been updated to include a suffix based on the environment. Ensure that this change is reflected in all places where the cache is accessed.

src/main/java/com/salessparrow/api/lib/globalConstants/CacheConstants.java (1)

• 29-37: Ensure that all references to these constants throughout the codebase have been updated to match the new names.

src/main/java/com/salessparrow/api/services/salesforce/AuthService.java (1)

• 130-131: The TODO comment indicates that there might be some unfinished work related to calling the getTokens method with different parameters. Ensure that this has been addressed before merging.

src/main/java/com/salessparrow/api/lib/salesforce/wrappers/SalesforceTokens.java (1)

• 34-53: Ensure that all calls to getTokens method throughout the codebase have been updated to match the new signature with the additional isTestUser parameter.

src/test/java/com/salessparrow/api/functional/controllers/authController/PostDisconnectTest.java (3)

• 4-6: The new import import static org.mockito.ArgumentMatchers.anyBoolean; has been added to accommodate the changes in the method signature of getTokens. Ensure that all calls to this function throughout the codebase have been updated to match the new signature.

• 96-96: The getTokens method now includes an additional boolean parameter. This change is consistent with the PR summary and seems to be correctly implemented here.

- when(mockGetTokens.getTokens(anyString(), anyString())).thenReturn(getTokensMockRes);
+ when(mockGetTokens.getTokens(anyString(), anyString(), anyBoolean())).thenReturn(getTokensMockRes);

• 133-133: The getTokens method now includes an additional boolean parameter. This change is consistent with the PR summary and seems to be correctly implemented here.

- when(mockGetTokens.getTokens(anyString(), anyString())).thenReturn(getTokensMockRes);
+ when(mockGetTokens.getTokens(anyString(), anyString(), anyBoolean())).thenReturn(getTokensMockRes);

src/test/java/com/salessparrow/api/functional/controllers/authController/PostSalesforceConnectTest.java (2)

• 11-14: The import import static org.mockito.ArgumentMatchers.anyBoolean; has been added. Ensure that the new test cases cover scenarios where this argument matcher is used.

• 169-170: The verification of the getTokens method now includes a check for the additional boolean parameter. This is a good practice as it ensures that the method is being called with the correct number of arguments.

src/test/java/com/salessparrow/api/unit/lib/salesforce/wrappers/SalesforceTokensTest.java (2)

• 88-94: Ensure that all calls to getTokens method throughout the codebase have been updated to match the new signature.

• 118-120: Ensure that all calls to getTokens method throughout the codebase have been updated to match the new signature.

[coderabbitai]

The method getTokens now accepts an additional boolean parameter. Make sure all calls to this method in your tests have been updated accordingly.

- when(mockGetTokens.getTokens(anyString(), anyString())).thenReturn(getTokensMockRes);
+ when(mockGetTokens.getTokens(anyString(), anyString(), anyBoolean())).thenReturn(getTokensMockRes);

[coderabbitai]

Review Status

Actionable comments generated: 2

Commits

Files that changed from the base of the PR and between 7dbee79 and fb5d026 commits.

Files selected for processing (5)

• src/main/java/com/salessparrow/api/config/CoreConstants.java (2 hunks)
• src/main/java/com/salessparrow/api/lib/globalConstants/CacheConstants.java (1 hunks)
• src/main/java/com/salessparrow/api/lib/salesforce/helper/SalesforceRequest.java (1 hunks)
• src/main/java/com/salessparrow/api/lib/salesforce/wrappers/SalesforceTokens.java (1 hunks)
• src/main/java/com/salessparrow/api/services/salesforce/AuthService.java (2 hunks)

Files skipped from review due to trivial changes (2)

• src/main/java/com/salessparrow/api/config/CoreConstants.java
• src/main/java/com/salessparrow/api/lib/salesforce/helper/SalesforceRequest.java

Additional comments (Suppressed): 5

src/main/java/com/salessparrow/api/lib/globalConstants/CacheConstants.java (2)

• 9-10: The CACHE_SUFFIX is declared as a constant but its value is determined at runtime. This could lead to unexpected behavior if the environment changes after the class has been loaded. Consider making it a regular static variable or ensure that the environment does not change during the application's lifetime.

• 16-17: The cache keys no longer have the SalesSparrow prefix ("ss_"). If this was intentional and all uses of these keys in the codebase have been updated accordingly, then this is fine. Otherwise, this might cause issues with key mismatches.

src/main/java/com/salessparrow/api/services/salesforce/AuthService.java (2)

• 93-109: The new code introduces a mechanism to handle test users. However, it's important to ensure that the defaultTestUserCode() method in CoreConstants is returning a unique and constant value for each environment. If this value changes or overlaps with actual user codes, it could lead to unexpected behavior.

String testUserCode = CoreConstants.defaultTestUserCode();
Boolean isTestUser = false;

code = params.getCode();
if (code.equals(testUserCode)) {
    isTestUser = true;
}

• 126-133: The fetchOauthTokensFromSalesforce method now accepts a boolean parameter isTestUser. Ensure that all calls to this method throughout the codebase have been updated to match the new signature. Also, verify that the getTokens method in salesforceTokens correctly handles the isTestUser parameter.

private void fetchOauthTokensFromSalesforce(Boolean isTestUser) {
    logger.info("Fetching OAuth Tokens from Salesforce");
    HttpResponse response = salesforceTokens.getTokens(this.code, this.redirectUri, isTestUser);
    JsonNode jsonNode = util.getJsonNode(response.getResponseBody());
}

src/main/java/com/salessparrow/api/lib/salesforce/wrappers/SalesforceTokens.java (1)

• 34-34: Ensure that all calls to getTokens method throughout the codebase have been updated to match the new signature.

[mohitcharkha]

LGTM