Skip to content

Fix cleanup and save actions not working#15115

Merged
subhramit merged 4 commits intomainfrom
cleanupFixes
Feb 15, 2026
Merged

Fix cleanup and save actions not working#15115
subhramit merged 4 commits intomainfrom
cleanupFixes

Conversation

@Siedlerchr
Copy link
Copy Markdown
Member

@Siedlerchr Siedlerchr commented Feb 14, 2026

Related issues and pull requests

Closes #15096
Closes #15112

PR Description

Make cleanup and save actions working again
Revert issues arising from the change in #14568

grafik

Steps to test

  1. Load chocolate Bib
  2. Quality cleanup
  3. Execute cleanup actions works

LIbrary save actions also works

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
Copy link
Copy Markdown
Contributor

Review Summary by Qodo

Fix cleanup and save actions functionality

🐞 Bug fix

Grey Divider

Walkthroughs

Description
• Fix cleanup and save actions not working after PR #14568
• Add checkbox to enable/disable field formatter cleanups
• Restore proper binding between cleanup enabled state and UI
• Fix boolean condition logic in cleanup initialization
Diagram
flowchart LR
  A["CleanupSingleFieldPanel"] -->|sets showCleanupEnabledButton| B["FieldFormatterCleanupsPanel"]
  B -->|manages enabled state| C["CheckBox cleanupsEnabled"]
  C -->|controls| D["Cleanup UI Elements"]
  E["SavingPropertiesView"] -->|binds cleanupsDisableProperty| F["FieldFormatterCleanupsPanel"]
  G["FieldFormatterCleanupsPanelViewModel"] -->|provides properties| B
Loading

Grey Divider

File Changes

1. jabgui/src/main/java/org/jabref/gui/cleanup/CleanupSingleFieldPanel.java 🐞 Bug fix +1/-0

Hide cleanup enabled button in cleanup panel

• Added call to setShowCleanupEnabledButton(false) to hide the cleanup enabled checkbox in cleanup
 dialog
• Ensures cleanup panel displays without the enable/disable button in this context

jabgui/src/main/java/org/jabref/gui/cleanup/CleanupSingleFieldPanel.java


2. jabgui/src/main/java/org/jabref/gui/cleanup/CleanupSingleFieldViewModel.java 🐞 Bug fix +1/-1

Fix cleanup enabled initialization logic

• Changed cleanupsEnabled initialization from initialCleanups.isEnabled() to hardcoded true
• Fixes boolean condition logic that was preventing cleanup actions from executing

jabgui/src/main/java/org/jabref/gui/cleanup/CleanupSingleFieldViewModel.java


3. jabgui/src/main/java/org/jabref/gui/commonfxcontrols/FieldFormatterCleanupsPanel.java ✨ Enhancement +21/-2

Add checkbox control for cleanup enabled state

• Added CheckBox cleanupsEnabled FXML field to manage cleanup enable/disable state
• Added bidirectional binding between checkbox and cleanupsDisableProperty in view model
• Added visibility and managed bindings for the checkbox based on cleanupEnabledManagedProperty
• Added public methods setShowCleanupEnabledButton() and cleanupsDisableProperty() for external
 control
• Removed Pos.CENTER alignment from action column cell factory
• Added imports for BooleanProperty, CheckBox, and BindingsHelper

jabgui/src/main/java/org/jabref/gui/commonfxcontrols/FieldFormatterCleanupsPanel.java


View more (3)
4. jabgui/src/main/java/org/jabref/gui/commonfxcontrols/FieldFormatterCleanupsPanelViewModel.java ✨ Enhancement +12/-0

Add properties for cleanup enabled state management

• Added cleanupEnabledManagedProperty to control visibility of the cleanup enabled checkbox
• Added cleanupsDisableProperty to track and manage the disabled state of cleanups
• Added getter methods for both new boolean properties

jabgui/src/main/java/org/jabref/gui/commonfxcontrols/FieldFormatterCleanupsPanelViewModel.java


5. jabgui/src/main/java/org/jabref/gui/libraryproperties/saving/SavingPropertiesView.java 🐞 Bug fix +1/-0

Bind cleanup disabled state in save properties

