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

feat/gitops/image-registry: Added revamped UI screens for Gitops and image registry #4135

Merged
merged 15 commits into from
Sep 5, 2023

Conversation

Saranya-jena
Copy link
Contributor

@Saranya-jena Saranya-jena commented Aug 17, 2023

Proposed changes

image image

Summarize your changes here to communicate with the maintainers and make sure to put the link of that issue

  • updated the APIs with new schema
  • updated the UI screens

Types of changes

What types of changes does your code introduce to Litmus? Put an x in the boxes that apply

  • New feature (non-breaking change which adds functionality)
  • Bugfix (non-breaking change which fixes an issue)
  • Breaking change (fix or feature that would cause existing functionality to not work as expected)
  • Documentation Update (if none of the other choices applies)

Checklist

Put an x in the boxes that apply. You can also fill these out after creating the PR. If you're unsure about any of them, don't hesitate to ask. We're here to help! This is simply a reminder of what we are going to look for before merging your code.

  • I have read the CONTRIBUTING doc
  • I have signed the commit for DCO to be passed.
  • Lint and unit tests pass locally with my changes
  • I have added tests that prove my fix is effective or that my feature works (if appropriate)
  • I have added necessary documentation (if appropriate)

Dependency

  • Please add the links to the dependent PR need to be merged before this (if any).

Special notes for your reviewer:

