Fix: exception dialog shows up when moving sidepanel down/up

[pluto-han]

Related issues and pull requests

Closes #15207

PR Description

The default constructor SidePanePreferences() used Map.of() which creates immutable map. Add or modify will throw an exception. I copied the map into mutable EnumMap.

Required verification screenshot:

Steps to test

1. Tick Tool -> Web search & groups. Now sidepane has two views.

2. Click either move panel down from "Web search" or move panel up from "Groups"

3. Works good, no exception dialog shows up.

Checklist

• I own the copyright of the code submitted and I license it under the MIT license
• I manually tested my changes in running JabRef (always required)
• [/] I added JUnit tests for changes (if applicable)
• I added screenshots in the PR description (if change is visible to the user)
• I added a screenshot in the PR description showing a library with a single entry with me as author and as title the issue number
• I described the change in CHANGELOG.md in a way that can be understood by the average user (if change is visible to the user)
• I checked the user documentation for up to dateness and submitted a pull request to our user documentation repository

[qodo-free-for-open-source-projects]

Review Summary by Qodo

Fix exception dialog when moving side panels

🐞 Bug fix

Walkthroughs

Description

• Fixed exception when moving side panels up/down by using mutable collections
• Replaced immutable Map.of() with mutable EnumMap in constructor
• Converted input collections to mutable EnumSet and EnumMap before wrapping
• Updated CHANGELOG.md with fix description

Diagram

flowchart LR
  A["Immutable Map/Set<br/>from Map.of()"] -->|Issue: Cannot modify| B["Exception thrown<br/>on add/modify"]
  C["Mutable EnumMap<br/>and EnumSet"] -->|Solution: Copy to mutable| D["FXCollections.observableMap<br/>observableSet"]
  D -->|Result: Works correctly| E["Side panels move<br/>without errors"]

Loading

File Changes

1. jabgui/src/main/java/org/jabref/gui/frame/SidePanePreferences.java 🐞 Bug fix +8/-2

Convert immutable collections to mutable before wrapping

• Added import for EnumMap utility class
• Modified constructor to convert input collections to mutable EnumSet and EnumMap before
 wrapping with FXCollections
• Ensures observable collections wrap mutable underlying collections instead of immutable ones

jabgui/src/main/java/org/jabref/gui/frame/SidePanePreferences.java

---

2. CHANGELOG.md 📝 Documentation +1/-0

Document side panel movement exception fix

• Added entry documenting the fix for exception dialog when moving side panels
• References issue #15207

CHANGELOG.md

---

[qodo-free-for-open-source-projects]

Code Review by Qodo

🐞 Bugs (1) 📘 Rule violations (1) 📎 Requirement gaps (0)

1. Awkward CHANGELOG wording ☑ 📘 Rule violation ✓ Correctness

Description

The new CHANGELOG entry contains grammatically incorrect/awkward phrasing (will show up) and
imprecise wording (side panels down/up) in user-facing text. This reduces clarity and
professionalism of release notes.

Code

CHANGELOG.md[29]

+- We fixed an issue where exception dialog will show up when moving side panels down/up. [#15207](https://github.com/JabRef/jabref/issues/15207)

Evidence

PR Compliance ID 33 requires user-facing text to be typo-free and grammatically correct; the added
CHANGELOG bullet contains awkward grammar and unclear phrasing.

CHANGELOG.md[29-29]
Best Practice: Learned patterns

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
The CHANGELOG entry added in this PR is user-facing but has awkward/incorrect grammar (e.g., `will show up`) and imprecise phrasing (`down/up`, `side panels`).
## Issue Context
Compliance requires user-facing text (including CHANGELOG entries) to be typo-free, grammatically correct, and precise.
## Fix Focus Areas
- CHANGELOG.md[29-29]

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

---

2. No regression test added 🐞 Bug ⛯ Reliability

Description

This change fixes a crash path caused by default preferences using an immutable Map.of(), but
there is no unit test ensuring SidePanePreferences.getDefault().getPreferredPositions() remains
mutable and pane reordering doesn’t throw. Without coverage, a future refactor could reintroduce the
immutable backing map and bring back the exception dialog.

Code

jabgui/src/main/java/org/jabref/gui/frame/SidePanePreferences.java[R24-30]

+        EnumSet<SidePaneType> mutableVisiblePanes = EnumSet.noneOf(SidePaneType.class);
+        mutableVisiblePanes.addAll(visiblePanes);
+        EnumMap<SidePaneType, Integer> mutablePreferredPositions = new EnumMap<>(SidePaneType.class);
+        mutablePreferredPositions.putAll(preferredPositions);
+
+        this.visiblePanes = FXCollections.observableSet(mutableVisiblePanes);
+        this.preferredPositions = FXCollections.observableMap(mutablePreferredPositions);

Evidence

The default preferences constructor passes Map.of() for preferred positions, and the side pane
move logic persists new ordering via setPreferredPositions, which performs clear()/putAll()
mutations. If the observable map wraps an immutable backing map (the original bug), these mutations
throw; current tests construct SidePanePreferences with mutable HashMap/HashSet and therefore
don’t cover the default Map.of() scenario.

jabgui/src/main/java/org/jabref/gui/frame/SidePanePreferences.java[34-40]
jabgui/src/main/java/org/jabref/gui/sidepane/SidePaneViewModel.java[114-119]
jabgui/src/main/java/org/jabref/gui/frame/SidePanePreferences.java[59-62]
jabgui/src/test/java/org/jabref/gui/sidepane/SidePaneViewModelTest.java[53-67]

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
The fix prevents exceptions caused by wrapping an immutable `Map.of()` inside a JavaFX `ObservableMap`, but there is no regression test ensuring this default-preferences path stays safe.
## Issue Context
- `SidePanePreferences.getDefault()` still passes `Map.of()`.
- Reordering panes triggers `SidePaneViewModel.updatePreferredPositions()` which calls `SidePanePreferences.setPreferredPositions()`, performing `clear()`/`putAll()` on the observable map.
- A test should fail if the backing map becomes immutable again.
## Fix Focus Areas
- jabgui/src/main/java/org/jabref/gui/frame/SidePanePreferences.java[21-40]
- jabgui/src/main/java/org/jabref/gui/frame/SidePanePreferences.java[59-62]
- jabgui/src/main/java/org/jabref/gui/sidepane/SidePaneViewModel.java[114-149]
- jabgui/src/test/java/org/jabref/gui/sidepane/SidePaneViewModelTest.java[53-113]
## Suggested test approach
1. Create `SidePanePreferences prefs = SidePanePreferences.getDefault();`
2. Wire it into a `SidePaneViewModel` (similar to `SidePaneViewModelTest`) and set up at least two panes.
3. Call `moveUp`/`moveDown` and assert:
- no exception is thrown
- `prefs.getPreferredPositions()` contains updated indices
Alternatively, a smaller unit test can assert mutability directly:
- `SidePanePreferences.getDefault().getPreferredPositions().put(SidePaneType.GROUPS, 0)` does not throw.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

---

ⓘ The new review experience is currently in Beta. Learn more

[testlens-app]

✅ All tests passed ✅

🏷️ Commit: 7b927fe
▶️ Tests: 11252 executed
⚪️ Checks: 51/51 completed

---

Learn more about TestLens at testlens.app.