• Added bidirectional binding between fieldFormatterCleanupsPanel.cleanupsDisableProperty() and
 viewModel.cleanupsDisableProperty()
• Ensures save actions properly synchronize cleanup enabled state with view model

jabgui/src/main/java/org/jabref/gui/libraryproperties/saving/SavingPropertiesView.java


6. jabgui/src/main/resources/org/jabref/gui/commonfxcontrols/FieldFormatterCleanupsPanel.fxml ✨ Enhancement +7/-5

Add checkbox UI and disable bindings in FXML

• Added CheckBox with fx:id cleanupsEnabled and text "%Enable field formatters"
• Added disable binding to multiple UI elements based on checkbox selected state
• Applied disable binding to TableView, SearchableComboBox fields, and action buttons
• Ensures all cleanup controls are disabled when checkbox is unchecked

jabgui/src/main/resources/org/jabref/gui/commonfxcontrols/FieldFormatterCleanupsPanel.fxml


Grey Divider

Qodo Logo

@qodo-free-for-open-source-projects
Copy link
Copy Markdown
Contributor

Code Review by Qodo

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

Grey Divider


Action required

1. Save actions may still not apply 🐞 Bug ✓ Correctness
Description
Even with the new enable/disable UI binding, save actions are applied only when
MetaData.getSaveActions() is present; clearing/omitting save actions metadata results in no save
actions being run during save, which can still look like “save actions not working” if the UI shows
a default configuration.
Code

jabgui/src/main/java/org/jabref/gui/libraryproperties/saving/SavingPropertiesView.java[50]

+        fieldFormatterCleanupsPanel.cleanupsDisableProperty().bindBidirectional(viewModel.cleanupsDisableProperty());
Evidence
SavingPropertiesView now binds the panel’s disable flag to the view model, but save-time behavior
depends solely on whether save actions are present in metadata. BibDatabaseWriter applies save
actions only if metaData.getSaveActions() is present. Therefore, any code path that results in
save actions metadata being absent will cause no actions to be executed, regardless of what the
library properties UI displays as a “default” configuration.

jabgui/src/main/java/org/jabref/gui/libraryproperties/saving/SavingPropertiesView.java[38-52]
jablib/src/main/java/org/jabref/logic/exporter/BibDatabaseWriter.java[104-113]
jablib/src/main/java/org/jabref/model/metadata/MetaData.java[167-173]

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

### Issue description
Save actions are only executed during save when `MetaData.getSaveActions()` is present. If the library properties UI can represent “default” save actions without writing them into metadata (or clears them), users will still observe save actions not being applied.

### Issue Context
`SavingPropertiesView` now binds `cleanupsDisableProperty` to the panel, but `BibDatabaseWriter` executes save actions only if `MetaData.getSaveActions()` is present.

### Fix Focus Areas
- jabgui/src/main/java/org/jabref/gui/libraryproperties/saving/SavingPropertiesView.java[38-52]
- jablib/src/main/java/org/jabref/logic/exporter/BibDatabaseWriter.java[104-113]
- jablib/src/main/java/org/jabref/model/metadata/MetaData.java[167-173]

### Notes
Prefer fixing persistence: store save actions metadata whenever the UI indicates save actions should be active (enabled and/or non-empty list), so the save pipeline reliably applies them.

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


2. Cleanup forces enabled true 🐞 Bug ✓ Correctness
Description
The cleanup dialog’s formatter tab now hard-codes cleanupsEnabled=true, ignoring the user’s
persisted cleanup preference (default is disabled). Because the dialog persists updated formatter
cleanups back to preferences, running cleanup can unexpectedly flip the global cleanup setting to
enabled.
Code

jabgui/src/main/java/org/jabref/gui/cleanup/CleanupSingleFieldViewModel.java[R17-18]

+        cleanupsEnabled.set(true);
        cleanups.setAll(initialCleanups.getConfiguredActions());
Evidence
The constructor change hard-codes enabled=true. The cleanup dialog persists the formatter tab
selection back to preferences.getCleanupPreferences().setFieldFormatterCleanups(...). The default
preference for CLEANUP_FIELD_FORMATTERS_ENABLED is false, so this change can silently override a
user’s disabled preference after using the cleanup dialog.

