-
-
Notifications
You must be signed in to change notification settings - Fork 2.8k
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
base: main
Are you sure you want to change the base?
Changes from 68 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,12 +1,23 @@ | ||
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; | ||
|
@@ -18,8 +29,10 @@ | |
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; | ||
import org.jabref.logic.util.NotificationService; | ||
|
||
import com.airhacks.afterburner.views.ViewLoader; | ||
import jakarta.inject.Inject; | ||
|
@@ -31,20 +44,28 @@ 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; | ||
@Inject private NotificationService notificationService; | ||
|
||
private final List<IntegrityMessage> messages; | ||
private final LibraryTab libraryTab; | ||
|
||
private IntegrityCheckDialogViewModel viewModel; | ||
private TableFilter<IntegrityMessage> tableFilter; | ||
|
||
public IntegrityCheckDialog(List<IntegrityMessage> messages, LibraryTab libraryTab) { | ||
private final double FIX_BUTTON_HEIGHT = 20.0; | ||
|
||
public IntegrityCheckDialog(List<IntegrityMessage> messages, | ||
LibraryTab libraryTab) { | ||
this.messages = messages; | ||
this.libraryTab = libraryTab; | ||
|
||
this.setTitle(Localization.lang("Check integrity")); | ||
this.initModality(Modality.NONE); | ||
|
||
|
@@ -76,10 +97,49 @@ 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); | ||
setGraphic(null); | ||
button.setOnAction(null); | ||
|
||
if (empty || getTableRow() == null || getTableRow().getItem() == null) { | ||
return; | ||
} | ||
calixtus marked this conversation as resolved.
Show resolved
Hide resolved
|
||
|
||
IntegrityMessage rowData = getTableRow().getItem(); | ||
Optional<IntegrityIssue> issue = IntegrityIssue.fromMessage(rowData); | ||
|
||
if (issue.isEmpty()) { | ||
return; | ||
} | ||
|
||
if (issue.get().getFix().isEmpty()) { | ||
setGraphic(new Label(Localization.lang("No fix available"))); | ||
} else { | ||
configureButton(issue.get().getFix().get(), () -> { | ||
viewModel.fix(issue.get(), rowData); | ||
removeRow(rowData); | ||
}); | ||
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 +151,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 +183,95 @@ public void clearFilters() { | |
}); | ||
} | ||
} | ||
|
||
private void updateEntryTypeCombo() { | ||
Set<String> entryTypes = new HashSet<>(viewModel.issueListProperty()); | ||
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) { | ||
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); | ||
removeRow(message); | ||
} | ||
|
||
public void removeRow(IntegrityMessage message) { | ||
Platform.runLater(() -> | ||
viewModel.columnsListProperty().getValue().removeIf(column -> | ||
Objects.equals(column.entry().getCitationKey().get(), message.entry().getCitationKey().get()) && | ||
Objects.equals(column.message(), message.message()) | ||
) | ||
); | ||
Platform.runLater(() -> viewModel.issueListProperty().removeIf(issue -> Objects.equals(issue, message.message()))); | ||
updateEntryTypeCombo(); | ||
} | ||
|
||
/** | ||
* 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 messages matching the selected issue type and have a fix | ||
.filter(message -> message.message().equals(issue.getText()) && hasFix(message)) | ||
.forEach(message -> { | ||
fix(issue, message); | ||
fixed.set(true); | ||
}); | ||
}); | ||
|
||
updateEntryTypeCombo(); | ||
|
||
if (fixed.get()) { | ||
notificationService.notify(Localization.lang("Fixed successfully.")); | ||
} else { | ||
notificationService.notify(Localization.lang("No fixes available.")); | ||
} | ||
} | ||
|
||
/** | ||
* 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(); | ||
} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,22 +1,47 @@ | ||
package org.jabref.gui.integrity; | ||
|
||
import java.util.List; | ||
import java.util.stream.Collectors; | ||
|
||
import javafx.beans.property.ListProperty; | ||
import javafx.beans.property.SimpleListProperty; | ||
import javafx.collections.FXCollections; | ||
import javafx.collections.ObservableList; | ||
|
||
import org.jabref.gui.AbstractViewModel; | ||
import org.jabref.logic.integrity.IntegrityIssue; | ||
import org.jabref.logic.integrity.IntegrityMessage; | ||
|
||
public class IntegrityCheckDialogViewModel extends AbstractViewModel { | ||
|
||
private final ListProperty<IntegrityMessage> columnsListProperty; | ||
private final ObservableList<String> issuesListProperty; | ||
private final ObservableList<IntegrityMessage> messages; | ||
|
||
public IntegrityCheckDialogViewModel(List<IntegrityMessage> messages) { | ||
this.messages = FXCollections.observableArrayList(messages); | ||
|
||
List<String> issuesList = messages.stream() | ||
.map(IntegrityMessage::message) | ||
.collect(Collectors.toList()); | ||
this.issuesListProperty = FXCollections.observableArrayList(issuesList); | ||
|
||
this.columnsListProperty = new SimpleListProperty<>(FXCollections.observableArrayList(messages)); | ||
} | ||
|
||
public ObservableList<String> issueListProperty() { | ||
return issuesListProperty; | ||
} | ||
|
||
public ListProperty<IntegrityMessage> columnsListProperty() { | ||
return this.columnsListProperty; | ||
} | ||
|
||
public ObservableList<IntegrityMessage> getMessages() { | ||
return messages; | ||
} | ||
|
||
public void fix(IntegrityIssue issue, IntegrityMessage message) { | ||
// fixes | ||
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. fixes,,,? |
||
} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -4,7 +4,6 @@ | |
import java.util.List; | ||
import java.util.Map; | ||
|
||
import org.jabref.logic.l10n.Localization; | ||
import org.jabref.model.entry.BibEntry; | ||
import org.jabref.model.entry.field.Field; | ||
|
||
|
@@ -21,7 +20,7 @@ public List<IntegrityMessage> check(BibEntry entry) { | |
for (Map.Entry<Field, String> field : entry.getFieldMap().entrySet()) { | ||
boolean asciiOnly = CharMatcher.ascii().matchesAllOf(field.getValue()); | ||
if (!asciiOnly) { | ||
results.add(new IntegrityMessage(Localization.lang("Non-ASCII encoded character found"), entry, | ||
results.add(new IntegrityMessage(IntegrityIssue.NON_ASCII_ENCODED_CHARACTER_FOUND.getText(), entry, | ||
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. Goes a bit beyond the scope of this PR, but you could refactor the IntegrityMessage to accept just the symbol and to resolve the symbol to a string at the caller. |
||
field.getKey())); | ||
} | ||
} | ||
|
Uh oh!
There was an error while loading. Please reload this page.