-
-
Notifications
You must be signed in to change notification settings - Fork 3.5k
Add fix options for integrity issues #12495
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from 57 commits
9b11edf
02d8e75
754b6ad
696f904
4ab36a8
28de796
70ae8eb
77df6ea
30c81ce
5b389da
3a0d9a1
a04e06a
8e9341a
235c74f
9839137
5754680
8f7bdc2
f57719c
2105a13
e37ba79
139777e
39dbfd1
16e9817
102e789
2d1a5a4
361765b
19a7881
6a5da7a
1bfd407
fd0045f
9dfed1a
7a45ddd
71efd96
409797f
cb0c1be
3e94343
30d4bbb
2990e3d
647cac8
225d235
0cb580f
4ea6831
2302dec
71a6510
a608a22
e52907f
eff137a
621ebaf
bcb079f
4861b01
e60595c
3cf54dd
a9dbf98
cafaf0c
e11499a
157dfe8
6f86c65
8502e76
a717a45
ee71b7f
b979659
0656364
1181dda
c9b3df2
7973711
31fecf8
9c2a7eb
fcdeea2
c60042f
1f08cfa
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,23 +1,36 @@ | ||
| package org.jabref.gui.integrity; | ||
|
|
||
| import java.util.Arrays; | ||
| import java.util.HashSet; | ||
| import java.util.List; | ||
| import java.util.Objects; | ||
| import java.util.Optional; | ||
| import java.util.Set; | ||
| import java.util.concurrent.atomic.AtomicBoolean; | ||
| import java.util.function.Function; | ||
|
|
||
| import javafx.application.Platform; | ||
| import javafx.beans.property.ReadOnlyStringWrapper; | ||
| import javafx.fxml.FXML; | ||
| import javafx.scene.control.Button; | ||
| import javafx.scene.control.ComboBox; | ||
| import javafx.scene.control.ContextMenu; | ||
| import javafx.scene.control.Label; | ||
| import javafx.scene.control.MenuButton; | ||
| import javafx.scene.control.TableCell; | ||
| import javafx.scene.control.TableColumn; | ||
| import javafx.scene.control.TableView; | ||
| import javafx.scene.input.MouseButton; | ||
| import javafx.scene.input.MouseEvent; | ||
| import javafx.stage.Modality; | ||
|
|
||
| import org.jabref.gui.DialogService; | ||
| import org.jabref.gui.LibraryTab; | ||
| import org.jabref.gui.theme.ThemeManager; | ||
| import org.jabref.gui.util.BaseDialog; | ||
| import org.jabref.gui.util.ValueTableCellFactory; | ||
| import org.jabref.gui.util.ViewModelTableRowFactory; | ||
| import org.jabref.logic.integrity.IntegrityIssue; | ||
| import org.jabref.logic.integrity.IntegrityMessage; | ||
| import org.jabref.logic.l10n.Localization; | ||
|
|
||
|
|
@@ -31,20 +44,30 @@ public class IntegrityCheckDialog extends BaseDialog<Void> { | |
| @FXML private TableColumn<IntegrityMessage, String> keyColumn; | ||
| @FXML private TableColumn<IntegrityMessage, String> fieldColumn; | ||
| @FXML private TableColumn<IntegrityMessage, String> messageColumn; | ||
| @FXML private TableColumn<IntegrityMessage, String> fixesColumn; | ||
| @FXML private MenuButton keyFilterButton; | ||
| @FXML private MenuButton fieldFilterButton; | ||
| @FXML private MenuButton messageFilterButton; | ||
| @FXML private ComboBox<String> entryTypeCombo; | ||
|
|
||
| @Inject private ThemeManager themeManager; | ||
|
|
||
| private final List<IntegrityMessage> messages; | ||
| private final LibraryTab libraryTab; | ||
| private final DialogService dialogService; | ||
|
|
||
| private IntegrityCheckDialogViewModel viewModel; | ||
| private TableFilter<IntegrityMessage> tableFilter; | ||
|
|
||
| public IntegrityCheckDialog(List<IntegrityMessage> messages, LibraryTab libraryTab) { | ||
| private final double FIX_BUTTON_HEIGHT = 20.0; | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The constant should be static as it doesn't depend on instance state and is used across multiple instances. This would improve memory usage and follow Java best practices. |
||
|
|
||
| public IntegrityCheckDialog(List<IntegrityMessage> messages, | ||
| LibraryTab libraryTab, | ||
| DialogService dialogService) { | ||
| this.messages = messages; | ||
| this.libraryTab = libraryTab; | ||
| this.dialogService = dialogService; | ||
|
|
||
| this.setTitle(Localization.lang("Check integrity")); | ||
| this.initModality(Modality.NONE); | ||
|
|
||
|
|
@@ -76,10 +99,55 @@ private void initialize() { | |
| .withOnMouseClickedEvent(this::handleRowClick) | ||
| .install(messagesTable); | ||
| messagesTable.setItems(viewModel.getMessages()); | ||
| updateEntryTypeCombo(); | ||
| keyColumn.setCellValueFactory(row -> new ReadOnlyStringWrapper(row.getValue().entry().getCitationKey().orElse(""))); | ||
| fieldColumn.setCellValueFactory(row -> new ReadOnlyStringWrapper(row.getValue().field().getDisplayName())); | ||
| messageColumn.setCellValueFactory(row -> new ReadOnlyStringWrapper(row.getValue().message())); | ||
|
|
||
| fixesColumn.setCellFactory(row -> new TableCell<>() { | ||
| private final Button button = new Button(); | ||
|
|
||
| @Override | ||
| protected void updateItem(String item, boolean empty) { | ||
| super.updateItem(item, empty); | ||
| if (empty || getTableRow() == null || getTableRow().getItem() == null) { | ||
| setGraphic(null); | ||
| return; | ||
| } | ||
|
calixtus marked this conversation as resolved.
|
||
| IntegrityMessage rowData = getTableRow().getItem(); | ||
| configureAction(rowData); | ||
| } | ||
|
|
||
| /** | ||
| * Configures the action for the button based on the provided {@link IntegrityMessage}. | ||
| * If a fix is available for the message, the button is configured to apply the fix when clicked. | ||
| * | ||
| * @param message the {@link IntegrityMessage} to check for available fixes | ||
| */ | ||
| private void configureAction(IntegrityMessage message) { | ||
| Optional<IntegrityIssue> issue = IntegrityIssue.fromMessage(message); | ||
| if (issue.isEmpty()) { | ||
| return; | ||
| } | ||
| if (issue.get().getFix().isEmpty()) { | ||
| setGraphic(new Label(Localization.lang("No fix available"))); | ||
| return; | ||
| } | ||
| configureButton(issue.get().getFix().toString(), () -> { | ||
| viewModel.fix(issue.get(), message); | ||
| viewModel.removeFromEntryTypes(message.field().getDisplayName()); | ||
| Platform.runLater(() -> viewModel.columnsListProperty().getValue().removeIf(column -> Objects.equals(column.message(), message.message()))); | ||
|
priyanshu16095 marked this conversation as resolved.
Outdated
|
||
| }); | ||
| setGraphic(button); | ||
| } | ||
|
|
||
| private void configureButton(String text, Runnable action) { | ||
| button.setText(text); | ||
| button.setPrefHeight(FIX_BUTTON_HEIGHT); | ||
| button.setOnAction(event -> action.run()); | ||
| } | ||
| }); | ||
|
|
||
| new ValueTableCellFactory<IntegrityMessage, String>() | ||
| .withText(Function.identity()) | ||
| .withTooltip(Function.identity()) | ||
|
|
@@ -91,6 +159,8 @@ private void initialize() { | |
| addMessageColumnFilter(keyColumn, keyFilterButton); | ||
| addMessageColumnFilter(fieldColumn, fieldFilterButton); | ||
| addMessageColumnFilter(messageColumn, messageFilterButton); | ||
|
|
||
| messagesTable.itemsProperty().bind(viewModel.columnsListProperty()); | ||
| } | ||
|
|
||
| private void addMessageColumnFilter(TableColumn<IntegrityMessage, String> messageColumn, MenuButton messageFilterButton) { | ||
|
|
@@ -121,4 +191,84 @@ public void clearFilters() { | |
| }); | ||
| } | ||
| } | ||
|
|
||
| private void updateEntryTypeCombo() { | ||
| Set<String> entryTypes = viewModel.getEntryTypes(); | ||
| Set<String> uniqueTexts = new HashSet<>(); | ||
| entryTypeCombo.getItems().clear(); | ||
|
|
||
| Arrays.stream(IntegrityIssue.values()) | ||
| .filter(issue -> issue.getFix().isPresent()) | ||
| .filter(issue -> entryTypes.contains(issue.getText())) | ||
| .filter(issue -> uniqueTexts.add(issue.getText())) | ||
| .forEach(issue -> entryTypeCombo.getItems().add(issue.getText())); | ||
|
|
||
| if (entryTypeCombo.getItems().isEmpty()) { | ||
| entryTypeCombo.getItems().add(Localization.lang("No fix available")); | ||
| } | ||
| entryTypeCombo.getSelectionModel().selectFirst(); | ||
| } | ||
|
|
||
| public void fix(IntegrityIssue issue, IntegrityMessage message) { | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Fix seems a bit slang. "resolveIssue" maybe? |
||
| viewModel.fix(issue, message); | ||
| viewModel.removeFromEntryTypes(message.field().getDisplayName()); | ||
| Platform.runLater(() -> viewModel.columnsListProperty().getValue().removeIf(column -> Objects.equals(column.message(), message.message()))); | ||
| } | ||
|
|
||
| /** | ||
| * Checks if a given {@link IntegrityMessage} has a fix available. | ||
| * | ||
| * @param message the {@link IntegrityMessage} to check | ||
| * @return {@code true} if a fix is available, {@code false} otherwise | ||
| */ | ||
| private boolean hasFix(IntegrityMessage message) { | ||
| return message != null && message.field() != null && IntegrityIssue.fromMessage(message) | ||
| .map(issue -> issue.getFix().isPresent()) | ||
| .orElse(false); | ||
| } | ||
|
|
||
| /** | ||
| * Attempts to fix all {@link IntegrityMessage} objects of the selected type. | ||
| * If fixes are available, they are applied, and the fixed messages are removed. | ||
| * A notification is shown to indicate success or failure. | ||
| */ | ||
| @FXML | ||
| private void fixByType() { | ||
| AtomicBoolean fixed = new AtomicBoolean(false); | ||
|
|
||
| String selectedType = entryTypeCombo.getSelectionModel().getSelectedItem(); | ||
| Optional<IntegrityIssue> selectedIssue = Arrays.stream(IntegrityIssue.values()) | ||
| .filter(issue -> issue.getText().equals(selectedType)) | ||
| .findFirst(); | ||
|
|
||
| selectedIssue.ifPresent(issue -> { | ||
| messagesTable.getItems().stream() | ||
| .filter(message -> message.message().equals(issue.getText()) && hasFix(message)) // Filter messages matching the selected issue type and have a fix | ||
| .forEach(message -> { | ||
| fix(issue, message); | ||
| fixed.set(true); | ||
| }); | ||
| }); | ||
|
|
||
| updateEntryTypeCombo(); | ||
|
|
||
| if (fixed.get()) { | ||
| dialogService.notify(Localization.lang("Fixed successfully.")); | ||
| } else { | ||
| dialogService.notify(Localization.lang("No fixes available.")); | ||
| } | ||
|
priyanshu16095 marked this conversation as resolved.
Outdated
|
||
| } | ||
|
|
||
| /** | ||
| * Attempts to fix all {@link IntegrityMessage} objects that have a fix available. | ||
| * Messages with applicable fixes are processed, and the corresponding UI elements are updated. | ||
| */ | ||
| @FXML | ||
| private void fixAll() { | ||
| messagesTable.getItems().stream() | ||
| .filter(this::hasFix) // Filter all messages that have a fix | ||
| .forEach(message -> IntegrityIssue.fromMessage(message).ifPresent(issue -> fix(issue, message))); | ||
|
|
||
| updateEntryTypeCombo(); | ||
| } | ||
| } | ||
Uh oh!
There was an error while loading. Please reload this page.