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.

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

fix: Revert "fix: RBAC hide right menu (#16902)" #16968

Merged
merged 2 commits into from
Oct 5, 2021
Merged

fix: Revert "fix: RBAC hide right menu (#16902)" #16968

merged 2 commits into from
Oct 5, 2021

Conversation

eschutho
Copy link
Member

@eschutho eschutho commented Oct 4, 2021 •
edited
Loading

This reverts commit 87baac7.

We were seeing some bugs on master where SQL Lab wouldn't load with a production build with this error:
Superset

SUMMARY

BEFORE/AFTER SCREENSHOTS OR ANIMATED GIF

production build after revert:

Superset

TESTING INSTRUCTIONS

ADDITIONAL INFORMATION

  • Has associated issue:
  • Required feature flags:
  • Changes UI
  • Includes DB Migration (follow approval process in SIP-59)
    • Migration is atomic, supports rollback & is backwards-compatible
    • Confirm DB migration upgrade and downgrade tested
    • Runtime estimates and downtime expectations provided
  • Introduces new feature or API
  • Removes existing feature or API

@eschutho eschutho requested a review from hughhhh October 4, 2021 23:07
@rusackas rusackas changed the title Revert "fix: RBAC hide right menu (#16902)" fix: Revert "fix: RBAC hide right menu (#16902)" Oct 4, 2021
etr2460
etr2460 approved these changes Oct 4, 2021
View reviewed changes
Copy link
Member

@etr2460 etr2460 left a comment

Choose a reason for hiding this comment

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

looks like this fixes it. we'll need to figure out why next

@eschutho
Copy link
Member Author

eschutho commented Oct 4, 2021

/testenv up

@apache apache deleted a comment from github-actions bot Oct 4, 2021
@apache apache deleted a comment from github-actions bot Oct 4, 2021
@apache apache deleted a comment from github-actions bot Oct 4, 2021
@apache apache deleted a comment from github-actions bot Oct 4, 2021
@rusackas
Copy link
Member

rusackas commented Oct 4, 2021

/testenv up

@github-actions
Copy link
Contributor

github-actions bot commented Oct 4, 2021

@rusackas Ephemeral environment spinning up at http://34.210.197.17:8080. Credentials are admin/admin. Please allow several minutes for bootstrapping and startup.

@eschutho
Copy link
Member Author

eschutho commented Oct 5, 2021

There was a test that was failing and seems to be failing on master as well, although it passes locally. Trying to fix that and then will merge this.

@eschutho eschutho force-pushed the elizabeth/revert-87baac765 branch from 07bfb58 to 0d382f6 Compare October 5, 2021 01:35
@codecov
Copy link

codecov bot commented Oct 5, 2021

Codecov Report

Merging #16968 (0d382f6) into master (739d841) will decrease coverage by 0.00%.
The diff coverage is 86.48%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master   #16968      +/-   ##
==========================================
- Coverage   76.92%   76.92%   -0.01%     
==========================================
  Files        1030     1030              
  Lines       55023    55017       -6     
  Branches     7463     7464       +1     
==========================================
- Hits        42329    42324       -5     
+ Misses      12441    12440       -1     
  Partials      253      253
Flag Coverage Δ
javascript 70.92% <86.48%> (-0.01%) ⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files Coverage Δ
superset-frontend/src/views/menu.tsx 0.00% <ø> (ø)
...uperset-frontend/src/components/Menu/MenuRight.tsx 91.22% <86.48%> (-1.08%) ⬇️
...ontend/src/dashboard/components/menu/HoverMenu.tsx 90.00% <0.00%> (-10.00%) ⬇️
...src/dashboard/components/gridComponents/Header.jsx 100.00% <0.00%> (ø)
...c/dashboard/components/gridComponents/Markdown.jsx 83.16% <0.00%> (ø)
...d/components/DashboardBuilder/DashboardBuilder.tsx 90.17% <0.00%> (ø)

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 739d841...0d382f6. Read the comment docs.

@eschutho
Copy link
Member Author

eschutho commented Oct 5, 2021

@lyndsiWilliams I also reverted one of your test changes.

@eschutho eschutho merged commit 5866d5e into apache:master Oct 5, 2021
@github-actions
Copy link
Contributor

github-actions bot commented Oct 5, 2021

Ephemeral environment shutdown and build artifacts deleted.

@rusackas rusackas deleted the elizabeth/revert-87baac765 branch October 5, 2021 02:37
sadpandajoe pushed a commit to preset-io/superset that referenced this pull request Oct 5, 2021
@eschutho @sadpandajoe
fix: Revert "fix: RBAC hide right menu (apache#16902)" (apache#16968)
c35513e 
* Revert "fix: RBAC hide right menu (apache#16902)"

This reverts commit 87baac7.

* fix failing test

(cherry picked from commit 5866d5e)
@rosemarie-chiu
Copy link
Contributor

🏷 2021.40

@eschutho eschutho added the v1.4 label Oct 26, 2021
eschutho added a commit to preset-io/superset that referenced this pull request Oct 27, 2021
@eschutho
fix: Revert "fix: RBAC hide right menu (apache#16902)" (apache#16968)
a6a3ced 
* Revert "fix: RBAC hide right menu (apache#16902)"

This reverts commit 87baac7.

* fix failing test

(cherry picked from commit 5866d5e)
opus-42 pushed a commit to opus-42/incubator-superset that referenced this pull request Nov 14, 2021
@eschutho
fix: Revert "fix: RBAC hide right menu (apache#16902)" (apache#16968)
7cee9e9 
* Revert "fix: RBAC hide right menu (apache#16902)"

This reverts commit 87baac7.

* fix failing test
QAlexBall pushed a commit to QAlexBall/superset that referenced this pull request Dec 28, 2021
@eschutho
fix: Revert "fix: RBAC hide right menu (apache#16902)" (apache#16968)
687c644 
* Revert "fix: RBAC hide right menu (apache#16902)"

This reverts commit 87baac7.

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

@rosemarie-chiu rosemarie-chiu rosemarie-chiu approved these changes

@hughhhh hughhhh hughhhh approved these changes

@etr2460 etr2460 etr2460 approved these changes

Assignees
No one assigned
Labels
preset:2021.40 preset-io size/L v1.4
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
5 participants
@eschutho @rusackas @rosemarie-chiu @etr2460 @hughhhh