Skip to content

fix: Allow dataset owners to see their datasets #20135

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
Merged
dpgaspar merged 1 commit into from
May 25, 2022
Merged

fix: Allow dataset owners to see their datasets #20135

dpgaspar merged 1 commit into from
May 25, 2022
+30 −0

Conversation

@cccs-tom
Copy link
Contributor

@cccs-tom cccs-tom commented May 19, 2022

SUMMARY

Allow dataset owners to view / access their own datasets.

TESTING INSTRUCTIONS

Detailed steps to reproduce are outlined in #19360.

ADDITIONAL INFORMATION

@cccs-tom cccs-tom changed the title Allow dataset owners to see their datasets fix: Allow dataset owners to see their datasets May 19, 2022
@codecov
Copy link

codecov bot commented May 19, 2022 •
edited
Loading

Codecov Report

Merging #20135 (678c353) into master (e2f11d3) will decrease coverage by 12.07%.
The diff coverage is 51.61%.

❗ Current head 678c353 differs from pull request most recent head 6487f8e. Consider uploading reports for the commit 6487f8e to get more accurate results

@@             Coverage Diff             @@
##           master   #20135       +/-   ##
===========================================
- Coverage   66.45%   54.37%   -12.08%     
===========================================
  Files        1721     1721               
  Lines       64497    64549       +52     
  Branches     6805     6811        +6     
===========================================
- Hits        42860    35100     -7760     
- Misses      19905    27716     +7811     
- Partials     1732     1733        +1
Flag Coverage Δ
hive ?
mysql ?
postgres ?
presto 53.58% <66.66%> (+0.01%) ⬆️
python 57.64% <76.19%> (-24.99%) ⬇️
sqlite ?
unit 49.43% <76.19%> (+0.02%) ⬆️

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

Impacted Files Coverage Δ
...plugins/legacy-plugin-chart-heatmap/src/Heatmap.js 0.00% <0.00%> (ø)
...in-chart-echarts/src/MixedTimeseries/buildQuery.ts 100.00% <ø> (ø)
.../plugin-chart-echarts/src/Timeseries/buildQuery.ts 66.66% <ø> (ø)
...hart-pivot-table/src/react-pivottable/utilities.js 0.00% <0.00%> (ø)
...c/SqlLab/components/ExploreResultsButton/index.tsx 100.00% <ø> (ø)
...ts/nativeFilters/FilterBar/ActionButtons/index.tsx 85.71% <ø> (ø)
.../src/explore/components/ControlPanelsContainer.tsx 79.80% <ø> (ø)
...et-frontend/src/explore/reducers/exploreReducer.js 28.75% <0.00%> (ø)
...t-frontend/src/explore/reducers/getInitialState.ts 0.00% <ø> (ø)
superset-frontend/src/utils/localStorageHelpers.ts 90.00% <ø> (ø)
... and 298 more

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 e2f11d3...6487f8e. Read the comment docs.

@cccs-tom cccs-tom mentioned this pull request May 19, 2022
Closed
8 tasks
villebro
villebro approved these changes May 20, 2022
View reviewed changes
Copy link
Member

@villebro villebro left a comment

Choose a reason for hiding this comment

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

LGTM, tested to work both with a logged in user and in anonymous mode (user is undefined):

AFTER

The logged in user is Gamma, who is set as an owner of this particular dataset:
image

BEFORE

The gamma user doesn't see the owned datasets:
image

@villebro villebro requested a review from dpgaspar May 20, 2022 06:16
dpgaspar
dpgaspar reviewed May 20, 2022
View reviewed changes
Copy link
Member

@dpgaspar dpgaspar left a comment

Choose a reason for hiding this comment

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

Thank you for the fix. Can you please add tests?

@cccs-tom
Copy link
Contributor Author

cccs-tom commented May 24, 2022

Thank you for the fix. Can you please add tests?

@dpgaspar Yes, no problem. I've been out of the office for the last few days, so I'll get on that this week.

@cccs-tom cccs-tom force-pushed the feature/grant-dataset-access-to-owners branch from 83ef053 to 6487f8e Compare May 24, 2022 22:16
@cccs-tom cccs-tom requested a review from dpgaspar May 24, 2022 22:16
@dpgaspar dpgaspar merged commit b0c6935 into apache:master May 25, 2022
@cccs-tom cccs-tom deleted the feature/grant-dataset-access-to-owners branch May 25, 2022 14:36
villebro pushed a commit that referenced this pull request May 26, 2022
@cccs-tom @villebro
fix: Allow dataset owners to see their datasets (#20135)
97c4095 
(cherry picked from commit b0c6935)
michael-s-molina pushed a commit that referenced this pull request May 26, 2022
@cccs-tom @michael-s-molina
fix: Allow dataset owners to see their datasets (#20135)
562e3a7 
(cherry picked from commit b0c6935)
philipher29 pushed a commit to ValtechMobility/superset that referenced this pull request Jun 9, 2022
@cccs-tom @philipher29
fix: Allow dataset owners to see their datasets (apache#20135)
cf82d93
@agusfigueroa-htg
Copy link

agusfigueroa-htg commented Jul 15, 2022

Hi! By looking at the release notes I see this not being part of 2.0, any idea in which version this fix will be released? Thanks!

@michael-s-molina
Copy link
Member

michael-s-molina commented Jul 15, 2022

Hi! By looking at the release notes I see this not being part of 2.0, any idea in which version this fix will be released? Thanks!

@agusfigueroa-htg This fix was already included in version 1.5.1 so it's part of 2.0.0 too.

@agusfigueroa-htg
Copy link

agusfigueroa-htg commented Jul 15, 2022 via email

@bnitesh bnitesh mentioned this pull request Aug 11, 2022
Closed
3 tasks
@villebro villebro mentioned this pull request Feb 20, 2023
Merged
9 tasks
@mistercrunch mistercrunch added 🍒 1.5.1 Cherry-picked to 1.5.1 🍒 1.5.2 Cherry-picked to 1.5.2 labels Mar 13, 2024
@mistercrunch mistercrunch added 🍒 1.5.3 Cherry-picked to 1.5.3 🏷️ bot A label used by `supersetbot` to keep track of which PR where auto-tagged with release labels 🚢 2.0.0 First shipped in 2.0.0 labels Mar 13, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@dpgaspar dpgaspar dpgaspar approved these changes

@villebro villebro villebro approved these changes

Assignees

No one assigned

Labels

🏷️ bot A label used by `supersetbot` to keep track of which PR where auto-tagged with release labels size/M 🍒 1.5.1 Cherry-picked to 1.5.1 🍒 1.5.2 Cherry-picked to 1.5.2 🍒 1.5.3 Cherry-picked to 1.5.3 🚢 2.0.0 First shipped in 2.0.0

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Dataset permissions: Users are unable to see the datasets that they create

6 participants

@cccs-tom @agusfigueroa-htg @michael-s-molina @dpgaspar @villebro @mistercrunch