fix bug opening a project after trying to S&R it#1674
Conversation
…e rather than using an empty GUID
📝 WalkthroughWalkthroughThis update refactors project ID handling across multiple backend components to consistently use nullable GUIDs, improving null safety and error handling. Several method signatures are updated to accept or return nullable GUIDs, and permission checks now require explicit null checks. Cache invalidation and project reset logic are also adjusted for improved consistency. Changes
Poem
Note ⚡️ AI Code Reviews for VS Code, Cursor, WindsurfCodeRabbit now has a plugin for VS Code, Cursor and Windsurf. This brings AI code reviews directly in the code editor. Each commit is reviewed immediately, finding bugs before the PR is raised. Seamless context handoff to your AI code agent ensures that you can easily incorporate review feedback. Note ⚡️ Faster reviews with cachingCodeRabbit now supports caching for code and dependencies, helping speed up reviews. This means quicker feedback, reduced wait times, and a smoother review experience overall. Cached data is encrypted and stored securely. This feature will be automatically enabled for all accounts on May 16th. To opt out, configure ✨ Finishing Touches
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
backend/LexBoxApi/GraphQL/ProjectMutations.cs (1)
363-368: Also unwrap for theFindAsynccall (optional clarity)
FindAsync(projectId)will compile, but passing the non-nullableprojectId.Valuemakes the intent clearer and avoids boxing aNullable<Guid>.
This is a nit – feel free to ignore if you prefer the current style.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
Cache: Disabled due to data retention organization setting
Knowledge Base: Disabled due to data retention organization setting
📒 Files selected for processing (7)
backend/LexBoxApi/Controllers/CrdtController.cs(1 hunks)backend/LexBoxApi/Controllers/IntegrationController.cs(1 hunks)backend/LexBoxApi/GraphQL/LexQueries.cs(1 hunks)backend/LexBoxApi/GraphQL/ProjectMutations.cs(5 hunks)backend/LexBoxApi/Services/PermissionService.cs(4 hunks)backend/LexBoxApi/Services/ProjectService.cs(3 hunks)backend/LexCore/ServiceInterfaces/IPermissionService.cs(1 hunks)
⏰ Context from checks skipped due to timeout of 90000ms (4)
- GitHub Check: Build FwHeadless / publish-fw-headless
- GitHub Check: Build API / publish-api
- GitHub Check: Analyze (javascript-typescript)
- GitHub Check: Analyze (csharp)
🔇 Additional comments (8)
backend/LexBoxApi/Controllers/CrdtController.cs (1)
93-98: Good update to handle nullable project ID correctly.This change properly handles the new nullable return type from
LookupProjectId. Usingis nullcheck and.Valueunwrapping improves safety by explicitly handling the case where a project code doesn't exist.backend/LexCore/ServiceInterfaces/IPermissionService.cs (1)
14-14: Appropriate interface update to support user override.The addition of the optional
overrideUserparameter creates consistency with other methods in the interface and allows for permission checks with an explicitly provided user context.backend/LexBoxApi/GraphQL/LexQueries.cs (1)
161-161: Improved permission check using project ID instead of code.This change correctly uses the project's ID for the permission check rather than the project code. This is more consistent and avoids potential issues with the project code to ID mapping.
backend/LexBoxApi/Controllers/IntegrationController.cs (1)
62-63: Good update to handle nullable project ID correctly.This change properly handles the new nullable return type from
LookupProjectId. Using explicit null checking and unwrapping the value only when non-null prevents potential null reference exceptions.backend/LexBoxApi/Services/PermissionService.cs (2)
86-89: New overload looks goodAdding
overrideUsertoAssertCanViewProject(Guid …)aligns the signature with the other Assert helpers and removes the previous need to duplicate logic. 👍
43-46: Good defensive null-handlingEarly-returning
falsewhenLookupProjectIdyieldsnullprevents unnecessary permission checks and possibleNullReferenceExceptions. Thanks for tightening this up.Also applies to: 95-98, 132-135, 190-193
backend/LexBoxApi/Services/ProjectService.cs (2)
60-66: Cache invalidation on creation – nice catchCalling
InvalidateProjectCodeCacheright after the repo is initialised ensures stale entries are purged the moment a project is created, fixing the original bug scenario.
Implementation is concise and in the correct transactional scope.
140-150: Nullable return & null-caching strategy look solidSwitching
LookupProjectIdtoGuid?and explicitly cachingnullavoids repeated DB hits while still allowing later invalidation (handled above). Implementation matches the call-sites. Well done.
fixes a bug reported by a user.
to reproduce:
The problem is that step 1 caused the project code to be associated with an empty guid (because it doesn't exist) and cache that association, when the new project was created that cache was not invalidated, then when asking for permission to view the project it was rejected because the user didn't have permission to view a project of empty guid.
I've added code to invalidate the code to id cache one project creation, in addition I've changed the signature of the project code lookup to return
Guid?so the caller is forced to explicitly handle the null case, rather than just gettingGuid.Empty