presto-ui forwardfit #24167

sh-shamsan · 2024-11-28T08:00:56Z

Description

This PR brings commits that upgrades and fixes side effects from an internal IBM presto repo, it mainly upgrades bootstrap version to 5, and jQuery to 3.7.1 along with other changes.

These commits were done in the older folder structure before creating a presto-ui module. So this PR resolves conflicts and also applies the upgrades needed to all the new components in prestodb/presto that were created that weren't in the IBM repo. Like QueryResults, SQLInput, SessionProps, QueryStageView, StaticQueryHeader, QueryPlanView, QueryOverView and the single page application

Motivation and Context

It brings the updates to help keep up the presto console up to date and reduce security vulnerabilities

Impact

The upgrades impact the UI colors and design a bit, nothing major. Bugs were addressed in this PR.

Test Plan

No flow errors
Builds successfully
Verified the presto UI in browser, verified too with @yhwang (thanks!)

Contributor checklist

Please make sure your submission complies with our contributing guide, in particular code style and commit standards.
PR description addresses the issue accurately and concisely. If the change is non-trivial, a GitHub Issue is referenced.
Documented new properties (with its default value), SQL syntax, functions, or other functionality.
If release notes are required, they follow the release notes guidelines.
Adequate tests were added if applicable.
CI passed.

Release Notes

Please follow release notes guidelines and fill in the release notes below.

== RELEASE NOTES ==

