Add labels to conflicting partnership date range errors #3405
Conversation
📝 WalkthroughWalkthroughThe changes introduce an explicit export for the asNonEmpty utility function, making it accessible outside its module. In the partnership date override validation handler, the logic is updated to enrich conflict data by loading and attaching organization names to each conflict entry instead of just partnership IDs. This enhancement requires the handler to depend on ResourceLoader and OrganizationLoader. The constructor of the handler is updated to include the new dependency, and the conflict enrichment logic is adjusted to utilize organization data when reporting date override conflicts. Changes
Possibly related PRs
✨ 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:
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 (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (2)
src/components/partnership/handlers/validate-partnership-date-overrides-on-project-change.handler.ts (2)
40-58: Nice enhancement to add organization names to conflict messages.The code now loads organization data and enriches the conflict information with more user-friendly organization names instead of just partnership IDs.
One small concern: The error handling for organization loading uses a bare fallback. Consider adding error logging here.
if (org instanceof Error) { // Shouldn't happen + // Log the error for debugging purposes + console.error(`Failed to load organization for partnership ${conflict.id}:`, org); return conflict; }
67-67: Non-null assertion is safe but could be handled differently.Since
conflictsis already verified as non-empty (line 39), andconflictsWithLabelsis derived from it, the non-null assertion is safe. However, you could make this more explicit:- asNonEmpty(conflictsWithLabels)!, + // conflictsWithLabels is non-empty because conflicts is non-empty + asNonEmpty(conflictsWithLabels) as NonEmptyArray<typeof conflictsWithLabels[number]>,
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
src/common/exceptions/date-override-conflict.exception.ts(1 hunks)src/components/partnership/handlers/validate-partnership-date-overrides-on-project-change.handler.ts(3 hunks)
⏰ Context from checks skipped due to timeout of 90000ms (11)
- GitHub Check: E2E Tests (neo4j 6/6)
- GitHub Check: E2E Tests (neo4j 5/6)
- GitHub Check: E2E Tests (neo4j 4/6)
- GitHub Check: E2E Tests (neo4j 3/6)
- GitHub Check: E2E Tests (neo4j 2/6)
- GitHub Check: Unit
- GitHub Check: Generate (head)
- GitHub Check: E2E Tests (neo4j 1/6)
- GitHub Check: Generate (base)
- GitHub Check: lint
- GitHub Check: Clean
🔇 Additional comments (4)
src/common/exceptions/date-override-conflict.exception.ts (1)
73-74: Good addition of the exported utility function.Making
asNonEmptyexplicitly exportable allows its reuse in other modules, supporting the enhanced conflict reporting in the partnership validation handler.src/components/partnership/handlers/validate-partnership-date-overrides-on-project-change.handler.ts (3)
1-4: Good import additions for enhanced functionality.The imports support the new feature of enriching conflict details with organization names.
12-15: Constructor updated with new dependency.The
ResourceLoaderdependency is correctly added to access theOrganizationLoader.
34-34: Using partnership ID as temporary label.This line sets up the initial label with the partnership ID, which will be replaced with organization names later in the code.
Left as TODO in a5bf2f9
We can eagerly load the label after we finish gel. I didn't want to bother with neo4j