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: Dashboard Chart permissions and caching, fixes: #26023 and #26020 #26024

Closed
wants to merge 1 commit into from
Closed

fix: Dashboard Chart permissions and caching, fixes: #26023 and #26020 #26024

wants to merge 1 commit into from

Conversation

casesolved-co-uk
Copy link
Contributor

@casesolved-co-uk casesolved-co-uk commented Apr 17, 2024 •
edited

fix: Dashboard Chart permissions and caching

fixes: #26023
fixes: #26020
fixes: #26010
replaces: #26019
related: #26021

version-14-hotfix
version-15-hotfix

@casesolved-co-uk casesolved-co-uk requested a review from a team as a code owner April 17, 2024 23:39
@casesolved-co-uk casesolved-co-uk requested review from ankush and removed request for a team April 17, 2024 23:39
@github-actions github-actions bot added the add-test-cases Add test case to validate fix or enhancement label Apr 17, 2024
@casesolved-co-uk casesolved-co-uk force-pushed the dashchart_perms2 branch 8 times, most recently from a220dc5 to c6f7d42 Compare April 18, 2024 01:20
@casesolved-co-uk
Copy link
Contributor Author

@ankush Still has a bug because get_permission_query_conditions doesn't check the Dashboard Chart doc permissions table, but has_permission does.

Much better than it was though bug and performance-wise.

@casesolved-co-uk casesolved-co-uk mentioned this pull request Apr 20, 2024
Closed
@stale Stale
Copy link

stale bot commented May 5, 2024

This pull request has been automatically marked as stale because it has not had recent activity. It will be closed within 3 days if no further activity occurs, but it only takes a comment to keep a contribution alive :) Also, even if it is closed, you can always reopen the PR when you're ready. Thank you for contributing.

@stale stale bot added inactive and removed inactive labels May 5, 2024
@casesolved-co-uk casesolved-co-uk force-pushed the dashchart_perms2 branch 3 times, most recently from 3814dfb to e2d7337 Compare May 7, 2024 18:43
@casesolved-co-uk
Copy link
Contributor Author

@ankush fixed and ready for review and merge

@casesolved-co-uk
fix: Dashboard Chart permissions and caching, fixes: frappe#26023 and f…
853a845 
…rappe#26020

chore: disable semgrep
chore: formatting
chore: formatting
chore: formatting
chore: formatting
feat: add permissions test case
chore: formatting
chore: formatting
fix: the actual bug! frappe#26010
fix: typos in test
fix: test typo
fix: chart permissions caching and roles and test chart delete
fix: role db escape
fix: normalise sql
fix: db query has no child tables
@casesolved-co-uk
Copy link
Contributor Author

Had problems with the listview db_query needing subquery for Has Role

@stale Stale
Copy link

stale bot commented May 22, 2024

This pull request has been automatically marked as stale because it has not had recent activity. It will be closed within 3 days if no further activity occurs, but it only takes a comment to keep a contribution alive :) Also, even if it is closed, you can always reopen the PR when you're ready. Thank you for contributing.

@stale stale bot added the inactive label May 22, 2024
@casesolved-co-uk
Copy link
Contributor Author

This bug fix is available here: https://github.com/CaseSolvedUK/frappe

@casesolved-co-uk casesolved-co-uk deleted the dashchart_perms2 branch May 22, 2024 14:56
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@ankush ankush Awaiting requested review from ankush ankush was automatically assigned from frappe/frappe-review-team

Assignees
No one assigned
Labels
add-test-cases Add test case to validate fix or enhancement inactive
Projects
None yet
Milestone
No milestone
1 participant
@casesolved-co-uk