fix(dashboard-banners): Fix cluster scope permission issue #3843
Conversation
This was intentional because there was a 403 Permission Error being thrown which prevented the user from dismissing the Cluster scoped banners while dismissals for other scopes seemed to be working. Not sure why. No doctype level role permission updates or has_permission overrides seemed to help in any way. Will investigate this further but pushing this workaround for now.
regdocs
force-pushed
the
dashboard-banners
branch
from
cc77936 to
e90ec57
Compare
| @frappe.whitelist() | ||
| def dismiss_banner(banner_name): | ||
| user = frappe.session.user | ||
| banner = frappe.get_doc("Dashboard Banner", banner_name) | ||
| if banner and banner.is_dismissible and not banner.is_global: | ||
| banner.append( | ||
| "user_dismissals", | ||
| { | ||
| "user": user, | ||
| "dismissed_at": frappe.utils.now(), | ||
| "parent": banner_name, | ||
| }, | ||
| ) | ||
| banner.save() | ||
| banner.save(ignore_permissions=True) | ||
| return True | ||
| return False |
There was a problem hiding this comment.
I think this entire block should be inside the banner controller.
ignore_permissions is a bad way to fix things. Most probably the issue is with permissions on a specific banner, which can be fixed with has_permission hook.
There was a problem hiding this comment.
I agree. I had tried the controller thing initially but was hitting a permission road block. Will attempt it again and let you know how that goes.
Codecov Report❌ Patch coverage is
❌ Your patch check has failed because the patch coverage (0.00%) is below the target coverage (75.00%). You can increase the patch coverage or adjust the target coverage. Additional details and impacted files@@ Coverage Diff @@
## develop #3843 +/- ##
===========================================
- Coverage 45.02% 45.01% -0.02%
===========================================
Files 714 714
Lines 48570 48570
===========================================
- Hits 21868 21863 -5
- Misses 26702 26707 +5 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
This is a workaround to prevent the cluster scoped banners' dismissal permission issue where the dismissal was failing for unprivileged Press Admin users but succeeding for System Managers. Need to investigate this further.