Skip to content

Commit c0642a7

Browse files
alexsukhinsubhramitkopporSiedlerchr
authored
Add tabs to Clean Up Entries dialog (#13852)
* Add cleanup dialog tabs with individual tab preferences * Fixed indentation and added commenting * Fix Trag-bot review issues - Removed trivial comments - Renamed PDF-related variables - Updated methods to return Optional - Used Optional property for FieldFormatterCleanups * Fix Trag-bot review issues - Removed trivial comments - Fixed CHANGELOG.md * Fix Trag-bot review issues - Removed trivial comments - Improved Optional checks - Perform null check for other parameters * Avoid nested Optionals, returning Optional<CleanupPreferences> directly * Refactor CleanupPreferences by keeping one assertion per test * Converted tests to assertEquals * Maintain consistent naming conventions * Returns CleanupPreferences directly since value is always present * Initial review refactor draft - Create new ViewModel, pull logic from Action and SingleAction into ViewModel - Move Apply button to each tab - Remove categories from ENUM and keep enums of all jobs in each respective tab to be used for cleanup * fix import error! * Reformat codebase (more carefully) (#13885) * Fix non record comments by carl # Conflicts: # jabgui/src/main/java/org/jabref/gui/edit/automaticfiededitor/MoveFieldValueAction.java # jabgui/src/main/java/org/jabref/gui/mergeentries/threewaymerge/cell/sidebuttons/ToggleMergeUnmergeButton.java # jabgui/src/main/java/org/jabref/gui/mergeentries/threewaymerge/fieldsmerger/CommentMerger.java # jabgui/src/main/java/org/jabref/gui/mergeentries/threewaymerge/fieldsmerger/FieldMerger.java # jabgui/src/main/java/org/jabref/gui/mergeentries/threewaymerge/fieldsmerger/FileMerger.java # jabgui/src/main/java/org/jabref/gui/mergeentries/threewaymerge/fieldsmerger/GroupMerger.java # jabgui/src/main/java/org/jabref/gui/mergeentries/threewaymerge/fieldsmerger/KeywordMerger.java # jablib/src/main/java/org/jabref/logic/layout/format/HTMLChars.java # jablib/src/main/java/org/jabref/model/entry/identifier/ArXivIdentifier.java # jablib/src/main/java/org/jabref/model/entry/identifier/EprintIdentifier.java * Add file exceptions * Remove shebang line * Remove shebang line * Remove shebang line * Expand variables & rename class --------- Co-authored-by: Oliver Kopp <[email protected]> * fix import error & merge * Apply OpenRewrite Cleanup * Refactor Cleanup Tabs - Moved cleanup panel logic into CleanupDialogViewModel for better separation of UI and logic - Changed tabSupplier and taskExecutor from Optional to nullable parameters - Moved updateWith logic into the record for cleaner preference updates. - General design improvements: more maintainable. * Fix getDisplayName method * Fix formatting * Trag-bot review and fix en properties * fix indentation plssss * format properly and change to observablelist * fix formatting entriestoprocess (please) * Updated names and changed optional dependencies back to nullable * Refactored panels to use separate ViewModels - Introduced ViewModels to encapsulate state and logic for panels. - Replaced direct UI manipulation with bidirectional bindings. - Ensures cleaner UI logic, easier maintenance * Moved ALL_JOBS to respective ViewModels, small naming changes * Replaced requireNotNull to @NotNull following #13957 * Address review feedback in CleanupDialogViewModel - Remove redundant comments following self-explanatory code - Add modifiedEntriesCount > 0 condition - Use "entry(s)" localization form for clean up message --------- Co-authored-by: Subhramit Basu <[email protected]> Co-authored-by: Oliver Kopp <[email protected]> Co-authored-by: Christoph <[email protected]>
1 parent 6d23890 commit c0642a7

20 files changed

+890
-467
lines changed

CHANGELOG.md

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,7 @@ Note that this project **does not** adhere to [Semantic Versioning](https://semv
1717

1818
### Changed
1919

20+
- We separated the "Clean up entries" dialog into three tabs for clarity [#13819](https://github.com/JabRef/jabref/issues/13819)
2021
- `JabKit`: `--porcelain` does not output any logs to the console anymore. [#14244](https://github.com/JabRef/jabref/pull/14244)
2122
- <kbd>Ctrl</kbd> + <kbd>Shift</kbd> + <kbd>L</kbd> now opens the terminal in the active library directory. [#14130](https://github.com/JabRef/jabref/issues/14130)
2223

Lines changed: 7 additions & 131 deletions
Original file line numberDiff line numberDiff line change
@@ -1,32 +1,16 @@
11
package org.jabref.gui.cleanup;
22

3-
import java.util.ArrayList;
4-
import java.util.List;
5-
import java.util.Optional;
63
import java.util.function.Supplier;
7-
import java.util.stream.Collectors;
84

95
import javax.swing.undo.UndoManager;
106

11-
import javafx.application.Platform;
12-
137
import org.jabref.gui.DialogService;
148
import org.jabref.gui.LibraryTab;
159
import org.jabref.gui.StateManager;
1610
import org.jabref.gui.actions.ActionHelper;
1711
import org.jabref.gui.actions.SimpleCommand;
18-
import org.jabref.gui.undo.NamedCompoundEdit;
19-
import org.jabref.gui.undo.UndoableFieldChange;
20-
import org.jabref.logic.JabRefException;
21-
import org.jabref.logic.cleanup.CleanupPreferences;
22-
import org.jabref.logic.cleanup.CleanupWorker;
23-
import org.jabref.logic.l10n.Localization;
2412
import org.jabref.logic.preferences.CliPreferences;
25-
import org.jabref.logic.util.BackgroundTask;
2613
import org.jabref.logic.util.TaskExecutor;
27-
import org.jabref.model.FieldChange;
28-
import org.jabref.model.database.BibDatabaseContext;
29-
import org.jabref.model.entry.BibEntry;
3014

3115
public class CleanupAction extends SimpleCommand {
3216

@@ -36,10 +20,6 @@ public class CleanupAction extends SimpleCommand {
3620
private final StateManager stateManager;
3721
private final TaskExecutor taskExecutor;
3822
private final UndoManager undoManager;
39-
private final List<JabRefException> failures;
40-
41-
private boolean isCanceled;
42-
private int modifiedEntriesCount;
4323

4424
public CleanupAction(Supplier<LibraryTab> tabSupplier,
4525
CliPreferences preferences,
@@ -53,7 +33,6 @@ public CleanupAction(Supplier<LibraryTab> tabSupplier,
5333
this.stateManager = stateManager;
5434
this.taskExecutor = taskExecutor;
5535
this.undoManager = undoManager;
56-
this.failures = new ArrayList<>();
5736

5837
this.executable.bind(ActionHelper.needsEntriesSelected(stateManager));
5938
}
@@ -64,119 +43,16 @@ public void execute() {
6443
return;
6544
}
6645

67-
if (stateManager.getSelectedEntries().isEmpty()) { // None selected. Inform the user to select entries first.
68-
dialogService.showInformationDialogAndWait(Localization.lang("Cleanup entry"), Localization.lang("First select entries to clean up."));
69-
return;
70-
}
71-
72-
isCanceled = false;
73-
modifiedEntriesCount = 0;
74-
7546
CleanupDialog cleanupDialog = new CleanupDialog(
7647
stateManager.getActiveDatabase().get(),
77-
preferences.getCleanupPreferences(),
78-
preferences.getFilePreferences()
79-
);
80-
81-
Optional<CleanupPreferences> chosenPreset = dialogService.showCustomDialogAndWait(cleanupDialog);
82-
83-
chosenPreset.ifPresent(preset -> {
84-
if (preset.isActive(CleanupPreferences.CleanupStep.RENAME_PDF) && preferences.getAutoLinkPreferences().shouldAskAutoNamingPdfs()) {
85-
boolean confirmed = dialogService.showConfirmationDialogWithOptOutAndWait(Localization.lang("Autogenerate PDF Names"),
86-
Localization.lang("Auto-generating PDF-Names does not support undo. Continue?"),
87-
Localization.lang("Autogenerate PDF Names"),
88-
Localization.lang("Cancel"),
89-
Localization.lang("Do not ask again"),
90-
optOut -> preferences.getAutoLinkPreferences().setAskAutoNamingPdfs(!optOut));
91-
if (!confirmed) {
92-
isCanceled = true;
93-
return;
94-
}
95-
}
96-
97-
preferences.getCleanupPreferences().setActiveJobs(preset.getActiveJobs());
98-
preferences.getCleanupPreferences().setFieldFormatterCleanups(preset.getFieldFormatterCleanups());
99-
100-
BackgroundTask.wrap(() -> cleanup(stateManager.getActiveDatabase().get(), preset))
101-
.onSuccess(result -> showResults())
102-
.onFailure(dialogService::showErrorDialogAndWait)
103-
.executeWith(taskExecutor);
104-
});
105-
}
106-
107-
/**
108-
* Runs the cleanup on the entry and records the change.
109-
*
110-
* @return true iff entry was modified
111-
*/
112-
private boolean doCleanup(BibDatabaseContext databaseContext, CleanupPreferences preset, BibEntry entry, NamedCompoundEdit compoundEdit) {
113-
// Create and run cleaner
114-
CleanupWorker cleaner = new CleanupWorker(
115-
databaseContext,
116-
preferences.getFilePreferences(),
117-
preferences.getTimestampPreferences()
48+
preferences,
49+
dialogService,
50+
stateManager,
51+
undoManager,
52+
tabSupplier,
53+
taskExecutor
11854
);
11955

120-
List<FieldChange> changes = cleaner.cleanup(preset, entry);
121-
122-
// Register undo action
123-
for (FieldChange change : changes) {
124-
compoundEdit.addEdit(new UndoableFieldChange(change));
125-
}
126-
127-
failures.addAll(cleaner.getFailures());
128-
129-
return !changes.isEmpty();
130-
}
131-
132-
private void showResults() {
133-
if (isCanceled) {
134-
return;
135-
}
136-
137-
if (modifiedEntriesCount > 0) {
138-
tabSupplier.get().markBaseChanged();
139-
}
140-
141-
if (modifiedEntriesCount == 0) {
142-
dialogService.notify(Localization.lang("No entry needed a clean up"));
143-
} else if (modifiedEntriesCount == 1) {
144-
dialogService.notify(Localization.lang("One entry needed a clean up"));
145-
} else {
146-
dialogService.notify(Localization.lang("%0 entries needed a clean up", Integer.toString(modifiedEntriesCount)));
147-
}
148-
}
149-
150-
private void cleanup(BibDatabaseContext databaseContext, CleanupPreferences cleanupPreferences) {
151-
this.failures.clear();
152-
153-
// undo granularity is on set of all entries
154-
NamedCompoundEdit compoundEdit = new NamedCompoundEdit(Localization.lang("Clean up entries"));
155-
156-
for (BibEntry entry : List.copyOf(stateManager.getSelectedEntries())) {
157-
if (doCleanup(databaseContext, cleanupPreferences, entry, compoundEdit)) {
158-
modifiedEntriesCount++;
159-
}
160-
}
161-
162-
compoundEdit.end();
163-
164-
if (compoundEdit.hasEdits()) {
165-
undoManager.addEdit(compoundEdit);
166-
}
167-
168-
if (!failures.isEmpty()) {
169-
showFailures(failures);
170-
}
171-
}
172-
173-
private void showFailures(List<JabRefException> failures) {
174-
String message = failures.stream()
175-
.map(exception -> "- " + exception.getLocalizedMessage())
176-
.collect(Collectors.joining("\n"));
177-
178-
Platform.runLater(() ->
179-
dialogService.showErrorDialogAndWait(Localization.lang("File Move Errors"), message)
180-
);
56+
dialogService.showCustomDialogAndWait(cleanupDialog);
18157
}
18258
}
Lines changed: 75 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -1,35 +1,87 @@
11
package org.jabref.gui.cleanup;
22

3-
import javafx.scene.control.ButtonType;
4-
import javafx.scene.control.ScrollPane;
3+
import java.util.List;
4+
import java.util.function.Supplier;
55

6+
import javax.swing.undo.UndoManager;
7+
8+
import javafx.fxml.FXML;
9+
import javafx.scene.control.Tab;
10+
import javafx.scene.control.TabPane;
11+
12+
import org.jabref.gui.DialogService;
13+
import org.jabref.gui.LibraryTab;
14+
import org.jabref.gui.StateManager;
615
import org.jabref.gui.util.BaseDialog;
716
import org.jabref.logic.FilePreferences;
817
import org.jabref.logic.cleanup.CleanupPreferences;
918
import org.jabref.logic.l10n.Localization;
19+
import org.jabref.logic.preferences.CliPreferences;
20+
import org.jabref.logic.util.TaskExecutor;
1021
import org.jabref.model.database.BibDatabaseContext;
22+
import org.jabref.model.entry.BibEntry;
23+
24+
import com.airhacks.afterburner.views.ViewLoader;
25+
26+
public class CleanupDialog extends BaseDialog<Void> {
27+
28+
@FXML private TabPane tabPane;
29+
30+
private final CleanupDialogViewModel viewModel;
31+
32+
// Constructor for multiple-entry cleanup
33+
public CleanupDialog(BibDatabaseContext databaseContext,
34+
CliPreferences preferences,
35+
DialogService dialogService,
36+
StateManager stateManager,
37+
UndoManager undoManager,
38+
Supplier<LibraryTab> tabSupplier,
39+
TaskExecutor taskExecutor) {
40+
41+
this.viewModel = new CleanupDialogViewModel(
42+
databaseContext, preferences, dialogService,
43+
stateManager, undoManager, tabSupplier, taskExecutor
44+
);
45+
46+
init(databaseContext, preferences);
47+
}
1148

12-
public class CleanupDialog extends BaseDialog<CleanupPreferences> {
13-
public CleanupDialog(BibDatabaseContext databaseContext, CleanupPreferences initialPreset, FilePreferences filePreferences) {
49+
// Constructor for single-entry cleanup
50+
public CleanupDialog(BibEntry targetEntry,
51+
BibDatabaseContext databaseContext,
52+
CliPreferences preferences,
53+
DialogService dialogService,
54+
StateManager stateManager,
55+
UndoManager undoManager) {
56+
57+
this.viewModel = new CleanupDialogViewModel(
58+
databaseContext, preferences, dialogService,
59+
stateManager, undoManager, null, null
60+
);
61+
62+
viewModel.setTargetEntries(List.of(targetEntry));
63+
64+
init(databaseContext, preferences);
65+
}
66+
67+
private void init(BibDatabaseContext databaseContext, CliPreferences preferences) {
1468
setTitle(Localization.lang("Clean up entries"));
15-
getDialogPane().setPrefSize(600, 650);
16-
getDialogPane().getButtonTypes().setAll(ButtonType.OK, ButtonType.CANCEL);
17-
18-
CleanupPresetPanel presetPanel = new CleanupPresetPanel(databaseContext, initialPreset, filePreferences);
19-
20-
// placing the content of the presetPanel in a scroll pane
21-
ScrollPane scrollPane = new ScrollPane();
22-
scrollPane.setFitToWidth(true);
23-
scrollPane.setFitToHeight(true);
24-
scrollPane.setContent(presetPanel);
25-
26-
getDialogPane().setContent(scrollPane);
27-
setResultConverter(button -> {
28-
if (button == ButtonType.OK) {
29-
return presetPanel.getCleanupPreset();
30-
} else {
31-
return null;
32-
}
33-
});
69+
70+
ViewLoader.view(this)
71+
.load()
72+
.setAsDialogPane(this);
73+
74+
CleanupPreferences initialPreset = preferences.getCleanupPreferences();
75+
FilePreferences filePreferences = preferences.getFilePreferences();
76+
77+
CleanupSingleFieldPanel singleFieldPanel = new CleanupSingleFieldPanel(initialPreset, viewModel);
78+
CleanupFileRelatedPanel fileRelatedPanel = new CleanupFileRelatedPanel(databaseContext, initialPreset, filePreferences, viewModel);
79+
CleanupMultiFieldPanel multiFieldPanel = new CleanupMultiFieldPanel(initialPreset, viewModel);
80+
81+
tabPane.getTabs().setAll(
82+
new Tab(Localization.lang("Single field"), singleFieldPanel),
83+
new Tab(Localization.lang("File-related"), fileRelatedPanel),
84+
new Tab(Localization.lang("Multi-field"), multiFieldPanel)
85+
);
3486
}
3587
}

0 commit comments

Comments
 (0)