Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

#

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Jump to bottom

HIPP-1777: Stop creating hidden production credentials #220

Merged
merged 2 commits into from
Dec 12, 2024
Merged

HIPP-1777: Stop creating hidden production credentials #220

merged 2 commits into from
Dec 12, 2024

Conversation

victorarbuesmallada
Copy link
Contributor

No description provided.

@victorarbuesmallada victorarbuesmallada force-pushed the feature/HIPP-1777-stop-creating-hidden-creds-prod branch from b4c98dd to ad3d11e Compare December 11, 2024 16:41
PaulCDurham
PaulCDurham reviewed Dec 12, 2024
View reviewed changes
Copy link
Contributor

@PaulCDurham PaulCDurham left a comment

Choose a reason for hiding this comment

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

Could you add a test verifying that register application does not create a credential in the production environment?

test/uk/gov/hmrc/apihubapplications/services/helpers/ApplicationEnricherSpec.scala
@@ -355,7 +355,6 @@ class ApplicationEnricherSpec extends AsyncFreeSpec
"credentialCreatingApplicationEnricher" - {
"must create a credential in the hip environments and enrich the application with it" in {
val expected = Seq(
testApplication.setCredentials(FakeHipEnvironments.primaryEnvironment, Seq(testClientResponse1.asNewHiddenCredential(clock))),
Copy link
Contributor

Choose a reason for hiding this comment

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

We still need to test creating credentials in the primary environment - why remove this?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

the reason was because I was filtering the credentials creation for production/primary environments on credentialCreatingApplicationEnricher, that's now done on the ApplicationsLifeCycleService.
Although as of now, credentialCreatingApplicationEnricher will never create primary environments credentials.

test/uk/gov/hmrc/apihubapplications/services/helpers/ApplicationEnricherSpec.scala Outdated
@@ -366,7 +365,7 @@ class ApplicationEnricherSpec extends AsyncFreeSpec
.thenReturn(Future.successful(Right(testClientResponse2)))

val results = Future.sequence(
FakeHipEnvironments.environments.map(
Seq(FakeHipEnvironments.secondaryEnvironment).map(
Copy link
Contributor

Choose a reason for hiding this comment

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

We still need to test creating credentials in the primary environment - why remove this?

@victorarbuesmallada
Copy link
Contributor Author

Could you add a test verifying that register application does not create a credential in the production environment?

I thought these lines would cover that?
https://github.com/hmrc/api-hub-applications/pull/220/files#diff-48cef6fe75b0af0cfc75387391f0bcf06484e6d6d78542e4be394283bb1eaf0dR76-R96

@victorarbuesmallada victorarbuesmallada force-pushed the feature/HIPP-1777-stop-creating-hidden-creds-prod branch from ad3d11e to 5a05fb1 Compare December 12, 2024 10:32
@victorarbuesmallada victorarbuesmallada merged commit 8721916 into main Dec 12, 2024
1 check passed
@victorarbuesmallada victorarbuesmallada deleted the feature/HIPP-1777-stop-creating-hidden-creds-prod branch December 12, 2024 12:28
# for free to join this conversation on GitHub. Already have an account? # to comment
Reviewers

@PaulCDurham PaulCDurham PaulCDurham approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@victorarbuesmallada @PaulCDurham