Skip to content
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

APIM 7550 fix get archived applications info #10072

Open
wants to merge 2 commits into
base: 4.2.x
Choose a base branch
from

Conversation

mukul-tyagi08
Copy link
Contributor

Issue

https://gravitee.atlassian.net/browse/APIM-7550

Description

  1. Refactored findByIds method as it was taking status into consideration.
  2. Created new findByIds method without any status. This method is then used to get archived applications info for subscriptions.

Additional context

@mukul-tyagi08 mukul-tyagi08 requested a review from a team as a code owner December 6, 2024 08:34
@mukul-tyagi08 mukul-tyagi08 force-pushed the APIM-7550-fix-get-archived-applications-info branch 3 times, most recently from 33f066a to dc780a4 Compare December 6, 2024 11:00
@mukul-tyagi08 mukul-tyagi08 force-pushed the APIM-7550-fix-get-archived-applications-info branch from dc780a4 to 950debb Compare December 6, 2024 11:10
@@ -233,17 +233,28 @@ public ApplicationEntity findById(final ExecutionContext executionContext, Strin
}

@Override
public Set<ApplicationListItem> findByIds(final ExecutionContext executionContext, Collection<String> applicationIds) {
public Set<ApplicationListItem> findByIds(ExecutionContext executionContext, Collection<String> applicationIds) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Previously, the findByIds method implicitly searched applications by id and ApplicationStatus.ACTIVE by default. By replacing it with your implementation, you've altered the behaviour for all existing resources or services that currently rely on this method.

Could you please verify whether this method is still being called elsewhere? If so, ensure that the default behaviour hasn't been unintentionally modified. If the change impacts existing functionality, consider using your findByIdsAndStatus method explicitly to preserve the intended behaviour.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

  1. I replaced/renamed findByIds to findByIdsAndStatus and updated all the usages/calls by adding additional parameter: ApplicationStatus.Active
  2. Then I added a new method findByIds

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants