WIP: jmccannon/ac/transaction-attempt-2

[jrmccannon]

🎟️ Tracking

📔 Objective

This is the second attempt at creating a transaction that can be used throughout the call stack.

Currently, there's an error when a second thread comes in. (I know...the entire point of this is to allow this...I'm working on it 😄 )

The error is

cannot start a transaction within a transaction

My current next steps are:

• Add a Transaction Scope wrapper
  • This will hold the transaction, connection and allow for helper properties/methods with interacting with the transaction
  • this could also allow for it to be put into DI and not be passed directly around.
• ✅ Add retry functionality for other requests to attempt to open transactions once the preceding transactions have finished.

Retry functionality has allowed for tests to pass, but I believe this will be a speed bump in the event we get hit with >10 requests at once.

📸 Screenshots

⏰ Reminders before review

• Contributor guidelines followed
• All formatters and local linters executed and passed
• Written new unit and / or integration tests where applicable
• Protected functional changes with optionality (feature flags)
• Used internationalization (i18n) for all UI strings
• CI builds passed
• Communicated to DevOps any deployment requirements
• Updated any necessary documentation (Confluence, contributing docs) or informed the documentation team

🦮 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

[codecov]

Codecov Report

❌ Patch coverage is 62.62887% with 145 lines in your changes missing coverage. Please review.
✅ Project coverage is 53.39%. Comparing base (29d6288) to head (68a28e0).
⚠️ Report is 50 commits behind head on main.

Files with missing lines	Patch %	Lines
...dminConsole/Repositories/OrganizationRepository.cs	15.58%	64 Missing and 1 partial ⚠️
...Console/Repositories/OrganizationUserRepository.cs	59.45%	30 Missing ⚠️
...Console/Repositories/OrganizationUserRepository.cs	62.50%	18 Missing ⚠️
...itwarden_license/src/Scim/Users/PostUserCommand.cs	75.92%	7 Missing and 6 partials ⚠️
...dminConsole/Repositories/OrganizationRepository.cs	85.00%	8 Missing and 1 partial ⚠️
...rastructure.Dapper/DapperSqlTransactionProvider.cs	0.00%	4 Missing ⚠️
...sers/InviteUsers/InviteOrganizationUsersCommand.cs	89.28%	2 Missing and 1 partial ⚠️
src/Core/Repositories/ISqlTransactionProvider.cs	84.21%	1 Missing and 2 partials ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #6283      +/-   ##
==========================================
+ Coverage   49.20%   53.39%   +4.19%     
==========================================
  Files        1768     1785      +17     
  Lines       78353    79537    +1184     
  Branches     6998     7070      +72     
==========================================
+ Hits        38551    42468    +3917     
+ Misses      38278    35463    -2815     
- Partials     1524     1606      +82     

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

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
• 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

[github-actions]

Checkmarx One – Scan Summary & Details – 5c27088a-de62-44ce-a4e3-fa6e4ef1a08c

Fixed Issues (6)

Great job! The following issues were fixed in this Pull Request

Severity	Issue	Source File / Package
	CSRF	/src/Api/Controllers/CollectionsController.cs: 176
	Use_Of_Hardcoded_Password	/src/Core/Constants.cs: 180
	Use_Of_Hardcoded_Password	/src/Identity/IdentityServer/RequestValidators/SendAccess/SendAccessConstants.cs: 60
	Use_Of_Hardcoded_Password	/src/Identity/IdentityServer/RequestValidators/SendAccess/SendAccessConstants.cs: 56
	Use_Of_Hardcoded_Password	/src/Identity/IdentityServer/RequestValidators/SendAccess/SendAccessConstants.cs: 24
	Use_Of_Hardcoded_Password	/src/Core/Constants.cs: 130

[sonarqubecloud]

Quality Gate passed

Issues
1 New issue
0 Accepted issues

Measures
0 Security Hotspots
0.0% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarQube Cloud

[eliykat]

Provided feedback in Slack, so removing my review. My main suggestion is to limit the footprint of the transaction to the private method of the invite command so that it's not passed around so much, and so that it's not held open for so long.