Fix unnecessary API calls when project already associated(AST-137848)#1473
Open
cx-sumit-morchhale wants to merge 3 commits intomainfrom
Open
Fix unnecessary API calls when project already associated(AST-137848)#1473cx-sumit-morchhale wants to merge 3 commits intomainfrom
cx-sumit-morchhale wants to merge 3 commits intomainfrom
Conversation
Consolidated duplicate project association checks at the findApplicationAndUpdate entry point to prevent unnecessary API calls for both flag enabled and disabled states. Changes: - Added top-level check in findApplicationAndUpdate to skip API calls when projectID already in ProjectIds - Removed redundant internal check from associateProjectToApplication function - Rewrote tests to verify behavior at entry point for both flag states The fix ensures: ✅ No unnecessary CreateProjectAssociation API calls when flag is ENABLED ✅ No unnecessary UpdateApplication API calls when flag is DISABLED ✅ Single responsibility principle - check happens at entry point only ✅ All tests pass with no regressions Co-Authored-By: Claude Haiku 4.5 <noreply@anthropic.com>
cx-sumit-morchhale
requested review from
cx-anjali-deore,
cx-anurag-dalke and
cx-umesh-waghode
as code owners
|
Great job! No new security vulnerabilities introduced in this pull request |
Extract repeated string literals into constants to satisfy goconst linter: - mockApplicationName = "MOCK" (used in 2 tests) - testProjectName = "test-project" (used in 2 tests) This follows Go best practices for test constants and resolves linting warnings. Co-Authored-By: Claude Haiku 4.5 <noreply@anthropic.com>
- Remove "MOCK" from default ProjectIds to fix integration tests that expect new project associations - Keep "ID-newProject" in ProjectIds only for ExistingApplication to preserve polling test functionality - Maintain ProjectID1, ProjectID2, test_project, ID-new-project-name in default list for unit tests - Fixes issue where MOCK project was incorrectly marked as already associated in 3 integration tests - Ensures top-level duplicate check in findApplicationAndUpdate works correctly for both flag states Fixes: AST-137848 Co-Authored-By: Claude Haiku 4.5 <noreply@anthropic.com>
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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
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.
Consolidated duplicate project association checks at the findApplicationAndUpdate entry point to prevent unnecessary API calls.
Changes:
The fix ensures:
✅ No unnecessary CreateProjectAssociation API calls when flag is ENABLED ✅ No unnecessary UpdateApplication API calls when flag is DISABLED ✅ Single responsibility principle - check happens at entry point only ✅ All tests pass with no regressions
By submitting this pull request, you agree to the terms within the Checkmarx Code of Conduct. Please review the contributing guidelines for guidance on creating high-quality pull requests.
Description
Please provide a summary of the changes and the related issue. Include relevant motivation and context.
Type of Change
Related Issues
Link any related issues or tickets.
Checklist
Screenshots (if applicable)
Add screenshots to help explain your changes.
Additional Notes
Add any other relevant information.