Signed-off-by: Saranya-jena <saranya.jena@harness.io>
Signed-off-by: Saranya-jena <saranya.jena@harness.io>
Signed-off-by: Saranya-jena <saranya.jena@harness.io>
Signed-off-by: Saranya-jena <saranya.jena@harness.io>
Signed-off-by: Saranya-jena <saranya.jena@harness.io>
Signed-off-by: Saranya-jena <saranya.jena@harness.io>
Signed-off-by: Saranya-jena <saranya.jena@harness.io>
Signed-off-by: Saranya-jena <saranya.jena@harness.io>
});
const [enableGitopsMutation, { loading: enableGitopsMutationLoading }] = enableGitOps({
onCompleted: () => {
showSuccess('Gitops enabled successfully');
Copy link
Member

Choose a reason for hiding this comment

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

strings


const [addImageRegistryMutation, { loading: addImageRegistryMutationLoading }] = addImageRegistry({
onCompleted: () => {
showSuccess('Image Registry added successfully');
Copy link
Member

Choose a reason for hiding this comment

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

strings

onError: err => showError(err.message)
});
return (
<Loader loading={getImageRegistryLoading}>
Copy link
Member

Choose a reason for hiding this comment

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

we're following practice of controllers calling just views, you can put this loader inside the view.

label: (
<Layout.Vertical style={{ gap: '1rem' }}>
<Text font={{ variation: FontVariation.BODY2_SEMI, weight: 'semi-bold' }} color={Color.BLACK}>
Locally in Litmus
Copy link
Member

Choose a reason for hiding this comment

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

strings, check the entire file

Comment on lines 281 to 290
rightElement={
(
<Container
flex={{ justifyContent: 'center', alignItems: 'center' }}
height="100%"
>
<CopyButton stringToCopy={sshPublicKey} />
</Container>
) as any
}
Copy link
Member

Choose a reason for hiding this comment

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

is this working? since rightElement takes iconName only 😕

Copy link
Contributor Author

Choose a reason for hiding this comment

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

yes it's working :)
image

height="100%"
style={{ overflowY: 'auto' }}
>
{/* <Loader
Copy link
Member

Choose a reason for hiding this comment

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

commented code.

Signed-off-by: Saranya-jena <saranya.jena@harness.io>
Signed-off-by: Saranya-jena <saranya.jena@harness.io>
Signed-off-by: Saranya-jena <saranya.jena@harness.io>

const [updateImageRegistryMutation, { loading: updateImageRegistryMutationLoading }] = updateImageRegistry({
onCompleted: () => {
showSuccess('Image Registry updated successfully');
Copy link
Member

Choose a reason for hiding this comment

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

strings

@@ -57,6 +59,9 @@ export const paths: UseRouteDefinitionsProps = {
toKubernetesChaosInfrastructures: ({ environmentID }) => `/environments/${environmentID}/kubernetes`,
toKubernetesChaosInfrastructureDetails: ({ chaosInfrastructureID, environmentID }) =>
`/environments/${environmentID}/kubernetes/${chaosInfrastructureID}`,
// chaos image registry routes
toImageRegistry: () => `/image-registry/`,
Copy link
Member

Choose a reason for hiding this comment

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

remove trailing /

@@ -14,6 +14,8 @@ aboutChaosCTL: >-
infrastructure:
acceptInvitation: Accept Invitation
accessToAllNamespaces: Access to all namespaces
accessToken: Access Token
accessTokenPlaceholder: Enter your Personal Acess Token
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
accessTokenPlaceholder: Enter your Personal Acess Token
accessTokenPlaceholder: Enter your Personal Access Token

customLabel: Custom
customRepo: Custom Repo
customValueText: All YAML files use the custom values.
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
customValueText: All YAML files use the custom values.
customValueText: All YAML files will use the custom values.

daily: Daily
dailyMessage: Your chaos experiment will run daily at your chosen time.
dateExecuted: Date Executed
dayOfMonthLabel: Day of month
dayOfWeekLabel: Day of week
declineInvitation: Decline Invitation
declineInvitationDescription: Are you sure you want to decline the invitation?
defaultValueOption: Use Default Values
defaultValueText: >-
All YAML files use these default values provided by Litmus. The values cannot
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
All YAML files use these default values provided by Litmus. The values cannot
All YAML files use these default values provided by Litmus. These values cannot

Comment on lines 114 to 125
enableGitops({
variables: {
projectID: projectID,
configurations: {
branch: values.branch ?? '',
repoURL: values.repoURL ?? '',
authType: values.authType,
token: values.token ?? '',
sshPrivateKey: values.sshPrivateKey ?? '',
userName: values.userName ?? '',
password: values.password ?? ''
}
Copy link
Member

Choose a reason for hiding this comment

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

call mutation only when values are present/defined

>
<CopyButton stringToCopy={sshPublicKey} />
</Container>
) as any
Copy link
Member

Choose a reason for hiding this comment

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

why as any?

import Loader from '@components/Loader';
import css from './ImageRegistry.module.scss';

enum ImageRegistryValues {
Copy link
Member

Choose a reason for hiding this comment

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

shift to models

Comment on lines 62 to 64
imageRegistryName: !getImageRegistryData?.imageRegistryInfo.isDefault
? getImageRegistryData?.imageRegistryInfo.imageRegistryName
: '',
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
imageRegistryName: !getImageRegistryData?.imageRegistryInfo.isDefault
? getImageRegistryData?.imageRegistryInfo.imageRegistryName
: '',
imageRegistryName: defaultTo(getImageRegistryData.imageRegistryInfo.isDefault, ''),

Copy link
Member

Choose a reason for hiding this comment

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

replace all ternary with this

Comment on lines 90 to 113
imageRegistryName: values.imageRegistryName
? imageRegValueType === ImageRegistryValues.DEFAULT
? 'docker.io'
: values.imageRegistryName
: '',
imageRepoName: values.imageRegistryRepo
? imageRegValueType === ImageRegistryValues.DEFAULT
? 'litmuschaos'
: values.imageRegistryRepo
: '',
imageRegistryType: values.registryType
? imageRegValueType === ImageRegistryValues.DEFAULT
? ImageRegistryType.PRIVATE
: values.registryType
: ImageRegistryType.PUBLIC,
secretName: values.secretName ? (imageRegValueType === ImageRegistryValues.DEFAULT ? '' : values.secretName) : '',
secretNamespace: values.imageRegistryName
? imageRegValueType === ImageRegistryValues.DEFAULT
? ''
: values.imageRegistryName
: '',
enableRegistry: imageRegValueType === ImageRegistryValues.DEFAULT ? false : true,
isDefault: imageRegValueType === ImageRegistryValues.DEFAULT ? true : false
};
Copy link
Member

Choose a reason for hiding this comment

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

use defaultTo

Signed-off-by: Saranya-jena <saranya.jena@harness.io>
….0.0/image-registry

Signed-off-by: Saranya-jena <saranya.jena@harness.io>
Signed-off-by: Saranya-jena <saranya.jena@harness.io>
Signed-off-by: Saranya-jena <saranya.jena@harness.io>
@Saranya-jena Saranya-jena merged commit 2d18956 into litmuschaos:master Sep 5, 2023
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.

4 participants