Skip to content

Conversation

@myieye
Copy link
Collaborator

@myieye myieye commented Sep 22, 2025

Resolves #2011

  • Changes draft deletion to not use ExecuteDeleteAsync, because it should be part of the db-transaction.
  • Delete-repo if any part of the project-creation fails
  • Only send email to requesting user after everything has succeeded.

I got this error while creating this PR. Ahhh creating things well 😆
image

@github-actions github-actions bot added the 📦 Lexbox issues related to any server side code, fw-headless included label Sep 22, 2025
@coderabbitai
Copy link

coderabbitai bot commented Sep 22, 2025

📝 Walkthrough

Walkthrough

Adjusts CreateProject flow in ProjectService: safely removes an optional draft project, wraps repository initialization and cache invalidation in a try/catch that commits on success, deletes the created repo and rethrows on failure, and removes a later unconditional transaction commit to centralize commit behavior.

Changes

Cohort / File(s) Summary of Changes
Project creation flow and transaction handling
backend/LexBoxApi/Services/ProjectService.cs
- Null-check removal of draftProject before deleting
- Wrap InitRepo and cache invalidations in try/catch; commit on success
- On failure, delete created repo via hgService.DeleteRepo and rethrow
- Remove unconditional post-email transaction commit

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Possibly related PRs

Suggested reviewers

  • hahn-kev

Poem

I thump my paws—commit is neat,
Try-catch burrows, safely fleet.
Drafts hop out with gentle care,
Repos cleaned on failure’s snare.
Cache winds shift, then all is right—
A tidy warren, tests alight. 🐇✨

Pre-merge checks and finishing touches

❌ Failed checks (1 warning)
Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. You can run @coderabbitai generate docstrings to improve docstring coverage.
✅ Passed checks (2 passed)
Check name Status Explanation
Title Check ✅ Passed The title "Delete draft projects as part of the project-create transaction" is concise, focused, and accurately summarizes the primary change (moving draft deletion into the project-creation DB transaction), which aligns with the ProjectService.cs changes and the PR objectives.
Description Check ✅ Passed The PR description clearly states the key changes (avoid ExecuteDeleteAsync so deletion is in the transaction, delete repo on failure, and send email only after success) and directly corresponds to the raw_summary and PR objectives, so it is on-topic and describes the changeset.
✨ Finishing touches
  • 📝 Generate Docstrings
🧪 Generate unit tests
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch 2011-partial-project-creation

Tip

👮 Agentic pre-merge checks are now available in preview!

Pro plan users can now enable pre-merge checks in their settings to enforce checklists before merging PRs.

  • Built-in checks – Quickly apply ready-made checks to enforce title conventions, require pull request descriptions that follow templates, validate linked issues for compliance, and more.
  • Custom agentic checks – Define your own rules using CodeRabbit’s advanced agentic capabilities to enforce organization-specific policies and workflows. For example, you can instruct CodeRabbit’s agent to verify that API documentation is updated whenever API schema files are modified in a PR. Note: Upto 5 custom checks are currently allowed during the preview period. Pricing for this feature will be announced in a few weeks.

Please see the documentation for more information.

Example:

reviews:
  pre_merge_checks:
    custom_checks:
      - name: "Undocumented Breaking Changes"
        mode: "warning"
        instructions: |
          Pass/fail criteria: All breaking changes to public APIs, CLI flags, environment variables, configuration keys, database schemas, or HTTP/GraphQL endpoints must be documented in the "Breaking Change" section of the PR description and in CHANGELOG.md. Exclude purely internal or private changes (e.g., code not exported from package entry points or explicitly marked as internal).

Please share your feedback with us on this Discord post.


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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@github-actions
Copy link

C# Unit Tests

130 tests   130 ✅  19s ⏱️
 20 suites    0 💤
  1 files      0 ❌

Results for commit a50899a.

Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 1

🧹 Nitpick comments (1)
backend/LexBoxApi/Services/ProjectService.cs (1)

72-77: Don’t let cleanup mask the original failure.

If DeleteRepo throws, the original exception is lost. Wrap it as best‑effort.

 catch
 {
   // CommitAsync() did not run [successfully], so we don't want a repo to exist
-  await hgService.DeleteRepo(input.Code);
+  try { await hgService.DeleteRepo(input.Code); }
+  catch { /* best-effort cleanup: ignore */ }
   throw;
 }
📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 8ce1d70 and a50899a.

📒 Files selected for processing (1)
  • backend/LexBoxApi/Services/ProjectService.cs (1 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
backend/LexBoxApi/Services/ProjectService.cs (1)
backend/LexCore/Entities/User.cs (1)
  • UpdateCreateProjectsPermission (30-33)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
  • GitHub Check: Build API / publish-api
  • GitHub Check: Analyze (csharp)
🔇 Additional comments (1)
backend/LexBoxApi/Services/ProjectService.cs (1)

55-58: Draft deletion now participates in the transaction — good change.

This guarantees the draft is only removed when the project creation succeeds.

@myieye myieye merged commit 6111559 into develop Oct 1, 2025
12 checks passed
@myieye myieye deleted the 2011-partial-project-creation branch October 1, 2025 12:46
@myieye myieye changed the title Delete draft projects as part of the project-create transaction Prevent partial project creation Oct 8, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

📦 Lexbox issues related to any server side code, fw-headless included

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Partial project creation

2 participants