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

Fixed getting Folders by Permissions #4897

Merged
merged 2 commits into from
Nov 3, 2021
Merged

Fixed getting Folders by Permissions #4897

merged 2 commits into from
Nov 3, 2021

Conversation

yog-it
Copy link
Contributor

@yog-it yog-it commented Nov 3, 2021

Fixes #4573

Summary

Retrieving folders per permissions needed to include conditionals for both grant and deny permissions; also using CTE instead of temp table for better performance.

Copy link
Contributor

@david-poindexter david-poindexter left a comment

Choose a reason for hiding this comment

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

I didn't have time to test out this PR, but I did notice the GitHub issue number here and think we should remove probably remove it. We'll have a record of it in the git history.

Copy link
Contributor

@valadas valadas left a comment

Choose a reason for hiding this comment

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

It's really funny because I was just looking into this today for a couple of hours and then came here and noticed this PR.

I did run some tests locally and it looks like it's behaving the way it should. Great work!

@valadas valadas added this to the 9.10.3 milestone Nov 3, 2021
…9.10.03.SqlDataProvider

Co-authored-by: David Poindexter <dpoindexter@nvisionative.com>
Copy link
Contributor

@mitchelsellers mitchelsellers left a comment

Choose a reason for hiding this comment

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

I too tested this one, and it seems to be working well

@david-poindexter david-poindexter merged commit 2d25791 into dnnsoftware:develop Nov 3, 2021
@cgentryls
Copy link

@yog-it thank you for working on and resolving this! I really appreciate it! And, I would like to thank all of you for your continued work on DNN! I inherited our DNN site a little over 6 years ago upgrading it from 4.9.5 to 7.4.1. Currently we are at 9.7.2. I work with other CMS platforms and DNN is by far the best! Thanks again and long live DNN!

@yog-it
Copy link
Contributor Author

yog-it commented Nov 3, 2021

@cgentryls my pleasure. I too have been a long time DNN user and agree completely, it's just one of the best open source CMS platforms out there. I started contributing only a short while ago, but love working with the project and helping out other DNN'ers.

@yog-it yog-it deleted the issue4573 branch November 3, 2021 19:52
@david-poindexter
Copy link
Contributor

@cgentryls and @yog-it thanks so much for your comments - this is what fuels us all! DNN Platform is fantastic and the community is "fantasticer"!!! 😁🎉

@valadas valadas modified the milestones: 9.10.3, 9.11.0 Sep 28, 2022
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.

Resource Manager permissions bug/issue in DNN 9.8.1 and above (HTML Editor Browse Server showing all folders)
5 participants