From b90cf997887853afaea2601f09f175e5cb50f67c Mon Sep 17 00:00:00 2001 From: Oliver Kopp Date: Tue, 8 Jun 2021 23:03:16 +0200 Subject: [PATCH 01/14] Add telemetry for web search --- src/main/java/org/jabref/gui/JabRefFrame.java | 9 ++++----- src/main/java/org/jabref/gui/actions/JabRefAction.java | 9 ++++----- .../gui/importer/fetcher/WebSearchPaneViewModel.java | 2 ++ 3 files changed, 10 insertions(+), 10 deletions(-) diff --git a/src/main/java/org/jabref/gui/JabRefFrame.java b/src/main/java/org/jabref/gui/JabRefFrame.java index ecbb1fa02c6..c2ba6d459a5 100644 --- a/src/main/java/org/jabref/gui/JabRefFrame.java +++ b/src/main/java/org/jabref/gui/JabRefFrame.java @@ -1040,11 +1040,10 @@ public void addTab(LibraryTab libraryTab, boolean raisePanel) { } private void trackOpenNewDatabase(LibraryTab libraryTab) { - Map properties = new HashMap<>(); - Map measurements = new HashMap<>(); - measurements.put("NumberOfEntries", (double) libraryTab.getBibDatabaseContext().getDatabase().getEntryCount()); - - Globals.getTelemetryClient().ifPresent(client -> client.trackEvent("OpenNewDatabase", properties, measurements)); + Globals.getTelemetryClient().ifPresent(client -> client.trackEvent( + "OpenNewDatabase", + Map.of(), + Map.of("NumberOfEntries", (double) libraryTab.getBibDatabaseContext().getDatabase().getEntryCount()))); } public LibraryTab addTab(BibDatabaseContext databaseContext, boolean raisePanel) { diff --git a/src/main/java/org/jabref/gui/actions/JabRefAction.java b/src/main/java/org/jabref/gui/actions/JabRefAction.java index b19c9ca110e..c358ca68956 100644 --- a/src/main/java/org/jabref/gui/actions/JabRefAction.java +++ b/src/main/java/org/jabref/gui/actions/JabRefAction.java @@ -77,10 +77,9 @@ private void trackExecute(String actionName) { } private void trackUserActionSource(String actionName, Sources source) { - Map properties = new HashMap<>(); - Map measurements = new HashMap<>(); - properties.put("Source", source.toString()); - - Globals.getTelemetryClient().ifPresent(telemetryClient -> telemetryClient.trackEvent(actionName, properties, measurements)); + Globals.getTelemetryClient().ifPresent(telemetryClient -> telemetryClient.trackEvent( + actionName, + Map.of("Source", source.toString()), + Map.of()); } } diff --git a/src/main/java/org/jabref/gui/importer/fetcher/WebSearchPaneViewModel.java b/src/main/java/org/jabref/gui/importer/fetcher/WebSearchPaneViewModel.java index 0ef1e6ff233..62e7f5daeb8 100644 --- a/src/main/java/org/jabref/gui/importer/fetcher/WebSearchPaneViewModel.java +++ b/src/main/java/org/jabref/gui/importer/fetcher/WebSearchPaneViewModel.java @@ -1,5 +1,6 @@ package org.jabref.gui.importer.fetcher; +import java.util.Map; import java.util.SortedSet; import javafx.beans.property.ListProperty; @@ -88,6 +89,7 @@ public void search() { } SearchBasedFetcher activeFetcher = getSelectedFetcher(); + Globals.getTelemetryClient().ifPresent(client -> client.trackEvent("search", Map.of("fetcher", activeFetcher.getName()), Map.of())); BackgroundTask task; task = BackgroundTask.wrap(() -> new ParserResult(activeFetcher.performSearch(getQuery().trim()))) From 692826379e6f932f86059061d124984fa34d4ff3 Mon Sep 17 00:00:00 2001 From: Oliver Kopp Date: Tue, 8 Jun 2021 23:08:06 +0200 Subject: [PATCH 02/14] Unzip query parsing code --- .../java/org/jabref/logic/importer/SearchBasedFetcher.java | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/src/main/java/org/jabref/logic/importer/SearchBasedFetcher.java b/src/main/java/org/jabref/logic/importer/SearchBasedFetcher.java index 3d20115bbf8..8e3e3d75533 100644 --- a/src/main/java/org/jabref/logic/importer/SearchBasedFetcher.java +++ b/src/main/java/org/jabref/logic/importer/SearchBasedFetcher.java @@ -36,12 +36,15 @@ default List performSearch(String searchQuery) throws FetcherException if (searchQuery.isBlank()) { return Collections.emptyList(); } - SyntaxParser parser = new StandardSyntaxParser(); + SyntaxParser parser = new StandardSyntaxParser(); + QueryNode queryNode; try { - return this.performSearch(parser.parse(searchQuery, NO_EXPLICIT_FIELD)); + queryNode = parser.parse(searchQuery, NO_EXPLICIT_FIELD); } catch (QueryNodeParseException e) { throw new FetcherException("An error occurred when parsing the query"); } + + return this.performSearch(queryNode); } } From 29d36a0dd2ab90db037c7b1e0714989ca76425c1 Mon Sep 17 00:00:00 2001 From: Oliver Kopp Date: Tue, 8 Jun 2021 23:13:22 +0200 Subject: [PATCH 03/14] Fix compilation error --- src/main/java/org/jabref/gui/actions/JabRefAction.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/main/java/org/jabref/gui/actions/JabRefAction.java b/src/main/java/org/jabref/gui/actions/JabRefAction.java index c358ca68956..fa083ba81f1 100644 --- a/src/main/java/org/jabref/gui/actions/JabRefAction.java +++ b/src/main/java/org/jabref/gui/actions/JabRefAction.java @@ -80,6 +80,6 @@ private void trackUserActionSource(String actionName, Sources source) { Globals.getTelemetryClient().ifPresent(telemetryClient -> telemetryClient.trackEvent( actionName, Map.of("Source", source.toString()), - Map.of()); + Map.of())); } } From 68f118b7c8a89056e5f637df6070dcb25dfd320f Mon Sep 17 00:00:00 2001 From: Oliver Kopp Date: Tue, 8 Jun 2021 23:54:07 +0200 Subject: [PATCH 04/14] Add query validation Co-authored-by: Carl Christian Snethlage <50491877+calixtus@users.noreply.github.com> --- .../gui/importer/fetcher/WebSearchPane.java | 5 +++ .../fetcher/WebSearchPaneViewModel.java | 41 +++++++++++++++++-- 2 files changed, 42 insertions(+), 4 deletions(-) diff --git a/src/main/java/org/jabref/gui/importer/fetcher/WebSearchPane.java b/src/main/java/org/jabref/gui/importer/fetcher/WebSearchPane.java index b12cb170d3b..59e708fcb0a 100644 --- a/src/main/java/org/jabref/gui/importer/fetcher/WebSearchPane.java +++ b/src/main/java/org/jabref/gui/importer/fetcher/WebSearchPane.java @@ -1,5 +1,6 @@ package org.jabref.gui.importer.fetcher; +import javafx.css.PseudoClass; import javafx.geometry.Pos; import javafx.scene.Node; import javafx.scene.control.Button; @@ -22,6 +23,7 @@ import org.jabref.gui.help.HelpAction; import org.jabref.gui.icon.IconTheme; import org.jabref.gui.search.SearchTextField; +import org.jabref.gui.util.BindingsHelper; import org.jabref.gui.util.ViewModelListCellFactory; import org.jabref.logic.importer.SearchBasedFetcher; import org.jabref.logic.l10n.Localization; @@ -31,6 +33,8 @@ public class WebSearchPane extends SidePaneComponent { + private static final PseudoClass QUERY_INVALID = PseudoClass.getPseudoClass("invalid"); + private final PreferencesService preferences; private final WebSearchPaneViewModel viewModel; @@ -73,6 +77,7 @@ protected Node createContentPane() { // Create text field for query input TextField query = SearchTextField.create(); query.getStyleClass().add("searchBar"); + BindingsHelper.includePseudoClassWhen(query, QUERY_INVALID, viewModel.queryInvalidationStatus().validProperty()); viewModel.queryProperty().bind(query.textProperty()); diff --git a/src/main/java/org/jabref/gui/importer/fetcher/WebSearchPaneViewModel.java b/src/main/java/org/jabref/gui/importer/fetcher/WebSearchPaneViewModel.java index 62e7f5daeb8..c8811f85ef6 100644 --- a/src/main/java/org/jabref/gui/importer/fetcher/WebSearchPaneViewModel.java +++ b/src/main/java/org/jabref/gui/importer/fetcher/WebSearchPaneViewModel.java @@ -2,6 +2,7 @@ import java.util.Map; import java.util.SortedSet; +import java.util.function.Predicate; import javafx.beans.property.ListProperty; import javafx.beans.property.ObjectProperty; @@ -13,6 +14,7 @@ import javafx.collections.ObservableList; import org.jabref.gui.DialogService; +import org.jabref.gui.Globals; import org.jabref.gui.StateManager; import org.jabref.gui.importer.ImportEntriesDialog; import org.jabref.gui.util.BackgroundTask; @@ -24,6 +26,15 @@ import org.jabref.preferences.PreferencesService; import com.tobiasdiez.easybind.EasyBind; +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; +import org.apache.lucene.queryparser.flexible.core.QueryNodeParseException; +import org.apache.lucene.queryparser.flexible.core.parser.SyntaxParser; +import org.apache.lucene.queryparser.flexible.standard.parser.StandardSyntaxParser; + +import static org.jabref.logic.importer.fetcher.transformers.AbstractQueryTransformer.NO_EXPLICIT_FIELD; public class WebSearchPaneViewModel { @@ -33,6 +44,9 @@ public class WebSearchPaneViewModel { private final DialogService dialogService; private final StateManager stateManager; + private final Validator searchQueryValidator; + private final SyntaxParser parser = new StandardSyntaxParser(); + public WebSearchPaneViewModel(PreferencesService preferencesService, DialogService dialogService, StateManager stateManager) { this.dialogService = dialogService; this.stateManager = stateManager; @@ -51,6 +65,21 @@ public WebSearchPaneViewModel(PreferencesService preferencesService, DialogServi int newIndex = fetchers.indexOf(newFetcher); preferencesService.storeSidePanePreferences(preferencesService.getSidePanePreferences().withWebSearchFetcherSelected(newIndex)); }); + + Predicate queryValid = queryText -> { + if (StringUtil.isBlank(queryText)) { + // in case user did not enter something, it is treated as valid (to avoid UI WTFs) + return true; + } + try { + parser.parse(queryText, NO_EXPLICIT_FIELD); + return true; + } catch (QueryNodeParseException e) { + return false; + } + }; + searchQueryValidator = new FunctionBasedValidator<>( + query, queryValid.negate(), ValidationMessage.error(Localization.lang("Search query unparseable"))); } public ObservableList getFetchers() { @@ -78,11 +107,11 @@ public StringProperty queryProperty() { } public void search() { - if (StringUtil.isBlank(getQuery())) { + String query = getQuery().trim(); + if (StringUtil.isBlank(query)) { dialogService.notify(Localization.lang("Please enter a search string")); return; } - if (stateManager.getActiveDatabase().isEmpty()) { dialogService.notify(Localization.lang("Please open or start a new library before searching")); return; @@ -92,12 +121,16 @@ public void search() { Globals.getTelemetryClient().ifPresent(client -> client.trackEvent("search", Map.of("fetcher", activeFetcher.getName()), Map.of())); BackgroundTask task; - task = BackgroundTask.wrap(() -> new ParserResult(activeFetcher.performSearch(getQuery().trim()))) - .withInitialMessage(Localization.lang("Processing %0", getQuery().trim())); + task = BackgroundTask.wrap(() -> new ParserResult(activeFetcher.performSearch(query))) + .withInitialMessage(Localization.lang("Processing %0", query)); task.onFailure(dialogService::showErrorDialogAndWait); ImportEntriesDialog dialog = new ImportEntriesDialog(stateManager.getActiveDatabase().get(), task); dialog.setTitle(activeFetcher.getName()); dialogService.showCustomDialogAndWait(dialog); } + + public ValidationStatus queryInvalidationStatus() { + return searchQueryValidator.getValidationStatus(); + } } From bd169fd12a837436362ff4724002f5c9d1d86499 Mon Sep 17 00:00:00 2001 From: Oliver Kopp Date: Wed, 9 Jun 2021 00:01:01 +0200 Subject: [PATCH 05/14] Add missing comment Co-authored-by: Carl Christian Snethlage <50491877+calixtus@users.noreply.github.com> --- .../java/org/jabref/gui/importer/fetcher/WebSearchPane.java | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/main/java/org/jabref/gui/importer/fetcher/WebSearchPane.java b/src/main/java/org/jabref/gui/importer/fetcher/WebSearchPane.java index 59e708fcb0a..597b0be9654 100644 --- a/src/main/java/org/jabref/gui/importer/fetcher/WebSearchPane.java +++ b/src/main/java/org/jabref/gui/importer/fetcher/WebSearchPane.java @@ -77,6 +77,8 @@ protected Node createContentPane() { // Create text field for query input TextField query = SearchTextField.create(); query.getStyleClass().add("searchBar"); + + // Weird issue: viewModel.queryValidationStatus().not() does not work... BindingsHelper.includePseudoClassWhen(query, QUERY_INVALID, viewModel.queryInvalidationStatus().validProperty()); viewModel.queryProperty().bind(query.textProperty()); From e889c4b0b0069cf8023ae63c173eff993b8749bb Mon Sep 17 00:00:00 2001 From: Oliver Kopp Date: Wed, 9 Jun 2021 07:08:47 +0200 Subject: [PATCH 06/14] Fix checkstyle --- src/main/java/org/jabref/gui/JabRefFrame.java | 1 - src/main/java/org/jabref/gui/actions/JabRefAction.java | 1 - 2 files changed, 2 deletions(-) diff --git a/src/main/java/org/jabref/gui/JabRefFrame.java b/src/main/java/org/jabref/gui/JabRefFrame.java index c2ba6d459a5..dd137b422c8 100644 --- a/src/main/java/org/jabref/gui/JabRefFrame.java +++ b/src/main/java/org/jabref/gui/JabRefFrame.java @@ -4,7 +4,6 @@ import java.nio.file.Path; import java.util.ArrayList; import java.util.Collections; -import java.util.HashMap; import java.util.List; import java.util.Map; import java.util.Objects; diff --git a/src/main/java/org/jabref/gui/actions/JabRefAction.java b/src/main/java/org/jabref/gui/actions/JabRefAction.java index fa083ba81f1..9d0e7217f85 100644 --- a/src/main/java/org/jabref/gui/actions/JabRefAction.java +++ b/src/main/java/org/jabref/gui/actions/JabRefAction.java @@ -1,6 +1,5 @@ package org.jabref.gui.actions; -import java.util.HashMap; import java.util.Map; import javafx.beans.binding.Bindings; From 1adbc27db680b619cd1a1f5653737b240944d36e Mon Sep 17 00:00:00 2001 From: Oliver Kopp Date: Wed, 9 Jun 2021 07:27:00 +0200 Subject: [PATCH 07/14] Include error message --- .../gui/importer/fetcher/WebSearchPane.java | 2 +- .../fetcher/WebSearchPaneViewModel.java | 33 +++++++++++-------- src/main/resources/l10n/JabRef_en.properties | 1 + 3 files changed, 21 insertions(+), 15 deletions(-) diff --git a/src/main/java/org/jabref/gui/importer/fetcher/WebSearchPane.java b/src/main/java/org/jabref/gui/importer/fetcher/WebSearchPane.java index 597b0be9654..f9a2e30a3fb 100644 --- a/src/main/java/org/jabref/gui/importer/fetcher/WebSearchPane.java +++ b/src/main/java/org/jabref/gui/importer/fetcher/WebSearchPane.java @@ -79,7 +79,7 @@ protected Node createContentPane() { query.getStyleClass().add("searchBar"); // Weird issue: viewModel.queryValidationStatus().not() does not work... - BindingsHelper.includePseudoClassWhen(query, QUERY_INVALID, viewModel.queryInvalidationStatus().validProperty()); + BindingsHelper.includePseudoClassWhen(query, QUERY_INVALID, viewModel.queryValidationStatus().validProperty().not()); viewModel.queryProperty().bind(query.textProperty()); diff --git a/src/main/java/org/jabref/gui/importer/fetcher/WebSearchPaneViewModel.java b/src/main/java/org/jabref/gui/importer/fetcher/WebSearchPaneViewModel.java index c8811f85ef6..43cfd1f4be0 100644 --- a/src/main/java/org/jabref/gui/importer/fetcher/WebSearchPaneViewModel.java +++ b/src/main/java/org/jabref/gui/importer/fetcher/WebSearchPaneViewModel.java @@ -32,6 +32,7 @@ import de.saxsys.mvvmfx.utils.validation.Validator; import org.apache.lucene.queryparser.flexible.core.QueryNodeParseException; import org.apache.lucene.queryparser.flexible.core.parser.SyntaxParser; +import org.apache.lucene.queryparser.flexible.standard.parser.ParseException; import org.apache.lucene.queryparser.flexible.standard.parser.StandardSyntaxParser; import static org.jabref.logic.importer.fetcher.transformers.AbstractQueryTransformer.NO_EXPLICIT_FIELD; @@ -66,20 +67,24 @@ public WebSearchPaneViewModel(PreferencesService preferencesService, DialogServi preferencesService.storeSidePanePreferences(preferencesService.getSidePanePreferences().withWebSearchFetcherSelected(newIndex)); }); - Predicate queryValid = queryText -> { - if (StringUtil.isBlank(queryText)) { - // in case user did not enter something, it is treated as valid (to avoid UI WTFs) - return true; - } - try { - parser.parse(queryText, NO_EXPLICIT_FIELD); - return true; - } catch (QueryNodeParseException e) { - return false; - } - }; searchQueryValidator = new FunctionBasedValidator<>( - query, queryValid.negate(), ValidationMessage.error(Localization.lang("Search query unparseable"))); + query, + queryText -> { + if (StringUtil.isBlank(queryText)) { + // in case user did not enter something, it is treated as valid (to avoid UI WTFs) + return null; + } + try { + parser.parse(queryText, NO_EXPLICIT_FIELD); + return null; + } catch (ParseException e) { + String element = e.currentToken.image; + int position = e.currentToken.beginColumn; + return ValidationMessage.error(Localization.lang("Invalid query element '%0' at position %1", element, position)); + } catch (QueryNodeParseException e) { + return ValidationMessage.error(""); + } + }); } public ObservableList getFetchers() { @@ -130,7 +135,7 @@ public void search() { dialogService.showCustomDialogAndWait(dialog); } - public ValidationStatus queryInvalidationStatus() { + public ValidationStatus queryValidationStatus() { return searchQueryValidator.getValidationStatus(); } } diff --git a/src/main/resources/l10n/JabRef_en.properties b/src/main/resources/l10n/JabRef_en.properties index 70ddb2bf6d4..09afc402645 100644 --- a/src/main/resources/l10n/JabRef_en.properties +++ b/src/main/resources/l10n/JabRef_en.properties @@ -909,6 +909,7 @@ You\ must\ restart\ JabRef\ for\ this\ to\ come\ into\ effect.=You must restart The\ following\ fetchers\ are\ available\:=The following fetchers are available: Could\ not\ find\ fetcher\ '%0'=Could not find fetcher '%0' Running\ query\ '%0'\ with\ fetcher\ '%1'.=Running query '%0' with fetcher '%1'. +Invalid\ query\ element\ '%0'\ at\ position\ %1=Invalid query element '%0' at position %1 Move\ file=Move file Rename\ file=Rename file From a62891c7d368525b1b9ee7c2643dbc8feb007de8 Mon Sep 17 00:00:00 2001 From: Oliver Kopp Date: Wed, 9 Jun 2021 07:34:31 +0200 Subject: [PATCH 08/14] Fix checkstyle --- .../org/jabref/gui/importer/fetcher/WebSearchPaneViewModel.java | 1 - 1 file changed, 1 deletion(-) diff --git a/src/main/java/org/jabref/gui/importer/fetcher/WebSearchPaneViewModel.java b/src/main/java/org/jabref/gui/importer/fetcher/WebSearchPaneViewModel.java index 43cfd1f4be0..44d4598d39d 100644 --- a/src/main/java/org/jabref/gui/importer/fetcher/WebSearchPaneViewModel.java +++ b/src/main/java/org/jabref/gui/importer/fetcher/WebSearchPaneViewModel.java @@ -2,7 +2,6 @@ import java.util.Map; import java.util.SortedSet; -import java.util.function.Predicate; import javafx.beans.property.ListProperty; import javafx.beans.property.ObjectProperty; From ffa9841705d710b39c323410477e36b41e1ac7a4 Mon Sep 17 00:00:00 2001 From: Carl Christian Snethlage Date: Thu, 10 Jun 2021 23:55:15 +0200 Subject: [PATCH 09/14] Added tests --- .../fetcher/WebSearchPaneViewModelTest.java | 54 +++++++++++++++++++ 1 file changed, 54 insertions(+) create mode 100644 src/test/java/org/jabref/gui/importer/fetcher/WebSearchPaneViewModelTest.java diff --git a/src/test/java/org/jabref/gui/importer/fetcher/WebSearchPaneViewModelTest.java b/src/test/java/org/jabref/gui/importer/fetcher/WebSearchPaneViewModelTest.java new file mode 100644 index 00000000000..7c3651385f1 --- /dev/null +++ b/src/test/java/org/jabref/gui/importer/fetcher/WebSearchPaneViewModelTest.java @@ -0,0 +1,54 @@ +package org.jabref.gui.importer.fetcher; + +import org.jabref.gui.DialogService; +import org.jabref.gui.StateManager; +import org.jabref.preferences.PreferencesService; + +import org.junit.jupiter.api.BeforeEach; +import org.junit.jupiter.api.Test; +import org.mockito.Mockito; + +import static org.junit.jupiter.api.Assertions.assertFalse; +import static org.junit.jupiter.api.Assertions.assertTrue; +import static org.mockito.Mockito.RETURNS_DEEP_STUBS; + +class WebSearchPaneViewModelTest { + + private PreferencesService preferencesService; + private DialogService dialogService; + private StateManager stateManager; + private WebSearchPaneViewModel viewModel; + + @BeforeEach + void setUp() { + preferencesService = Mockito.mock(PreferencesService.class, RETURNS_DEEP_STUBS); + dialogService = Mockito.mock(DialogService.class); + stateManager = Mockito.mock(StateManager.class); + + viewModel = new WebSearchPaneViewModel(preferencesService, dialogService, stateManager); + } + + @Test + void falseQueryValidationStatus() { + viewModel.queryProperty().setValue("Miami !Beach AND OR Blue"); + assertFalse(viewModel.queryValidationStatus().validProperty().getValue()); + } + + @Test + void correctQueryValidationStatus() { + viewModel.queryProperty().setValue("Miami AND Beach OR Houston AND Texas"); + assertTrue(viewModel.queryValidationStatus().validProperty().getValue()); + } + + @Test + void notFalseQueryValidationStatus() { + viewModel.queryProperty().setValue("Miami !Beach AND OR Blue"); + assertTrue(viewModel.queryValidationStatus().validProperty().not().getValue()); + } + + @Test + void notCorrectQueryValidationStatus() { + viewModel.queryProperty().setValue("Miami AND Beach OR Houston AND Texas"); + assertFalse(viewModel.queryValidationStatus().validProperty().not().getValue()); + } +} From e8207f5d5ebac254e54a6332b67e3aa0f69afb10 Mon Sep 17 00:00:00 2001 From: Carl Christian Snethlage Date: Mon, 21 Jun 2021 21:21:29 +0200 Subject: [PATCH 10/14] Added textfield for testing purpose --- .../org/jabref/gui/importer/fetcher/WebSearchPane.java | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/src/main/java/org/jabref/gui/importer/fetcher/WebSearchPane.java b/src/main/java/org/jabref/gui/importer/fetcher/WebSearchPane.java index f9a2e30a3fb..f9bd7218da5 100644 --- a/src/main/java/org/jabref/gui/importer/fetcher/WebSearchPane.java +++ b/src/main/java/org/jabref/gui/importer/fetcher/WebSearchPane.java @@ -82,6 +82,13 @@ protected Node createContentPane() { BindingsHelper.includePseudoClassWhen(query, QUERY_INVALID, viewModel.queryValidationStatus().validProperty().not()); viewModel.queryProperty().bind(query.textProperty()); + TextField queryValid = new TextField(); + EasyBind.subscribe(viewModel.queryValidationStatus().validProperty(), + valid -> { + if (!valid && viewModel.queryValidationStatus().getHighestMessage().isPresent()) { + queryValid.setText(viewModel.queryValidationStatus().getHighestMessage().get().getMessage()); + } + }); // Allows to trigger search on pressing enter query.setOnKeyPressed(event -> { @@ -98,7 +105,7 @@ protected Node createContentPane() { // Put everything together VBox container = new VBox(); container.setAlignment(Pos.CENTER); - container.getChildren().addAll(fetcherContainer, query, search); + container.getChildren().addAll(fetcherContainer, query, search, queryValid); return container; } From 58c195b27e1a9ecf3e1cf7985cdd4f2daa544da0 Mon Sep 17 00:00:00 2001 From: Carl Christian Snethlage Date: Mon, 21 Jun 2021 21:25:04 +0200 Subject: [PATCH 11/14] Added textfield for testing purpose again --- .../java/org/jabref/gui/importer/fetcher/WebSearchPane.java | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/main/java/org/jabref/gui/importer/fetcher/WebSearchPane.java b/src/main/java/org/jabref/gui/importer/fetcher/WebSearchPane.java index f9bd7218da5..a8aaeceda03 100644 --- a/src/main/java/org/jabref/gui/importer/fetcher/WebSearchPane.java +++ b/src/main/java/org/jabref/gui/importer/fetcher/WebSearchPane.java @@ -87,6 +87,8 @@ protected Node createContentPane() { valid -> { if (!valid && viewModel.queryValidationStatus().getHighestMessage().isPresent()) { queryValid.setText(viewModel.queryValidationStatus().getHighestMessage().get().getMessage()); + } else { + queryValid.setText(""); } }); From 28cf9cb8e2d5243da99622436272d67093d7751e Mon Sep 17 00:00:00 2001 From: Oliver Kopp Date: Mon, 21 Jun 2021 22:28:46 +0200 Subject: [PATCH 12/14] Fix CSS Co-authored-by: Carl Christian Snethlage <50491877+calixtus@users.noreply.github.com> Co-authored-by: Christoph --- src/main/java/org/jabref/gui/Base.css | 4 ++++ .../jabref/gui/importer/fetcher/WebSearchPane.java | 13 ++++++------- 2 files changed, 10 insertions(+), 7 deletions(-) diff --git a/src/main/java/org/jabref/gui/Base.css b/src/main/java/org/jabref/gui/Base.css index c6679c830f1..284b6822b93 100644 --- a/src/main/java/org/jabref/gui/Base.css +++ b/src/main/java/org/jabref/gui/Base.css @@ -1236,3 +1236,7 @@ TextFlow * { .fontsizeSpinner{ -fx-pref-width: 5em; } + +.text-field:invalid { + -fx-background-color: rgba(240, 128, 128, 0.5); +} diff --git a/src/main/java/org/jabref/gui/importer/fetcher/WebSearchPane.java b/src/main/java/org/jabref/gui/importer/fetcher/WebSearchPane.java index a8aaeceda03..f528e92c3cd 100644 --- a/src/main/java/org/jabref/gui/importer/fetcher/WebSearchPane.java +++ b/src/main/java/org/jabref/gui/importer/fetcher/WebSearchPane.java @@ -6,6 +6,7 @@ import javafx.scene.control.Button; import javafx.scene.control.ComboBox; import javafx.scene.control.TextField; +import javafx.scene.control.Tooltip; import javafx.scene.input.KeyCode; import javafx.scene.layout.HBox; import javafx.scene.layout.Priority; @@ -78,17 +79,15 @@ protected Node createContentPane() { TextField query = SearchTextField.create(); query.getStyleClass().add("searchBar"); - // Weird issue: viewModel.queryValidationStatus().not() does not work... - BindingsHelper.includePseudoClassWhen(query, QUERY_INVALID, viewModel.queryValidationStatus().validProperty().not()); - viewModel.queryProperty().bind(query.textProperty()); - TextField queryValid = new TextField(); EasyBind.subscribe(viewModel.queryValidationStatus().validProperty(), valid -> { if (!valid && viewModel.queryValidationStatus().getHighestMessage().isPresent()) { - queryValid.setText(viewModel.queryValidationStatus().getHighestMessage().get().getMessage()); + query.setTooltip(new Tooltip(viewModel.queryValidationStatus().getHighestMessage().get().getMessage())); + query.pseudoClassStateChanged(QUERY_INVALID, true); } else { - queryValid.setText(""); + query.setTooltip(null); + query.pseudoClassStateChanged(QUERY_INVALID, false); } }); @@ -107,7 +106,7 @@ protected Node createContentPane() { // Put everything together VBox container = new VBox(); container.setAlignment(Pos.CENTER); - container.getChildren().addAll(fetcherContainer, query, search, queryValid); + container.getChildren().addAll(fetcherContainer, query, search); return container; } From e3ec6c4b413673e3783ba3a0023abefa1bc15e14 Mon Sep 17 00:00:00 2001 From: Oliver Kopp Date: Mon, 21 Jun 2021 22:33:50 +0200 Subject: [PATCH 13/14] Check for "AND" in query Co-authored-by: Carl Christian Snethlage <50491877+calixtus@users.noreply.github.com> Co-authored-by: Christoph --- .../jabref/gui/importer/fetcher/WebSearchPaneViewModel.java | 6 +++++- src/main/resources/l10n/JabRef_en.properties | 1 + .../gui/importer/fetcher/WebSearchPaneViewModelTest.java | 6 ++++++ 3 files changed, 12 insertions(+), 1 deletion(-) diff --git a/src/main/java/org/jabref/gui/importer/fetcher/WebSearchPaneViewModel.java b/src/main/java/org/jabref/gui/importer/fetcher/WebSearchPaneViewModel.java index 44d4598d39d..bc7502eae5b 100644 --- a/src/main/java/org/jabref/gui/importer/fetcher/WebSearchPaneViewModel.java +++ b/src/main/java/org/jabref/gui/importer/fetcher/WebSearchPaneViewModel.java @@ -79,7 +79,11 @@ public WebSearchPaneViewModel(PreferencesService preferencesService, DialogServi } catch (ParseException e) { String element = e.currentToken.image; int position = e.currentToken.beginColumn; - return ValidationMessage.error(Localization.lang("Invalid query element '%0' at position %1", element, position)); + if (element == null) { + return ValidationMessage.error(Localization.lang("Invalid query. Check position %0.", position)); + } else { + return ValidationMessage.error(Localization.lang("Invalid query element '%0' at position %1", element, position)); + } } catch (QueryNodeParseException e) { return ValidationMessage.error(""); } diff --git a/src/main/resources/l10n/JabRef_en.properties b/src/main/resources/l10n/JabRef_en.properties index 0819ca8205a..41f8eab44dd 100644 --- a/src/main/resources/l10n/JabRef_en.properties +++ b/src/main/resources/l10n/JabRef_en.properties @@ -911,6 +911,7 @@ You\ must\ restart\ JabRef\ for\ this\ to\ come\ into\ effect.=You must restart The\ following\ fetchers\ are\ available\:=The following fetchers are available: Could\ not\ find\ fetcher\ '%0'=Could not find fetcher '%0' Running\ query\ '%0'\ with\ fetcher\ '%1'.=Running query '%0' with fetcher '%1'. +Invalid\ query.\ Check\ position\ %0.=Invalid query. Check position %0. Invalid\ query\ element\ '%0'\ at\ position\ %1=Invalid query element '%0' at position %1 Move\ file=Move file diff --git a/src/test/java/org/jabref/gui/importer/fetcher/WebSearchPaneViewModelTest.java b/src/test/java/org/jabref/gui/importer/fetcher/WebSearchPaneViewModelTest.java index 7c3651385f1..6903ce89cae 100644 --- a/src/test/java/org/jabref/gui/importer/fetcher/WebSearchPaneViewModelTest.java +++ b/src/test/java/org/jabref/gui/importer/fetcher/WebSearchPaneViewModelTest.java @@ -28,6 +28,12 @@ void setUp() { viewModel = new WebSearchPaneViewModel(preferencesService, dialogService, stateManager); } + @Test + void queryConsistingOfASingleAndIsNotValid() { + viewModel.queryProperty().setValue("AND"); + assertFalse(viewModel.queryValidationStatus().validProperty().getValue()); + } + @Test void falseQueryValidationStatus() { viewModel.queryProperty().setValue("Miami !Beach AND OR Blue"); From b3100c0ebf00e45c2cd91ff94eac699a7f9ba86f Mon Sep 17 00:00:00 2001 From: Oliver Kopp Date: Mon, 21 Jun 2021 22:37:24 +0200 Subject: [PATCH 14/14] =?UTF-8?q?I=20=E2=9D=A4=20checkstyle?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- src/main/java/org/jabref/gui/importer/fetcher/WebSearchPane.java | 1 - 1 file changed, 1 deletion(-) diff --git a/src/main/java/org/jabref/gui/importer/fetcher/WebSearchPane.java b/src/main/java/org/jabref/gui/importer/fetcher/WebSearchPane.java index f528e92c3cd..0d26794d691 100644 --- a/src/main/java/org/jabref/gui/importer/fetcher/WebSearchPane.java +++ b/src/main/java/org/jabref/gui/importer/fetcher/WebSearchPane.java @@ -24,7 +24,6 @@ import org.jabref.gui.help.HelpAction; import org.jabref.gui.icon.IconTheme; import org.jabref.gui.search.SearchTextField; -import org.jabref.gui.util.BindingsHelper; import org.jabref.gui.util.ViewModelListCellFactory; import org.jabref.logic.importer.SearchBasedFetcher; import org.jabref.logic.l10n.Localization;