jabgui/src/main/java/org/jabref/gui/cleanup/CleanupSingleFieldViewModel.java[12-23]
jabgui/src/main/java/org/jabref/gui/cleanup/CleanupDialogViewModel.java[119-131]
jablib/src/main/java/org/jabref/logic/preferences/JabRefCliPreferences.java[660-664]

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 cleanup dialog formatter tab forces `enabled=true` and then persists that selection back to global cleanup preferences. This can unexpectedly override a user’s preference.

### Issue Context
Defaults set `CLEANUP_FIELD_FORMATTERS_ENABLED` to false. The dialog writes formatter settings back to preferences when applying a formatter tab selection.

### Fix Focus Areas
- jabgui/src/main/java/org/jabref/gui/cleanup/CleanupSingleFieldViewModel.java[12-23]
- jabgui/src/main/java/org/jabref/gui/cleanup/CleanupDialogViewModel.java[119-131]
- jablib/src/main/java/org/jabref/logic/preferences/JabRefCliPreferences.java[660-664]

### Suggested directions
- Either restore `cleanupsEnabled.set(initialCleanups.isEnabled())` and expose a user-controlled toggle, or
- Keep formatters always enabled for the one-off cleanup run, but do **not** persist the forced enabled flag back into preferences.

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



Remediation recommended

3. setShowCleanupEnabledButton(Boolean) param 📘 Rule violation ⛯ Reliability
Description
setShowCleanupEnabledButton is a public API that takes a boxed boolean (Boolean), allowing null,
and uses a boolean switch parameter, reducing clarity and increasing null-related risk. This
violates JabRef guidance to avoid boolean public params and to avoid null-prone APIs in new/changed
code.
Code

jabgui/src/main/java/org/jabref/gui/commonfxcontrols/FieldFormatterCleanupsPanel.java[R146-152]

+    public void setShowCleanupEnabledButton(Boolean enable) {
+        viewModel.cleanupEnabledManagedProperty().setValue(enable);
+    }
+
+    public BooleanProperty cleanupsDisableProperty() {
+        return viewModel.cleanupsDisableProperty();
+    }
Evidence
The checklist discourages public methods that accept boolean switch parameters and prefers null-safe
contracts (avoid passing/returning null). The changed code introduces a public method with a boxed
Boolean parameter (setShowCleanupEnabledButton(Boolean enable)), which is nullable and also acts
as a boolean switch.

AGENTS.md
AGENTS.md
jabgui/src/main/java/org/jabref/gui/commonfxcontrols/FieldFormatterCleanupsPanel.java[146-152]

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

## Issue description
`FieldFormatterCleanupsPanel#setShowCleanupEnabledButton(Boolean enable)` is public, takes a nullable boxed `Boolean`, and uses a boolean switch parameter, which is discouraged by JabRef rules and increases null-related risk.

## Issue Context
This method is invoked by callers (e.g., panels embedding `FieldFormatterCleanupsPanel`) to control whether the enable/disable checkbox is shown.

## Fix Focus Areas
- jabgui/src/main/java/org/jabref/gui/commonfxcontrols/FieldFormatterCleanupsPanel.java[146-152]
- jabgui/src/main/java/org/jabref/gui/cleanup/CleanupSingleFieldPanel.java[33-33]

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


4. Clear-all ignores disabled state 🐞 Bug ✓ Correctness
Description
Most controls are disabled when the new "Enable field formatters" checkbox is unchecked, but the
"clear all" button remains enabled, allowing list mutation while the feature is shown as disabled.
Code

↗ jabgui/src/main/resources/org/jabref/gui/commonfxcontrols/FieldFormatterCleanupsPanel.fxml

                </tooltip>
Evidence
The FXML disables the table, add controls, and other buttons based on cleanupsEnabled.selected,
but the clear-all button lacks the same disable binding, creating inconsistent behavior.

jabgui/src/main/resources/org/jabref/gui/commonfxcontrols/FieldFormatterCleanupsPanel.fxml[16-61]

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 clear-all button stays enabled even when the feature is disabled via the new checkbox, enabling unexpected mutations.

### Issue Context
Other controls already use `disable=&quot;${!cleanupsEnabled.selected}&quot;`.

