Skip to content

[management] Prevent deletion of groups linked to flow groups#5439

Merged
bcmmbaga merged 2 commits intomainfrom
fix/prevent-flow-group-deletion
Feb 24, 2026
Merged

[management] Prevent deletion of groups linked to flow groups#5439
bcmmbaga merged 2 commits intomainfrom
fix/prevent-flow-group-deletion

Conversation

@bcmmbaga
Copy link
Copy Markdown
Contributor

@bcmmbaga bcmmbaga commented Feb 24, 2026

Describe your changes

Issue ticket number and link

Stack

Checklist

  • Is it a bug fix
  • Is a typo/documentation fix
  • Is a feature enhancement
  • It is a refactor
  • Created tests that fail without the change (if possible)

By submitting this pull request, you confirm that you have read and agree to the terms of the Contributor License Agreement.

Documentation

Select exactly one:

  • I added/updated documentation for this change
  • Documentation is not needed for this change (explain why)

Docs PR URL (required if "docs added" is checked)

Paste the PR link from https://github.com/netbirdio/docs here:

https://github.com/netbirdio/docs/pull/__

Summary by CodeRabbit

  • Bug Fixes

    • Groups linked to traffic event logging configurations are now protected from accidental deletion. Users attempting to delete such groups will receive a clear error message.
  • Tests

    • Added test coverage for group deletion protection mechanisms.

Signed-off-by: bcmmbaga <bethuelmbaga12@gmail.com>
Signed-off-by: bcmmbaga <bethuelmbaga12@gmail.com>
@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai Bot commented Feb 24, 2026

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

Configuration used: defaults

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between ef82905 and d375d54.

📒 Files selected for processing (2)
  • management/server/group.go
  • management/server/group_test.go

📝 Walkthrough

Walkthrough

This PR adds validation logic to prevent deletion of groups that are linked to flow groups (traffic event logging settings). The DeleteGroups function now retrieves extra account settings, extracts FlowGroups, and passes this information to validateDeleteGroup, which rejects deletion attempts for groups in the flowGroups list.

Changes

Cohort / File(s) Summary
Group Deletion Validation
management/server/group.go
Added retrieval of extra settings in DeleteGroups, extended validateDeleteGroup signature to accept flowGroups parameter, and introduced a guard condition to reject deletion if groupID exists in flowGroups (for traffic event logging dependencies).
Deletion Validation Test Coverage
management/server/group_test.go
Added comprehensive test TestDefaultAccountManager_DeleteGroupLinkedToFlowGroup that mocks the settings manager, verifies rejection of group deletion when linked to flow groups, and validates bulk deletion behavior with mixed group types.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~12 minutes

Possibly related PRs

Suggested reviewers

  • mlsmaycon

Poem

🐰 A group seeks deletion, but alas! ✨
Flow groups hold it fast,
A guard stands firm, validation's call,
Safeguards prevent the fall! 🛡️

🚥 Pre-merge checks | ✅ 1 | ❌ 2

❌ Failed checks (2 warnings)

Check name Status Explanation Resolution
Description check ⚠️ Warning The description follows the required template structure but lacks critical details: the 'Describe your changes' section is empty, and no issue ticket number/link is provided despite being a required field. Fill in the 'Describe your changes' section with a summary of the changes, and provide the issue ticket number and link in the designated field.
Docstring Coverage ⚠️ Warning Docstring coverage is 20.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (1 passed)
Check name Status Explanation
Title check ✅ Passed The title clearly and concisely describes the main change: preventing deletion of groups linked to flow groups, which matches the core functionality added in the changeset.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
  • 📝 Generate docstrings (stacked PR)
  • 📝 Generate docstrings (commit on current branch)
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch fix/prevent-flow-group-deletion

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.

@sonarqubecloud
Copy link
Copy Markdown

@bcmmbaga bcmmbaga merged commit afe6d9f into main Feb 24, 2026
46 of 47 checks passed
@bcmmbaga bcmmbaga deleted the fix/prevent-flow-group-deletion branch February 24, 2026 18:19
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