[PM-23134] Update PolicyDetails sprocs for performance #6421
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
…zationUserRepository - Implemented multiple test cases to verify the behavior of GetByUserIdWithPolicyDetailsAsync for different user statuses (Confirmed, Accepted, Invited, Revoked). - Ensured that the method returns correct policy details based on user status and organization. - Added tests for scenarios with multiple organizations and non-existing policy types. - Included checks for provider users and custom user permissions. These tests enhance coverage and ensure the correctness of policy retrieval logic.
… access as a provider
…access logic - Introduced a Common Table Expression (CTE) for organization users to streamline the selection process based on user status and email. - Added a CTE for providers to enhance clarity and maintainability. - Updated the main query to utilize the new CTEs, improving readability and performance. - Ensured that the procedure correctly identifies provider access based on user permissions.
…ure to enhance user access logic - Introduced a Common Table Expression (CTE) for organization users to improve selection based on user status and email. - Updated the main query to utilize the new CTEs, enhancing readability and performance. - Adjusted the logic for identifying provider access to ensure accurate policy retrieval based on user permissions.
- Created a new view, UserProviderAccessView, to streamline user access to provider organizations. - Introduced two stored procedures: PolicyDetails_ReadByUserId and OrganizationUser_ReadByUserIdWithPolicyDetails, enhancing the logic for retrieving policy details based on user ID and policy type. - Utilized Common Table Expressions (CTEs) to improve query readability and performance, ensuring accurate policy retrieval based on user permissions and organization status.
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #6421 +/- ##
==========================================
+ Coverage 50.42% 54.65% +4.22%
==========================================
Files 1853 1853
Lines 82394 82120 -274
Branches 7265 7260 -5
==========================================
+ Hits 41550 44880 +3330
+ Misses 39260 35574 -3686
- Partials 1584 1666 +82 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
|
New Issues (1)Checkmarx found the following issues in this Pull Request
|
…licyType and update unit tests
… implementations in PolicyRepository classes
JimmyVo16
requested changes
Oct 7, 2025
| [dbo].[OrganizationUserView] OU | ||
| WHERE | ||
| (OU.[Status] <> 0 AND OU.[UserId] = @UserId) | ||
| OR (OU.[Status] = 0 AND OU.[Email] = @UserEmail AND @UserEmail IS NOT NULL) |
There was a problem hiding this comment.
We should use UNION instead of OR. Here’s an example: maybe we can turn this UNION into a view and use it for both stored procedures.
There was a problem hiding this comment.
@bitwarden/dept-dbops what do you think?
There was a problem hiding this comment.
I did a quick test and the UNION does perform slightly better than the OR clause, which is what I expected.
|
@JimmyVo16 has already been looking at this so I'll let him handle the review. |
…ure to use UNION instead of OR
JimmyVo16
approved these changes
Oct 9, 2025
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
🎟️ Tracking
https://bitwarden.atlassian.net/browse/PM-23134
📔 Objective
Modified
OrganizationUser_ReadByUserIdWithPolicyDetailsto use a CTE and the new stored procedureUserProviderAccessView, as outlined in the dbops update.Updated
PolicyRequirementQueryto usePolicyDetails_ReadByUserIdsPolicyType.⏰ Reminders before review
🦮 Reviewer guidelines
:+1:) or similar for great changes:memo:) or ℹ️ (:information_source:) for notes or general info:question:) for questions:thinking:) or 💭 (:thought_balloon:) for more open inquiry that's not quite a confirmed issue and could potentially benefit from discussion:art:) for suggestions / improvements:x:) or:warning:) for more significant problems or concerns needing attention:seedling:) or ♻️ (:recycle:) for future improvements or indications of technical debt:pick:) for minor or nitpick changes