### Fix Focus Areas
- jabgui/src/main/resources/org/jabref/gui/commonfxcontrols/FieldFormatterCleanupsPanel.fxml[49-60]

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


Grey Divider

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

Grey Divider

Qodo Logo

saveOrderConfigPanel.sortableFieldsProperty().bind(viewModel.sortableFieldsProperty());
saveOrderConfigPanel.sortCriteriaProperty().bindBidirectional(viewModel.sortCriteriaProperty());

fieldFormatterCleanupsPanel.cleanupsDisableProperty().bindBidirectional(viewModel.cleanupsDisableProperty());
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Action required

1. Save actions may still not apply 🐞 Bug ✓ Correctness

Even with the new enable/disable UI binding, save actions are applied only when
MetaData.getSaveActions() is present; clearing/omitting save actions metadata results in no save
actions being run during save, which can still look like “save actions not working” if the UI shows
a default configuration.
Agent Prompt
### Issue description
Save actions are only executed during save when `MetaData.getSaveActions()` is present. If the library properties UI can represent “default” save actions without writing them into metadata (or clears them), users will still observe save actions not being applied.

### Issue Context
`SavingPropertiesView` now binds `cleanupsDisableProperty` to the panel, but `BibDatabaseWriter` executes save actions only if `MetaData.getSaveActions()` is present.

### Fix Focus Areas
- jabgui/src/main/java/org/jabref/gui/libraryproperties/saving/SavingPropertiesView.java[38-52]
- jablib/src/main/java/org/jabref/logic/exporter/BibDatabaseWriter.java[104-113]
- jablib/src/main/java/org/jabref/model/metadata/MetaData.java[167-173]

### Notes
Prefer fixing persistence: store save actions metadata whenever the UI indicates save actions should be active (enabled and/or non-empty list), so the save pipeline reliably applies them.

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

Comment on lines +17 to 18
cleanupsEnabled.set(true);
cleanups.setAll(initialCleanups.getConfiguredActions());
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Action required

2. Cleanup forces enabled true 🐞 Bug ✓ Correctness

The cleanup dialog’s formatter tab now hard-codes cleanupsEnabled=true, ignoring the user’s
persisted cleanup preference (default is disabled). Because the dialog persists updated formatter
cleanups back to preferences, running cleanup can unexpectedly flip the global cleanup setting to
enabled.
Agent Prompt
### Issue description
The cleanup dialog formatter tab forces `enabled=true` and then persists that selection back to global cleanup preferences. This can unexpectedly override a user’s preference.

### Issue Context
Defaults set `CLEANUP_FIELD_FORMATTERS_ENABLED` to false. The dialog writes formatter settings back to preferences when applying a formatter tab selection.

### Fix Focus Areas
- jabgui/src/main/java/org/jabref/gui/cleanup/CleanupSingleFieldViewModel.java[12-23]
- jabgui/src/main/java/org/jabref/gui/cleanup/CleanupDialogViewModel.java[119-131]
- jablib/src/main/java/org/jabref/logic/preferences/JabRefCliPreferences.java[660-664]

### Suggested directions
- Either restore `cleanupsEnabled.set(initialCleanups.isEnabled())` and expose a user-controlled toggle, or
- Keep formatters always enabled for the one-off cleanup run, but do **not** persist the forced enabled flag back into preferences.

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

@testlens-app
Copy link
Copy Markdown

testlens-app Bot commented Feb 14, 2026

✅ All tests passed ✅

🏷️ Commit: dd680d8
▶️ Tests: 11192 executed
⚪️ Checks: 52/52 completed


Learn more about TestLens at testlens.app.

@subhramit subhramit added this pull request to the merge queue Feb 15, 2026
@github-actions github-actions Bot added the status: to-be-merged PRs which are accepted and should go into the merge-queue. label Feb 15, 2026
Merged via the queue into main with commit bae2346 Feb 15, 2026
53 checks passed
@subhramit subhramit deleted the cleanupFixes branch February 15, 2026 08:25
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

status: to-be-merged PRs which are accepted and should go into the merge-queue.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

can't globally shorten DOIs Library save actions and cleanup single field does not work

2 participants