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.

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

Ensure every account has exactly one default dashboard #3236

Merged
merged 3 commits into from
Nov 25, 2024

Conversation

johannaengland
Copy link
Contributor

Another part of fixing #3150.

As mentioned in there it is possible to delete ones default dashboard and no other is set to be default instead, essentially meaning a user can have no default dashboards. We also observed that it has somehow been possible to set multiple dashboards as default (see #2680), we still don't know how that has been possible and if it is still possible.

This PR adds two migrations:

  • the first one will find all accounts that have no default dashboard and set one to default
  • the second one will find all accounts that have more than one default dashboard and remove the default marker from all but one

Since it has not been possible to delete the last dashboard (as seen in #3232) we can assume that each account has at least one dashboard.

Copy link

github-actions bot commented Nov 22, 2024

🦙 MegaLinter status: ✅ SUCCESS

Descriptor Linter Files Fixed Errors Elapsed time
✅ PYTHON black 992 0 12.96s
✅ PYTHON ruff 988 0 0.1s

See detailed report in MegaLinter reports

MegaLinter is graciously provided by OX Security

Copy link

github-actions bot commented Nov 22, 2024

Test results

    9 files      9 suites   8m 54s ⏱️
2 143 tests 2 143 ✅ 0 💤 0 ❌
4 025 runs  4 025 ✅ 0 💤 0 ❌

Results for commit 0071357.

♻️ This comment has been updated with latest results.

Copy link

codecov bot commented Nov 22, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 60.30%. Comparing base (f31d510) to head (0071357).
Report is 4 commits behind head on master.

Additional details and impacted files
@@           Coverage Diff           @@
##           master    #3236   +/-   ##
=======================================
  Coverage   60.30%   60.30%           
=======================================
  Files         605      605           
  Lines       43685    43685           
  Branches       48       48           
=======================================
  Hits        26343    26343           
  Misses      17330    17330           
  Partials       12       12           

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.


🚨 Try these New Features:

@stveit
Copy link
Contributor

stveit commented Nov 22, 2024

im guessing the title should be renamed to

Ensure every account has exactly one default dashboard

@johannaengland johannaengland changed the title Ensure every account has exactly one dashboard Ensure every account has exactly one default dashboard Nov 22, 2024
Comment on lines +1 to +2
-- This migration is to ensure that for accounts that don't have a default
-- dashboard we set a default dashboard
Copy link
Member

Choose a reason for hiding this comment

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

Just leaving a quick comment before the weekend: This is the weakness of NAV's homegrown migration system: This migration file will conflict with #3231. One of us needs to change our PR accordingly :)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Indeed, let's see who it will be 😅

@johannaengland johannaengland force-pushed the bugfix/clean-dashboards-to-one-default branch from de748d8 to 0071357 Compare November 25, 2024 13:09
Copy link

sonarcloud bot commented Nov 25, 2024

@johannaengland johannaengland merged commit 855819a into master Nov 25, 2024
14 checks passed
@johannaengland johannaengland deleted the bugfix/clean-dashboards-to-one-default branch November 25, 2024 13:32
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants