Skip to content

feat: add confirmation warning for category group unlinking impact#853

Open
romanetar wants to merge 1 commit intomasterfrom
feature/validate-category-removal-from-group
Open

feat: add confirmation warning for category group unlinking impact#853
romanetar wants to merge 1 commit intomasterfrom
feature/validate-category-removal-from-group

Conversation

@romanetar
Copy link
Copy Markdown

@romanetar romanetar commented Mar 31, 2026

ref https://app.clickup.com/t/86b8rp3vh

Summary by CodeRabbit

  • New Features

    • Added confirmation dialog when unlinking items to prevent accidental removals.
  • Localization

    • Added translated warning message for the confirmation prompt.

@romanetar romanetar requested a review from smarcet March 31, 2026 19:06
@coderabbitai
Copy link
Copy Markdown

coderabbitai bot commented Mar 31, 2026

📝 Walkthrough

Walkthrough

The pull request adds a confirmation dialog using SweetAlert2 to the track unlinking function, requiring user confirmation before removing a category from a group. A corresponding localized warning message is added to the English language file.

Changes

Cohort / File(s) Summary
Confirmation Dialog Addition
src/components/forms/event-category-group-form.js
Modified handleTrackUnLink to display a SweetAlert2 confirmation dialog with translated title and message before executing the unlink operation.
Localization String
src/i18n/en.json
Added unlink_track_warning key under edit_event_category_group to provide translated confirmation text for the unlinking warning.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~8 minutes

Suggested reviewers

  • smarcet

Poem

🐰 A gentle "Are you sure?" now guards the way,
Before a category slips away,
Sweet alerts and words so clear,
Keep accidents far from here! ✨

🚥 Pre-merge checks | ✅ 3
✅ Passed checks (3 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title accurately describes the main change: adding a confirmation warning dialog when unlinking category groups, which is the primary modification across the affected files.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.

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

✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch feature/validate-category-removal-from-group

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

Copy link
Copy Markdown

@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.

🧹 Nitpick comments (2)
src/components/forms/event-category-group-form.js (1)

100-106: Localize the modal cancel button text as well.

confirmButtonText is translated, but cancel falls back to SweetAlert’s default label. Adding cancelButtonText keeps the dialog fully i18n-consistent.

Suggested update
     Swal.fire({
       title: T.translate("general.are_you_sure"),
       text: T.translate("edit_event_category_group.unlink_track_warning"),
       type: "warning",
       showCancelButton: true,
-      confirmButtonText: T.translate("general.yes")
+      confirmButtonText: T.translate("general.yes"),
+      cancelButtonText: T.translate("general.cancel")
     }).then((result) => {
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@src/components/forms/event-category-group-form.js` around lines 100 - 106,
The Swal.fire call in the unlink confirmation modal (inside
event-category-group-form.js where Swal.fire is invoked) localizes
confirmButtonText but not the cancel button; update the Swal.fire options to
include cancelButtonText: T.translate("general.cancel") (or the appropriate i18n
key) and ensure showCancelButton: true is present so the cancel button uses the
translated label.
src/i18n/en.json (1)

1320-1320: Polish the warning copy for readability and consistency.

The new message works, but a small wording/capitalization tweak would read cleaner in the modal.

Suggested copy update
-    "unlink_track_warning": "if you remove a category from the category group, any activities linked to that category will automatically be removed from the selection plan. Do you want to continue?",
+    "unlink_track_warning": "If you remove a category from this category group, any activities linked to that category will be automatically removed from the selection plan. Do you want to continue?",
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@src/i18n/en.json` at line 1320, Update the i18n string keyed by
"unlink_track_warning" to improve capitalization and readability: change the
value to start with a capital "If" and simplify the middle clause to read like
"Any activities linked to that category will be removed from the selection
plan." so the full message becomes something like "If you remove a category from
the category group, any activities linked to that category will be removed from
the selection plan. Do you want to continue?" Replace the existing value for
"unlink_track_warning" in src/i18n/en.json accordingly.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Nitpick comments:
In `@src/components/forms/event-category-group-form.js`:
- Around line 100-106: The Swal.fire call in the unlink confirmation modal
(inside event-category-group-form.js where Swal.fire is invoked) localizes
confirmButtonText but not the cancel button; update the Swal.fire options to
include cancelButtonText: T.translate("general.cancel") (or the appropriate i18n
key) and ensure showCancelButton: true is present so the cancel button uses the
translated label.

In `@src/i18n/en.json`:
- Line 1320: Update the i18n string keyed by "unlink_track_warning" to improve
capitalization and readability: change the value to start with a capital "If"
and simplify the middle clause to read like "Any activities linked to that
category will be removed from the selection plan." so the full message becomes
something like "If you remove a category from the category group, any activities
linked to that category will be removed from the selection plan. Do you want to
continue?" Replace the existing value for "unlink_track_warning" in
src/i18n/en.json accordingly.

ℹ️ Review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: c5d27bdf-678e-471a-9deb-daa0de8eeb68

📥 Commits

Reviewing files that changed from the base of the PR and between f19785d and 80bc0b4.

📒 Files selected for processing (2)
  • src/components/forms/event-category-group-form.js
  • src/i18n/en.json

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.

1 participant