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

Saptune details view #1865

Merged
merged 22 commits into from Oct 2, 2023
Merged

Saptune details view #1865

merged 22 commits into from Oct 2, 2023

Conversation

arbulu89
Copy link
Contributor

@arbulu89 arbulu89 commented Sep 25, 2023

Description

Add the saptune details view.

Preview

image

Disclaimer

The pr is already big enough, we decided to move the status icons for Saptune Service Status in a follow up pr.

How was this tested?

Automated Tests.

@arbulu89 arbulu89 added the enhancement New feature or request label Sep 25, 2023
@arbulu89 arbulu89 force-pushed the saptune-details-view branch 2 times, most recently from 0f5b7bc to 4493b6f Compare September 25, 2023 13:37
@EMaksy EMaksy force-pushed the saptune-details-view branch 7 times, most recently from 0af552c to fa43bac Compare September 27, 2023 07:58
Copy link
Contributor Author

@arbulu89 arbulu89 left a comment

Choose a reason for hiding this comment

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

Some preliminary comments.
We need to work a bit more on those factories i think

assets/js/components/HostDetails/HostDetailsPage.jsx Outdated Show resolved Hide resolved
assets/js/components/SaptuneDetails/SaptuneDetails.jsx Outdated Show resolved Hide resolved
assets/js/lib/test-utils/factories/hosts.js Outdated Show resolved Hide resolved
assets/js/lib/test-utils/factories/hosts.js Outdated Show resolved Hide resolved
@EMaksy EMaksy force-pushed the saptune-details-view branch 5 times, most recently from 0585125 to 1f3cdd0 Compare September 27, 2023 16:31
Copy link
Contributor Author

@arbulu89 arbulu89 left a comment

Choose a reason for hiding this comment

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

Okey!
Once the tests are updated we can merge

assets/js/lib/test-utils/factories/hosts.js Outdated Show resolved Hide resolved
assets/js/lib/test-utils/factories/hosts.js Outdated Show resolved Hide resolved
assets/js/lib/test-utils/factories/hosts.js Outdated Show resolved Hide resolved
@EMaksy
Copy link
Member

EMaksy commented Sep 28, 2023

Also one minor thing i will update the existing tests

@EMaksy EMaksy marked this pull request as ready for review September 28, 2023 06:56
@EMaksy EMaksy force-pushed the saptune-details-view branch 3 times, most recently from 5bd0f0b to 2511519 Compare September 28, 2023 17:58
@EMaksy EMaksy self-assigned this Sep 28, 2023
@EMaksy EMaksy force-pushed the saptune-details-view branch 2 times, most recently from 6aef97f to 548172a Compare September 29, 2023 07:27
@EMaksy EMaksy added the javascript Pull requests that update Javascript code label Sep 29, 2023
@EMaksy EMaksy force-pushed the saptune-details-view branch 2 times, most recently from 4f66516 to 4375dfa Compare September 29, 2023 11:46
Copy link
Contributor Author

@arbulu89 arbulu89 left a comment

Choose a reason for hiding this comment

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

Looks good!
Just left a mini comment.
I cannot approve as contributor of the PR

describe('SaptuneDetailsPage', () => {
it('should render not found when the host is missing', () => {
const hosts = hostFactory.buildList(2);
const hostID = 'NonExistingUUID';
Copy link
Contributor Author

Choose a reason for hiding this comment

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

We should use faker.string.uuid(). The uuid will be unique

@arbulu89 arbulu89 merged commit 802e336 into main Oct 2, 2023
26 checks passed
@arbulu89 arbulu89 deleted the saptune-details-view branch October 2, 2023 12:19
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request javascript Pull requests that update Javascript code
Development

Successfully merging this pull request may close these issues.

None yet

3 participants