Skip to content

#413066 validation and ui for the full config (the parts that should …#54

Merged
rsteppac merged 7 commits intodevelopfrom
feature/413066-common-service-configuration-validation
Jul 4, 2025
Merged

#413066 validation and ui for the full config (the parts that should …#54
rsteppac merged 7 commits intodevelopfrom
feature/413066-common-service-configuration-validation

Conversation

@rsteppac
Copy link
Copy Markdown
Collaborator

@rsteppac rsteppac commented Jul 2, 2025

…be editable by the customer), but currently limited to a single connector. ConfigurationWriterTest.kt still needs to have the mocks adjusted and currently has most of its tests disabled.

…be editable by the customer), but currently limited to a single connector. ConfigurationWriterTest.kt still needs to have the mocks adjusted and currently has most of its tests disabled.
@rsteppac rsteppac requested a review from Lesrac July 2, 2025 07:55
Ralf Steppacher added 4 commits July 2, 2025 16:08
…be editable by the customer), but currently limited to a single connector. ConfigurationWriterTest.kt still needs to have the mocks adjusted and currently has most of its tests disabled.
Lesrac
Lesrac previously approved these changes Jul 3, 2025
logger.debug { "set '${changedConfigItem.propertyPath}' to '${newValue}' as type '${newValue::class}'" }
}

is Float -> {
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

do we have any type defined as float? Or is that so that it would be covered? (if so, then what about Long?)

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

Yes, it is there "just in case". And yes, I forgot to add Long. Added now.

<include resource="org/springframework/boot/logging/logback/console-appender.xml" />

<logger name="com.swisscom.health.des.cdr.client" level="DEBUG" />
<!-- <logger name="org" level="INFO" />-->
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

should be removed

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

Done.

name = DomainObjects.ConfigurationItem.CDR_API_HOST,
modifier = modifier.padding(8.dp).fillMaxWidth(),
initiallyExpanded = false,
options = { listOf("cdr.health.swisscom.ch", "stg.cdr.health.swisscom.ch") },
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I'm not sure that we want to have this in place, as we risk that someone selects the wrong one accidentally and then our support will have issues finding out why they get an error regarding "wrong credentials"

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

Will replace it with a regular text field and use the prod URL as the hint text.


Divider(modifier = modifier)

// TODO: replace with a dropdown to select the environment and set CDR host and tenant ID accordingly
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

The tenant-id is probably not something that we want to put in a public repository

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

Will replace it with a regular text field and squash the history of the branch.

@rsteppac
Copy link
Copy Markdown
Collaborator Author

rsteppac commented Jul 4, 2025

Bypassed review requirement as Daniel is on vacation...

@rsteppac rsteppac merged commit 9f28262 into develop Jul 4, 2025
4 checks passed
@rsteppac rsteppac deleted the feature/413066-common-service-configuration-validation branch July 4, 2025 12:48
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants