Skip to content
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

Fix duplicate external file type issue when change language and other corresponding issue of external file type preferences #10496

Merged
merged 63 commits into from
Dec 17, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
63 commits
Select commit Hold shift + click to select a range
2a4d378
change name to extension comparison in fromString and toStringList
papatekken Sep 24, 2023
5bb7c74
Merge branch 'JabRef:main' into fix-10127
papatekken Oct 8, 2023
7187b69
fix problem of abort button still add external file type entry in Edi…
papatekken Oct 8, 2023
cee577c
Merge branch 'JabRef:main' into fix-10127
papatekken Oct 8, 2023
341c52c
Block the Close action of EditExternalFileTypeEntryDialog when value …
papatekken Oct 15, 2023
76519e5
Merge branch 'JabRef:main' into fix-10127
papatekken Oct 15, 2023
f7cc511
ExternalFileTypesTab only scroll to bottom when new fileType added su…
papatekken Oct 15, 2023
f6166c0
check if external file type exists then cannot add a new one
papatekken Oct 15, 2023
df69445
Merge branch 'main' into fix-10127
papatekken Oct 19, 2023
5fe6df9
update changelog.md
papatekken Oct 19, 2023
e194bb9
Merge branch 'JabRef:main' into fix-10127
papatekken Oct 20, 2023
f0b21d0
reformat the new entry in CHANGELOG.md
papatekken Oct 20, 2023
7ccf1f6
Merge branch 'JabRef:main' into fix-10127
papatekken Oct 22, 2023
a84d4da
added ExternalFileTypesTabViewModelTest and run rewriteRun
papatekken Oct 22, 2023
b5c1d16
updated the comment format
papatekken Oct 22, 2023
ac06f61
rephase entry in CHANGELOG.md
papatekken Oct 23, 2023
c5bd645
Merge branch 'JabRef:main' into fix-10127
papatekken Oct 27, 2023
c604ecb
change viewModel.addNewType to return boolean
papatekken Oct 27, 2023
5bf5688
replace .trim.getEqual() with isblank()
papatekken Oct 30, 2023
ca3ed25
refactor logic of externalFileType add and edit
papatekken Nov 5, 2023
ee00a17
Merge branch 'main' into fix-10127
papatekken Nov 5, 2023
d797e03
add test for preference externalFileType addNewType() and isValidExte…
papatekken Nov 7, 2023
0399982
Merge branch 'JabRef:main' into fix-10127
papatekken Nov 7, 2023
2c5de27
updated changelog.md
papatekken Nov 7, 2023
ca54d17
updated codestyle of ExternalFileTypesTab.java
papatekken Nov 7, 2023
7da3779
updated codestyle of ExternalFileTypesEntryDialog.java
papatekken Nov 7, 2023
4f7ce6d
rename externalFileTypes test data file
papatekken Nov 7, 2023
74aa42d
gradle rewrite for some code formatting
papatekken Nov 7, 2023
c9f130c
refactor folder structure of ExternalFileTypeItemViewModelTestData
papatekken Nov 7, 2023
52b1ad7
reverse change of folder structure of ExternalFileTypeItemViewModelTe…
papatekken Nov 7, 2023
1cbb1e2
remove extra import statement from ExternalFileTypeItemViewModelTestD…
papatekken Nov 7, 2023
33c6db3
Merge branch 'main' into fix-10127
papatekken Nov 8, 2023
afa9a5c
add ExternalFileTypesTabViewModelTestData to TestArchitectureTest
papatekken Nov 10, 2023
78bce2a
Merge branch 'fix-10127' of github.com:papatekken/jabref into fix-10127
papatekken Nov 10, 2023
2b92fc8
Merge branch 'main' into fix-10127
papatekken Nov 10, 2023
7eb9eeb
Update CHANGELOG.md
papatekken Nov 10, 2023
c290c86
remove unnecessary variable
papatekken Nov 10, 2023
490a8a5
reformat code
papatekken Nov 10, 2023
8b89bf6
refactor TestArchitectureTest
papatekken Nov 10, 2023
7a08321
Merge branch 'JabRef:main' into fix-10127
papatekken Nov 11, 2023
d97608f
Merge branch 'main' into fix-10127
papatekken Nov 14, 2023
660dd03
Merge branch 'main' into fix-10127
papatekken Nov 14, 2023
e5ddf1f
Merge branch 'main' into fix-10127
papatekken Nov 15, 2023
5fb1c2c
Merge branch 'main' into fix-10127
papatekken Nov 16, 2023
e488411
Merge branch 'JabRef:main' into fix-10127
papatekken Dec 3, 2023
05024d3
EditExternalFileTypeEntryDialog form OK button disabled if empty exte…
papatekken Dec 7, 2023
a33f038
Merge branch 'JabRef:main' into fix-10127
papatekken Dec 7, 2023
424adb7
EditExternalFileTypeEntryDialog OK button disabled if duplicated file…
papatekken Dec 11, 2023
ce80a75
Merge branch 'main' into fix-10127
papatekken Dec 11, 2023
7bc67c4
remove exclamation mark in error message
papatekken Dec 11, 2023
ca9eaeb
change message File Extension exists already from info to debug
papatekken Dec 11, 2023
eec134f
refactor ExternalFileTypesTabViewModelTest.java
papatekken Dec 11, 2023
73cf3d5
updated TestArchitectureTest for removing ExternalFileTypeItemViewMod…
papatekken Dec 11, 2023
d983edd
extract methods for logics in isValidExternalFileType()
papatekken Dec 11, 2023
3fa21a3
add validator for name and MIME type
papatekken Dec 11, 2023
59e8ff4
code reformatting
papatekken Dec 11, 2023
0a2bcf0
reformatting code
papatekken Dec 11, 2023
275cf46
update language key
papatekken Dec 11, 2023
fd0835f
add unique name and mimetype validation for OK Button of EditExternal…
papatekken Dec 12, 2023
cb9210e
Merge branch 'main' into fix-10127
papatekken Dec 12, 2023
134251d
add message in JabRef_en.properties
papatekken Dec 12, 2023
a9db8af
Merge branch 'main' into fix-10127
papatekken Dec 15, 2023
b4e8c21
add validation visualizaion and improve error message grammar
Siedlerchr Dec 16, 2023
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,7 @@ Note that this project **does not** adhere to [Semantic Versioning](https://semv
- We fixed an issue where in the merge dialog the file field of entries was not correctly merged when the first and second entry both contained values inside the file field. [#10572](https://github.com/JabRef/jabref/issues/10572)
- We fixed some small inconsistencies in the user interface. [#10507](https://github.com/JabRef/jabref/issues/10507) [#10458](https://github.com/JabRef/jabref/issues/10458) [#10660](https://github.com/JabRef/jabref/issues/10660)
- We fixed the issue where the Hayagriva YAML exporter would not include a parent field for the publisher/series. [#10596](https://github.com/JabRef/jabref/issues/10596)
- We fixed issues in the external file type dialog w.r.t. duplicate entries in the case of a language switch. [#10271](https://github.com/JabRef/jabref/issues/10271)

### Removed

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -57,7 +57,7 @@ public ExporterViewModel saveExporter() {
// Check that there are no empty strings.
if (layoutFile.get().isEmpty() || name.get().isEmpty() || extension.get().isEmpty()
|| !layoutFile.get().endsWith(".layout")) {
LOGGER.info("One of the fields is empty or invalid!");
LOGGER.info("One of the fields is empty or invalid.");
return null;
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -148,10 +148,10 @@ public static String toStringList(Collection<ExternalFileType> fileTypes) {

for (ExternalFileType type : fileTypes) {
results.add(type);
// See if we can find a type with matching name in the default type list:
// See if we can find a type with matching extension in the default type list:
ExternalFileType found = null;
for (ExternalFileType defType : defTypes) {
if (defType.getName().equals(type.getName())) {
if (defType.getExtension().equals(type.getExtension())) {
found = defType;
break;
}
Expand Down Expand Up @@ -221,11 +221,11 @@ public static Set<ExternalFileType> fromString(String storedFileTypes) {
} else {
// A new or modified entry type. Construct it from the string array:
ExternalFileType type = CustomExternalFileType.buildFromArgs(val);
// Check if there is a default type with the same name. If so, this is a
// Check if there is a default type with the same extension. If so, this is a
// modification of that type, so remove the default one:
ExternalFileType toRemove = null;
for (ExternalFileType defType : types) {
if (type.getName().equals(defType.getName())) {
if (type.getExtension().equals(defType.getExtension())) {
toRemove = defType;
break;
}
Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,7 @@
package org.jabref.gui.preferences.externalfiletypes;

import javafx.application.Platform;
import javafx.collections.ObservableList;
import javafx.event.ActionEvent;
import javafx.fxml.FXML;
import javafx.scene.control.Button;
Expand All @@ -13,9 +15,11 @@
import org.jabref.gui.desktop.os.NativeDesktop;
import org.jabref.gui.util.BaseDialog;
import org.jabref.gui.util.FileDialogConfiguration;
import org.jabref.gui.util.IconValidationDecorator;
import org.jabref.logic.util.OS;

import com.airhacks.afterburner.views.ViewLoader;
import de.saxsys.mvvmfx.utils.validation.visualization.ControlsFxVisualizer;
import jakarta.inject.Inject;

public class EditExternalFileTypeEntryDialog extends BaseDialog<Void> {
Expand All @@ -33,20 +37,26 @@ public class EditExternalFileTypeEntryDialog extends BaseDialog<Void> {

private final NativeDesktop nativeDesktop = OS.getNativeDesktop();
private final FileDialogConfiguration fileDialogConfiguration = new FileDialogConfiguration.Builder().withInitialDirectory(nativeDesktop.getApplicationDirectory()).build();

private final ExternalFileTypeItemViewModel item;

private final ObservableList<ExternalFileTypeItemViewModel> fileTypes;
private EditExternalFileTypeViewModel viewModel;

public EditExternalFileTypeEntryDialog(ExternalFileTypeItemViewModel item, String dialogTitle) {
this.item = item;
private final ControlsFxVisualizer visualizer = new ControlsFxVisualizer();

public EditExternalFileTypeEntryDialog(ExternalFileTypeItemViewModel item, String dialogTitle, ObservableList<ExternalFileTypeItemViewModel> fileTypes) {
this.item = item;
this.fileTypes = fileTypes;
this.setTitle(dialogTitle);

ViewLoader.view(this)
.load()
.setAsDialogPane(this);

getDialogPane().getButtonTypes().setAll(ButtonType.OK, ButtonType.CANCEL);

final Button confirmDialogButton = (Button) getDialogPane().lookupButton(ButtonType.OK);
confirmDialogButton.disableProperty().bind(viewModel.validationStatus().validProperty().not());
this.setResultConverter(button -> {
if (button == ButtonType.OK) {
viewModel.storeSettings();
Expand All @@ -57,19 +67,26 @@ public EditExternalFileTypeEntryDialog(ExternalFileTypeItemViewModel item, Strin

@FXML
public void initialize() {
viewModel = new EditExternalFileTypeViewModel(item);
visualizer.setDecoration(new IconValidationDecorator());

viewModel = new EditExternalFileTypeViewModel(item, fileTypes);

icon.setGraphic(viewModel.getIcon());

defaultApplication.selectedProperty().bindBidirectional(viewModel.defaultApplicationSelectedProperty());
customApplication.selectedProperty().bindBidirectional(viewModel.customApplicationSelectedProperty());
selectedApplication.disableProperty().bind(viewModel.defaultApplicationSelectedProperty());
btnBrowse.disableProperty().bind(viewModel.defaultApplicationSelectedProperty());

extension.textProperty().bindBidirectional(viewModel.extensionProperty());
name.textProperty().bindBidirectional(viewModel.nameProperty());
mimeType.textProperty().bindBidirectional(viewModel.mimeTypeProperty());
selectedApplication.textProperty().bindBidirectional(viewModel.selectedApplicationProperty());

Platform.runLater(() -> {
visualizer.initVisualization(viewModel.extensionValidation(), extension, true);
visualizer.initVisualization(viewModel.nameValidation(), name, true);
visualizer.initVisualization(viewModel.mimeTypeValidation(), mimeType, true);
});
}

@FXML
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,21 +4,41 @@
import javafx.beans.property.SimpleBooleanProperty;
import javafx.beans.property.SimpleStringProperty;
import javafx.beans.property.StringProperty;
import javafx.collections.ObservableList;
import javafx.scene.Node;

import org.jabref.logic.l10n.Localization;
import org.jabref.model.strings.StringUtil;

import de.saxsys.mvvmfx.utils.validation.CompositeValidator;
import de.saxsys.mvvmfx.utils.validation.FunctionBasedValidator;
import de.saxsys.mvvmfx.utils.validation.ValidationMessage;
import de.saxsys.mvvmfx.utils.validation.ValidationStatus;
import de.saxsys.mvvmfx.utils.validation.Validator;

public class EditExternalFileTypeViewModel {
private final ExternalFileTypeItemViewModel fileTypeViewModel;

private final StringProperty nameProperty = new SimpleStringProperty("");
private final StringProperty mimeTypeProperty = new SimpleStringProperty("");
private final StringProperty extensionProperty = new SimpleStringProperty("");
private final StringProperty selectedApplicationProperty = new SimpleStringProperty("");
private final BooleanProperty defaultApplicationSelectedProperty = new SimpleBooleanProperty(false);
private final BooleanProperty customApplicationSelectedProperty = new SimpleBooleanProperty(false);

public EditExternalFileTypeViewModel(ExternalFileTypeItemViewModel fileTypeViewModel) {
private final ObservableList<ExternalFileTypeItemViewModel> fileTypes;
private final String originalExtension;
private final String originalName;
private final String originalMimeType;
private CompositeValidator extensionValidator;
private CompositeValidator nameValidator;
private CompositeValidator mimeTypeValidator;
private CompositeValidator validator;

public EditExternalFileTypeViewModel(ExternalFileTypeItemViewModel fileTypeViewModel, ObservableList<ExternalFileTypeItemViewModel> fileTypes) {
this.fileTypeViewModel = fileTypeViewModel;

this.fileTypes = fileTypes;
this.originalExtension = fileTypeViewModel.extensionProperty().getValue();
this.originalName = fileTypeViewModel.nameProperty().getValue();
this.originalMimeType = fileTypeViewModel.mimetypeProperty().getValue();
extensionProperty.setValue(fileTypeViewModel.extensionProperty().getValue());
nameProperty.setValue(fileTypeViewModel.nameProperty().getValue());
mimeTypeProperty.setValue(fileTypeViewModel.mimetypeProperty().getValue());
Expand All @@ -29,6 +49,92 @@ public EditExternalFileTypeViewModel(ExternalFileTypeItemViewModel fileTypeViewM
customApplicationSelectedProperty.setValue(true);
selectedApplicationProperty.setValue(fileTypeViewModel.applicationProperty().getValue());
}

setupValidation();
}

private void setupValidation() {
validator = new CompositeValidator();
extensionValidator = new CompositeValidator();

Validator extensionisNotBlankValidator = new FunctionBasedValidator<>(
extensionProperty,
StringUtil::isNotBlank,
ValidationMessage.error(Localization.lang("Please enter a name for the extension."))
);
Validator sameExtensionValidator = new FunctionBasedValidator<>(
extensionProperty,
extension -> {
for (ExternalFileTypeItemViewModel fileTypeItem : fileTypes) {
if (extension.equalsIgnoreCase(fileTypeItem.extensionProperty().get()) && !extension.equalsIgnoreCase(originalExtension)) {
return false;
}
}
return true;
},
ValidationMessage.error(Localization.lang("There already exists an external file type with the same extension"))
);
extensionValidator.addValidators(sameExtensionValidator, extensionisNotBlankValidator);

nameValidator = new CompositeValidator();
Validator sameNameValidator = new FunctionBasedValidator<>(
nameProperty,
name -> {
for (ExternalFileTypeItemViewModel fileTypeItem : fileTypes) {
if (name.equalsIgnoreCase(fileTypeItem.nameProperty().get()) && !name.equalsIgnoreCase(originalName)) {
return false;
}
}
return true;
},
ValidationMessage.error(Localization.lang("There already exists an external file type with the same name"))
);

Validator nameIsNotBlankValidator = new FunctionBasedValidator<>(
nameProperty,
StringUtil::isNotBlank,
ValidationMessage.error(Localization.lang("Please enter a name."))
);
nameValidator.addValidators(sameNameValidator, nameIsNotBlankValidator);

mimeTypeValidator = new CompositeValidator();
Validator mimeTypeIsNotBlankValidator = new FunctionBasedValidator<>(
mimeTypeProperty,
StringUtil::isNotBlank,
ValidationMessage.error(Localization.lang("Please enter a name for the MIME type."))
);

Validator sameMimeTypeValidator = new FunctionBasedValidator<>(
mimeTypeProperty,
mimeType -> {
for (ExternalFileTypeItemViewModel fileTypeItem : fileTypes) {
if (mimeType.equalsIgnoreCase(fileTypeItem.mimetypeProperty().get()) && !mimeType.equalsIgnoreCase(originalMimeType)) {
return false;
}
}
return true;
},
ValidationMessage.error(Localization.lang("There already exists an external file type with the same MIME type"))
);
mimeTypeValidator.addValidators(sameMimeTypeValidator, mimeTypeIsNotBlankValidator);

validator.addValidators(extensionValidator, sameExtensionValidator, nameValidator, sameNameValidator, mimeTypeValidator, sameMimeTypeValidator);
}

public ValidationStatus validationStatus() {
return validator.getValidationStatus();
}

public ValidationStatus extensionValidation() {
return extensionValidator.getValidationStatus();
}

public ValidationStatus mimeTypeValidation() {
return mimeTypeValidator.getValidationStatus();
}

public ValidationStatus nameValidation() {
return nameValidator.getValidationStatus();
}

public Node getIcon() {
Expand Down Expand Up @@ -59,6 +165,10 @@ public BooleanProperty customApplicationSelectedProperty() {
return customApplicationSelectedProperty;
}

public BooleanProperty validExtensionTypeProperty() {
return defaultApplicationSelectedProperty;
}

public void storeSettings() {
fileTypeViewModel.nameProperty().setValue(nameProperty.getValue().trim());
fileTypeViewModel.mimetypeProperty().setValue(mimeTypeProperty.getValue().trim());
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -78,19 +78,27 @@ public void initialize() {
.install(fileTypesTableIconColumn);
new ValueTableCellFactory<ExternalFileTypeItemViewModel, Boolean>()
.withGraphic(none -> IconTheme.JabRefIcons.EDIT.getGraphicNode())
.withOnMouseClickedEvent((type, none) -> event -> viewModel.edit(type))
.withOnMouseClickedEvent((type, none) -> event -> editType(type))
.install(fileTypesTableEditColumn);
new ValueTableCellFactory<ExternalFileTypeItemViewModel, Boolean>()
.withGraphic(none -> IconTheme.JabRefIcons.DELETE_ENTRY.getGraphicNode())
.withOnMouseClickedEvent((type, none) -> event -> viewModel.remove(type))
.install(fileTypesTableDeleteColumn);
}

private void editType(ExternalFileTypeItemViewModel type) {
if (viewModel.edit(type)) {
fileTypesTable.getSelectionModel().selectLast();
fileTypesTable.scrollTo(viewModel.getFileTypes().size() - 1);
}
}

@FXML
private void addNewType() {
viewModel.addNewType();
fileTypesTable.getSelectionModel().selectLast();
fileTypesTable.scrollTo(viewModel.getFileTypes().size() - 1);
if (viewModel.addNewType()) {
fileTypesTable.getSelectionModel().selectLast();
fileTypesTable.scrollTo(viewModel.getFileTypes().size() - 1);
}
}

@FXML
Expand Down
Loading
Loading