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

Add ensa version sap system details #1559

Merged
merged 3 commits into from
Jun 21, 2023
Merged

Conversation

arbulu89
Copy link
Contributor

@arbulu89 arbulu89 commented Jun 21, 2023

Description

Add ensa version value to SAP system details view. By now, using main branch the default "-" will be shown, as the backend part is deregistration branch. I'm sending the PR to main to avoid yet much more conflicts

image

Storybook stories added for SAP system and Database details.

How was this tested?

Tested and stories added

@arbulu89 arbulu89 added the enhancement New feature or request label Jun 21, 2023
@@ -12,6 +12,9 @@ import {
systemInstancesTableConfiguration,
} from './tableConfigs';

export const renderEnsaVersion = (ensaVersion) =>
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I'm not sure if the place for this function is the best one.
Suggestions?

Copy link
Contributor

Choose a reason for hiding this comment

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

You can both leave it here or have a dedicated file, if we maybe get to have more and more render functions we can have something like renderers.js or something like that. For now I think we can leave it

Copy link
Contributor

Choose a reason for hiding this comment

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

(if you happen to find another file name that isn't utils feels free)

@arbulu89 arbulu89 requested review from nelsonkopliku and EMaksy June 21, 2023 11:30
@arbulu89 arbulu89 marked this pull request as ready for review June 21, 2023 11:37
Copy link
Contributor

@jagabomb jagabomb left a comment

Choose a reason for hiding this comment

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

LGTM

Copy link
Contributor

@dottorblaster dottorblaster left a comment

Choose a reason for hiding this comment

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

LGTM

Copy link
Member

@nelsonkopliku nelsonkopliku left a comment

Choose a reason for hiding this comment

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

Great job!

Well, I feel guilty about the many keysToCamel you were forced to add 🙈

The initial intent about it was enabling the possibility to transform snake_case api responses/requests to camelCase objects in the code and vice-versa, but it never happened 😅

I guess we could just get rid of it, we use snake_case mixed with camelCase anyway.
Feel also free to track debt.

@arbulu89
Copy link
Contributor Author

arbulu89 commented Jun 21, 2023

Feel also free to track debt.

Done! Ticket to track this added to our refinement backlog

@arbulu89 arbulu89 merged commit bc95a61 into main Jun 21, 2023
@arbulu89 arbulu89 deleted the add-ensa-version-sap-system-details branch June 21, 2023 13:22
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Development

Successfully merging this pull request may close these issues.

4 participants