Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: Prevent Merging Empty Incidents #3611

Open
wants to merge 4 commits into
base: main
Choose a base branch
from

Conversation

AkashJana18
Copy link

@AkashJana18 AkashJana18 commented Feb 20, 2025

Closes #3370
/claim #3370

📑 Description

This update enhances the incident merging functionality by introducing stricter validation, improved error handling, and clearer messaging to ensure a more reliable and user-friendly experience. Improves overall system reliability by preventing unintended actions.

Key Changes:

1. Validation Enhancements:

  • Prevents merging when no source incidents are selected.
  • Ensures the destination incident ID is valid before proceeding.

2. Improved Response Handling:

  • Clearly differentiates between merged, skipped, and failed incidents.
  • Provides detailed messages for better visibility into the process.

3. Refined User Experience:

  • Prevents invalid actions before sending requests.
  • Displays more informative success and failure messages.

4. Code Optimizations:

  • Streamlined logging for better clarity.
  • Reduced redundant processing with early exits.

✅ Checks

  • My pull request adheres to the code style of this project
  • My code requires changes to the documentation
  • I have updated the documentation as required
  • All the tests have passed

ℹ Additional Information

🛠️ How to Test:

  1. Attempt merging without selecting source incidents → Should show an error and prevent the action.
  2. Merge valid incidents → Should proceed successfully with a confirmation message.
  3. Merge incidents where some are invalid → Should provide detailed feedback on skipped/failed incidents.
  4. Check responses → The returned messages should align with the UI updates for consistency.

Copy link

vercel bot commented Feb 20, 2025

@AkashJana18 is attempting to deploy a commit to the KeepHQ Team on Vercel.

A member of the Team first needs to authorize it.

@dosubot dosubot bot added the size:L This PR changes 100-499 lines, ignoring generated files. label Feb 20, 2025
@dosubot dosubot bot added the Incident label Feb 20, 2025
@AkashJana18 AkashJana18 changed the title Fix: Prevent Merging Empty Incidents fix: Prevent Merging Empty Incidents Feb 20, 2025
Copy link
Member

@talboren talboren left a comment

Choose a reason for hiding this comment

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

@AkashJana18 can you add a few screenshots of some use cases and how it looks?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
🙋 Bounty claim Incident size:L This PR changes 100-499 lines, ignoring generated files.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[🐛 Bug]: Merge incidents not working in some cases
2 participants