Skip to content

Conversation

@betodealmeida
Copy link
Member

@betodealmeida betodealmeida commented Sep 10, 2021

SUMMARY

When importing a dataset, chart or dashboard, add the importer to the list of owners.

BEFORE/AFTER SCREENSHOTS OR ANIMATED GIF

N/A

TESTING INSTRUCTIONS

  1. Import a dashboard.
  2. Check that the user is an owner of the imported dashboard, as well as any associated charts and datasets.

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

Copy link
Member

@amitmiran137 amitmiran137 left a comment

Choose a reason for hiding this comment

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

I'm wondering if there is a usecase where you wouldn't want this new behaviour by default upon importing and instead allow customizing the owner

@eschutho
Copy link
Member

I'm wondering if there is a usecase where you wouldn't want this new behaviour by default upon importing and instead allow customizing the owner

@amitmiran137 like having the option to assign someone else as the owner?

@amitmiran137
Copy link
Member

amitmiran137 commented Sep 10, 2021

I'm wondering if there is a usecase where you wouldn't want this new behaviour by default upon importing and instead allow customizing the owner

@amitmiran137 like having the option to assign someone else as the owner?

Yes. Upon load dashboards in new environments

@codecov
Copy link

codecov bot commented Sep 15, 2021

Codecov Report

Merging #16656 (70eb714) into master (86290cc) will increase coverage by 0.00%.
The diff coverage is 100.00%.

Impacted file tree graph

@@           Coverage Diff           @@
##           master   #16656   +/-   ##
=======================================
  Coverage   76.76%   76.77%           
=======================================
  Files        1007     1007           
  Lines       54125    54133    +8     
  Branches     7374     7374           
=======================================
+ Hits        41550    41558    +8     
  Misses      12335    12335           
  Partials      240      240           
Flag Coverage Δ
mysql 81.75% <100.00%> (+0.04%) ⬆️
postgres 81.81% <100.00%> (+<0.01%) ⬆️
python 81.89% <100.00%> (+<0.01%) ⬆️
sqlite 81.42% <100.00%> (+<0.01%) ⬆️

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

Impacted Files Coverage Δ
superset/charts/commands/importers/v1/utils.py 100.00% <100.00%> (ø)
superset/dashboards/commands/importers/v1/utils.py 81.42% <100.00%> (+0.83%) ⬆️
superset/datasets/commands/importers/v1/utils.py 60.00% <100.00%> (+1.02%) ⬆️

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 86290cc...70eb714. Read the comment docs.

@betodealmeida betodealmeida merged commit 092ef5b into apache:master Sep 15, 2021
opus-42 pushed a commit to opus-42/incubator-superset that referenced this pull request Nov 14, 2021
* fix: set importer as owner

* Fix tests
QAlexBall pushed a commit to QAlexBall/superset that referenced this pull request Dec 28, 2021
* fix: set importer as owner

* Fix tests
@geido
Copy link
Member

geido commented Jan 25, 2022

Fixes #17467

@mistercrunch mistercrunch added 🏷️ bot A label used by `supersetbot` to keep track of which PR where auto-tagged with release labels 🚢 1.4.0 First shipped in 1.4.0 labels Mar 13, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

🏷️ bot A label used by `supersetbot` to keep track of which PR where auto-tagged with release labels size/M 🚢 1.4.0 First shipped in 1.4.0

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants