Skip to content

[ui-Quick Start] Convert the Quick Start page within administrator server in ReactJS #4025

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.

Sign up for GitHub

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

Jump to bottom
Open
wants to merge 17 commits into
base: master
Choose a base branch
from
Open

[ui-Quick Start] Convert the Quick Start page within administrator server in ReactJS #4025

wants to merge 17 commits into from

Conversation

ananya-agarwal
Copy link
Collaborator

@ananya-agarwal ananya-agarwal commented Mar 4, 2025 •
edited
Loading

What changes were proposed in this pull request?

Update the Quick Start page to use React and align it with the Cloudera Design Language. Quick Start has been renamed as Overview at some places.

How was this patch tested?

Unit tests, manual testing

Updated pictures of the Quick Start page:

Screenshot 2025-04-01 at 4 06 05 PM Screenshot 2025-03-24 at 1 40 20 PM Screenshot 2025-03-24 at 1 42 23 PM Screenshot 2025-03-24 at 1 43 50 PM

Please review Hue Contributing Guide before opening a pull request.

Harshg999
Harshg999 reviewed Mar 4, 2025
View reviewed changes
Copy link
Collaborator

@Harshg999 Harshg999 left a comment

Choose a reason for hiding this comment

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

Nice change! I've added some quick review comments.

bjornalm
bjornalm requested changes Mar 6, 2025
View reviewed changes
@github-actions GitHub Actions
Copy link

⚠️ No unit test files modified. Please ensure that changes are properly tested. ⚠️

1 similar comment
@github-actions GitHub Actions
Copy link

⚠️ No unit test files modified. Please ensure that changes are properly tested. ⚠️

@github-actions GitHub Actions
Copy link

⚠️ No unit test files modified. Please ensure that changes are properly tested. ⚠️

1 similar comment
@github-actions GitHub Actions
Copy link

⚠️ No unit test files modified. Please ensure that changes are properly tested. ⚠️

@github-actions GitHub Actions
Copy link

github-actions bot commented Mar 16, 2025 •
edited
Loading

Python Code Coverage

Python Coverage Report •
FileStmtsMissCoverMissing
desktop/core/src/desktop
   api_public.py38717255%47–48, 53–54, 65–66, 71–72, 77–78, 83–84, 89–90, 95–96, 101–102, 110–111, 116–117, 122–123, 150–152, 154, 164, 166, 168, 173, 175, 177, 182–183, 188–189, 194–195, 200–201, 206, 208, 210, 239–240, 245, 247–248, 250, 258–259, 264–265, 270–271, 276–277, 282–283, 288–289, 294–295, 300–301, 306–307, 312–313, 318–319, 324–325, 330–331, 336–337, 342–343, 348–349, 354–355, 360–361, 366–367, 372–373, 378–379, 384–385, 390–391, 396–397, 402–403, 408–409, 414–415, 420–421, 426–427, 432–433, 441–442, 450–451, 456–457, 462–463, 471–472, 477–478, 483–484, 489–490, 495–496, 501–502, 507–508, 513–514, 522–523, 528–529, 534–535, 540–541, 546–547, 552–553, 558–559, 564–565, 570–571, 576–577, 582–583, 591–592, 597–598, 603–604, 611–614, 619, 621, 641, 643–644
TOTAL536412667350% 

Pytest Report

Tests Skipped Failures Errors Time
1090 106 💤 0 ❌ 0 🔥 6m 1s ⏱️

@github-actions GitHub Actions
Copy link

⚠️ No unit test files modified. Please ensure that changes are properly tested. ⚠️

1 similar comment
@github-actions GitHub Actions
Copy link

⚠️ No unit test files modified. Please ensure that changes are properly tested. ⚠️

tabraiz12
tabraiz12 requested changes Mar 19, 2025
View reviewed changes
@github-actions GitHub Actions
Copy link

⚠️ No unit test files modified. Please ensure that changes are properly tested. ⚠️

@github-actions GitHub Actions
Copy link

⚠️ No unit test files modified. Please ensure that changes are properly tested. ⚠️

1 similar comment
@github-actions GitHub Actions
Copy link

⚠️ No unit test files modified. Please ensure that changes are properly tested. ⚠️

@github-actions GitHub Actions
Copy link

⚠️ No unit test files modified. Please ensure that changes are properly tested. ⚠️

1 similar comment
@github-actions GitHub Actions
Copy link

⚠️ No unit test files modified. Please ensure that changes are properly tested. ⚠️

@ananya-agarwal ananya-agarwal force-pushed the Overview branch 3 times, most recently from 9a6bc6e to ce5b692 Compare April 15, 2025 15:45
bjornalm
bjornalm requested changes Apr 15, 2025
View reviewed changes
Copy link
Collaborator

@bjornalm bjornalm left a comment

Choose a reason for hiding this comment

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

Nice work, we are almost there :-)

tabraiz12
tabraiz12 approved these changes Apr 17, 2025
View reviewed changes
Copy link
Collaborator

@tabraiz12 tabraiz12 left a comment

Choose a reason for hiding this comment

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

LGTM

bjornalm
bjornalm requested changes Apr 17, 2025
View reviewed changes
desktop/core/src/desktop/js/apps/admin/Overview/Examples.tsx
try {
const appWithExtraData = exampleAppsWithData.find(app => app.id === exampleApp.id);
if (appWithExtraData && appWithExtraData.data) {
for (const eachData of appWithExtraData.data) {
Copy link
Collaborator

Choose a reason for hiding this comment

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

Currently you're looping through exampleAppsWithData.data and calling installExamplesCall without await. These are fire-and-forget calls, and any errors inside them won’t be caught. This could lead to unexpected behavior or race conditions.

desktop/core/src/desktop/js/apps/admin/Overview/Examples.tsx
: t('An error occurred while installing examples.');
huePubSub.publish('hue.global.error', { message: errorMessage });
} finally {
setInstallingAppId('');
Copy link
Collaborator

Choose a reason for hiding this comment

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

since installExamplesCall is async this will be executed before they finish...

desktop/core/src/desktop/js/apps/admin/Overview/Examples.tsx
const message = t('Examples refreshed');
huePubSub.publish('hue.global.info', { message });
} catch (error) {
const errorMessage = error.message
Copy link
Collaborator

Choose a reason for hiding this comment

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

Don't we want the error to say which example installation that failed?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@bjornalm bjornalm bjornalm requested changes

@JohanAhlen JohanAhlen Awaiting requested review from JohanAhlen

@ramprasadagarwal ramprasadagarwal Awaiting requested review from ramprasadagarwal

@Harshg999 Harshg999 Awaiting requested review from Harshg999

@tabraiz12 tabraiz12 Awaiting requested review from tabraiz12

Requested changes must be addressed to merge this pull request.

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

Successfully merging this pull request may close these issues.
5 participants
@ananya-agarwal @tabraiz12 @bjornalm @ramprasadagarwal @Harshg999