General Changes
* Upgrade bootstrap to version 5.
* Upgrade jQuery to version 3.7.1.
* Replace depreciated ``dagre-d3`` with ``dagre-d3-es`` in response to a high severity vulnerability [WS-2022-0322](https://github.com/opensearch-project/OpenSearch-Dashboards/issues/2482).

linux-foundation-easycla · 2024-11-28T08:01:01Z

The committers listed above are authorized under a signed CLA.

✅ login: sh-shamsan / name: Shahad Shamsan (bfec191, 36ff293, 8923c39, dd19f54, d42c85c, ec7bf90, 7926add, a281a52)
✅ login: yhwang / name: Yihong Wang (e3f5668)

steveburnett

LGTM! (docs)

Pull branch, local doc build, doc looks good. Thanks!

ethanyzhang · 2025-01-30T17:04:44Z

@unidevel Could you help review this?

yhwang

Appreciate the efforts for the upgrade and some cleanups

2 minor comments only.

presto-ui/src/components/PageTitle.jsx

presto-ui/src/components/QueryHeader.jsx

steveburnett · 2025-02-04T18:54:04Z

New release note guidelines. Please remove the manual PR link in the following format from the release note entries for this PR.


:pr:`12345`

I have updated the Release Notes Guidelines to remove the examples of manually adding the PR link.

Also, suggest rephrase the third release note entry from

Switch from dagre-d3 to dagre-d3-es cuz it's deprecated and in response to a high severity WS-2022-0322

to

Replace depreciated ``dagre-d3`` with ``dagre-d3-es`` in response to a high severity vulnerability [WS-2022-0322](https://github.com/opensearch-project/OpenSearch-Dashboards/issues/2482).

Note: I found the link searching for WS-2022-0322. If the link is not the correct link, please correct it.

presto-ui/src/components/LivePlan.jsx

unidevel · 2025-02-04T21:32:17Z

Fixed

~~The status check buttons not aligned vertically in the middle:~~

Current:

unidevel · 2025-02-04T21:42:01Z

Fixed

~~The query items is not well aligned and spaced:~~

Current:

Issues (From top to bottom)
~~1. The height of black background is smaller than the left one. Previously they are same height.~~
~~2. There is margin on the right of the time in previous version.~~
~~3. The height of gray background for SQL area is smaller than the left area. Previously they are same height.~~

Previous(0.290):

unidevel · 2025-02-04T21:55:03Z

fixed

In the Query details, at bottom of the page, there is the Show: All dropdown, seems the dropdown style is lost.

Current:

steveburnett

LGTM! (docs)

Pull branch, local doc build, doc looks good. Thanks!

yingsu00

@sh-shamsan The EasyCLA is failing. Can you please sign it?
Also, there are some items I didn't see you marked fixed. Can you please confirm everything is fixed?

yingsu00

The last two commits failed EasyCLA. Maybe @yhwang need to sign it?

yingsu00

Commit "Upgrade bootstrap and jquery for single page app" has a UI test failure https://github.com/prestodb/presto/actions/runs/13209836310/job/36881104233. Can you please fix it? Every commit needs to pass all tests. Thanks!

yhwang · 2025-02-10T22:59:22Z

@yingsu00 I have CLA. the CLA failing is from other emails. But I think we can skip that. And for the flow error. @unidevel fixed it in this commit: 6c370d8

yingsu00 · 2025-02-10T23:05:20Z

6c370d8

It cannot be a separate commit. EVERY commit in the PR needs to pass ALL tests. It needs to be squashed with the failing commit.

yingsu00 · 2025-02-10T23:08:15Z

@sh-shamsan Can you please mark if the changes were fixed on @unidevel's original comments? Like the first a few ones, can you please cross out the lines if it's fixed? Thanks!

yhwang · 2025-02-11T04:24:56Z

@yingsu00 all GH actions pass except the CLA. I also tracked the remaining style issues on the other issue. We will address them in a separate issue.

ethanyzhang · 2025-02-11T17:53:49Z

@rschlussel Can you share some guidance on the CLA part - the main authors are all signed but we there were some old commits we cherry-picked from our internal fork and we added coauthors. Those may not have CLA signed. Do we need to spend time to chase all of them?

rschlussel · 2025-02-11T21:55:09Z

I can't merge it without the cla check passing.

ethanyzhang · 2025-02-11T21:56:06Z

We are rebasing to fix

Presto web app CVE-related package updates and its side effect fixes. The main change is to update bootstrap to 5 and jQuery to 3.7.1 to resolve the security issues. This cherry-pick resolves all conflicts introduced along with applying the updates to new prestodb components: QueryOverview, QueryPlanView, QueryResults, QueryStageView, QueryViewer, SQLInput, StaticQueryHeader, and SessionProps. And adds a minor change to the proxy object in webpack.config to resolve an issue with yarn serve command not working. Co-authored-by: Roney Thomas <[email protected]> Co-authored-by: Alia-Alsheha <[email protected]> Co-authored-by: Mohammad-Linjawi <[email protected]> Co-authored-by: Dawi-Alotaibi <[email protected]> Co-authored-by: a-alosaimi <[email protected]> Co-authored-by: Shibil-Rahman-P2 <[email protected]>

test event click Co-authored-by: Sarah Alqahtani <[email protected]>

Co-authored-by: sarah-alqahtani <[email protected]>

Check if event ID exists before processing and prevent child events Co-authored-by: Sarah Alqahtani <[email protected]>

…mponent to queryId & nodeId. Co-authored-by: Mohammed Alhazmi <[email protected]>

switch from dagre-d3 to dagre-d3-es.

this commit upgrades bootstrap and jquery in the single page application along with bug fixes in the main components like stage view page - add bootstrap js/css and highligh.js properly to the single-page application's html. - use proper css to hide/show component - update nav bar css to new boostrap - update the modal dialog to align with the new bootstrap usage. - update d3 imports in d3utils - fix flow error - fix no key warning in the <ul> list - regenerate yarn.lock to fix flow error - fix alignments and dropdown Co-authored-by: Shahad <[email protected]> Co-authored-by: unidevel <[email protected]> Signed-off-by: Yihong Wang <[email protected]>

yhwang · 2025-02-12T00:28:37Z

@rschlussel everything passed finally!

prestodb-ci added the from:IBM PR from IBM label Nov 28, 2024

prestodb-ci requested a review from a team November 28, 2024 08:01

prestodb-ci requested review from zuyu and agrawalreetika and removed request for a team November 28, 2024 08:01

sh-shamsan force-pushed the ui-forwardfit branch from 98ef9ae to 5c9ec2f Compare November 28, 2024 09:56

sh-shamsan force-pushed the ui-forwardfit branch from c92d467 to 5c9ec2f Compare December 7, 2024 01:20

sh-shamsan force-pushed the ui-forwardfit branch from 1d99df5 to 715c8c7 Compare January 20, 2025 14:11

steveburnett previously approved these changes Jan 21, 2025

View reviewed changes

ethanyzhang requested a review from unidevel January 30, 2025 17:04

sh-shamsan dismissed steveburnett’s stale review via e1a4cc3 February 3, 2025 11:54

sh-shamsan force-pushed the ui-forwardfit branch 5 times, most recently from 076c618 to ee5d950 Compare February 4, 2025 13:06

sh-shamsan marked this pull request as ready for review February 4, 2025 14:53

sh-shamsan requested review from elharo, a team and yhwang as code owners February 4, 2025 14:53

sh-shamsan requested a review from presto-oss February 4, 2025 14:53

yhwang previously approved these changes Feb 4, 2025

View reviewed changes

presto-ui/src/components/PageTitle.jsx Outdated Show resolved Hide resolved

presto-ui/src/components/QueryHeader.jsx Outdated Show resolved Hide resolved

unidevel reviewed Feb 4, 2025

View reviewed changes

presto-ui/src/components/LivePlan.jsx Show resolved Hide resolved

sh-shamsan requested a review from steveburnett February 5, 2025 12:53

unidevel previously approved these changes Feb 8, 2025

View reviewed changes

unidevel dismissed their stale review via 6c370d8 February 10, 2025 17:49

yhwang approved these changes Feb 10, 2025

View reviewed changes

steveburnett approved these changes Feb 10, 2025

View reviewed changes

yingsu00 reviewed Feb 10, 2025

View reviewed changes

yhwang force-pushed the ui-forwardfit branch from 6c370d8 to 8c70cdd Compare February 10, 2025 23:32

yhwang mentioned this pull request Feb 10, 2025

[tracking] Enhancements for the UX #21062

Open

16 tasks

yhwang force-pushed the ui-forwardfit branch from 8c70cdd to 78c004d Compare February 11, 2025 21:58

sh-shamsan and others added 9 commits February 11, 2025 14:25

Fix issue with Node details not showing on Stage page

8923c39

test event click Co-authored-by: Sarah Alqahtani <[email protected]>

Update stage navigation to match selected plan event

bfec191

Co-authored-by: sarah-alqahtani <[email protected]>

Check if event ID exists before processing and prevent child events

ec7bf90

Check if event ID exists before processing and prevent child events Co-authored-by: Sarah Alqahtani <[email protected]>

Added data sanitization to queryId and nodeId, along with encodeURICo…

dd19f54

…mponent to queryId & nodeId. Co-authored-by: Mohammed Alhazmi <[email protected]>

fix(dependencies): switch from dagre-d3 to dagre-d3-es

7926add

switch from dagre-d3 to dagre-d3-es.

add signature index and event type for type safety

d42c85c

Update presto console docs

a281a52

yhwang force-pushed the ui-forwardfit branch from 78c004d to e3f5668 Compare February 11, 2025 22:25

yingsu00 approved these changes Feb 12, 2025

View reviewed changes

yingsu00 merged commit b835409 into prestodb:master Feb 12, 2025
55 checks passed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

presto-ui forwardfit #24167

presto-ui forwardfit #24167

sh-shamsan commented Nov 28, 2024 •

edited

Loading

linux-foundation-easycla bot commented Nov 28, 2024 •

edited

Loading

steveburnett left a comment

ethanyzhang commented Jan 30, 2025

yhwang left a comment

steveburnett commented Feb 4, 2025

unidevel commented Feb 4, 2025 •

edited

Loading

unidevel commented Feb 4, 2025 •

edited

Loading

unidevel commented Feb 4, 2025 •

edited by yhwang

Loading

steveburnett left a comment

yingsu00 left a comment

yingsu00 left a comment

yingsu00 left a comment •

edited

Loading

yhwang commented Feb 10, 2025

yingsu00 commented Feb 10, 2025

yingsu00 commented Feb 10, 2025

yhwang commented Feb 11, 2025

ethanyzhang commented Feb 11, 2025

rschlussel commented Feb 11, 2025

ethanyzhang commented Feb 11, 2025

yhwang commented Feb 12, 2025

presto-ui forwardfit #24167

presto-ui forwardfit #24167

Conversation

sh-shamsan commented Nov 28, 2024 • edited Loading

Description

Motivation and Context

Impact

Test Plan

Contributor checklist

Release Notes

linux-foundation-easycla bot commented Nov 28, 2024 • edited Loading

steveburnett left a comment

Choose a reason for hiding this comment

ethanyzhang commented Jan 30, 2025

yhwang left a comment

Choose a reason for hiding this comment

steveburnett commented Feb 4, 2025

unidevel commented Feb 4, 2025 • edited Loading

unidevel commented Feb 4, 2025 • edited Loading

unidevel commented Feb 4, 2025 • edited by yhwang Loading

steveburnett left a comment

Choose a reason for hiding this comment

yingsu00 left a comment

Choose a reason for hiding this comment

yingsu00 left a comment

Choose a reason for hiding this comment

yingsu00 left a comment • edited Loading

Choose a reason for hiding this comment

yhwang commented Feb 10, 2025

yingsu00 commented Feb 10, 2025

yingsu00 commented Feb 10, 2025

yhwang commented Feb 11, 2025

ethanyzhang commented Feb 11, 2025

rschlussel commented Feb 11, 2025

ethanyzhang commented Feb 11, 2025

yhwang commented Feb 12, 2025

sh-shamsan commented Nov 28, 2024 •

edited

Loading

linux-foundation-easycla bot commented Nov 28, 2024 •

edited

Loading

unidevel commented Feb 4, 2025 •

edited

Loading

unidevel commented Feb 4, 2025 •

edited

Loading

unidevel commented Feb 4, 2025 •

edited by yhwang

Loading

yingsu00 left a comment •

edited

Loading