Skip to content

Conversation

@eliykat
Copy link
Member

@eliykat eliykat commented Jan 3, 2026

🎟️ Tracking

N/A - draft/experimental work only

📔 Objective

The OrganizationUser object is ambiguous.

  • It can represent invited, accepted and confirmed users, all of which use different combinations of properties. This is handled through nullable properties, but it's left to the developer to know which are or should be populated.
  • It can represent revoked users. Today we treat revoked as a separate status, but it is really an overlay on the other statuses. This is because the OrganizationUser object retains its characteristics of its underlying status. (e.g. an OrgUser that is invited and then revoked still has the arrangement of an invited user, and will look different to an OrgUser who is confirmed and then revoked.)

This PR is a proof of concept for how we might have stronger typed classes for representing OrganizationUsers in the code.

Core ideas:

  • separate classes to represent invited, accepted, and confirmed states. These only have the properties that are relevant to that status.
  • make revoked a boolean flag on each class, rather than a separate class. This makes it easy to keep track of the "real" status of the user, while still recording whether they are revoked. (I would like to make this change in the database as well but that is out of scope here.)
  • mapping methods to transform between each state - for the accepted/confirmed commands
  • mapping methods to transform between each state and the OrganizationUser database object - to smooth over saving and loading from the database

For feedback from the team.

Possible additional improvements:

  • querying by userId will (should) never return an invited user, so there should be a more restrictive interface or OneOf that excludes invited users in that case
  • we need additional query classes for each main way to query an orgUser object (by id, by userId, by orgId, etc)
  • may consider moving the mapping methods off the objects themselves to prevent general mis/use

📸 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

@github-actions
Copy link
Contributor

github-actions bot commented Jan 3, 2026

Logo
Checkmarx One – Scan Summary & Detailsb34fb447-b05f-4c37-b982-0fb71226ec2a

Great job! No new security vulnerabilities introduced in this pull request

@codecov
Copy link

codecov bot commented Jan 3, 2026

Codecov Report

❌ Patch coverage is 0% with 243 lines in your changes missing coverage. Please review.
✅ Project coverage is 54.72%. Comparing base (e9d53c0) to head (f96dba4).

Files with missing lines Patch % Lines
...rganizationUsers/Models/InvitedOrganizationUser.cs 0.00% 78 Missing ⚠️
...ganizationUsers/Models/AcceptedOrganizationUser.cs 0.00% 77 Missing ⚠️
...anizationUsers/Models/ConfirmedOrganizationUser.cs 0.00% 64 Missing ⚠️
...ures/OrganizationUsers/GetOrganizationUserQuery.cs 0.00% 17 Missing ⚠️
...onUsers/Interfaces/IOrganizationUserPermissions.cs 0.00% 7 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main    #6794      +/-   ##
==========================================
- Coverage   54.87%   54.72%   -0.16%     
==========================================
  Files        1925     1930       +5     
  Lines       85355    85598     +243     
  Branches     7652     7674      +22     
==========================================
+ Hits        46842    46845       +3     
- Misses      36726    36967     +241     
+ Partials     1787     1786       -1     

☔ 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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants