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

Add query validation for web search #7809

Merged
merged 18 commits into from
Jun 21, 2021
Merged

Add query validation for web search #7809

merged 18 commits into from
Jun 21, 2021

Conversation

koppor
Copy link
Member

@koppor koppor commented Jun 8, 2021

Fixes #7411
Refs #7575

CHANGELOG not required, because last released version contained a query validator (see #7391 (comment)). We removed it, because we switched to a general lucene query syntax in #7350

  • Change in CHANGELOG.md described in a way that is understandable for the average user (if applicable)
  • Tests created for changes (if applicable)
  • Manually tested changed features in running JabRef (always required)
  • Screenshots added in PR description (for UI changes)
  • Checked documentation: Is the information available and up to date? If not created an issue at https://github.com/JabRef/user-documentation/issues or, even better, submitted a pull request to the documentation repository.

koppor and others added 4 commits June 8, 2021 23:03
Co-authored-by: Carl Christian Snethlage <50491877+calixtus@users.noreply.github.com>
@koppor koppor added the status: ready-for-review Pull Requests that are ready to be reviewed by the maintainers label Jun 8, 2021
koppor and others added 3 commits June 9, 2021 00:01
Co-authored-by: Carl Christian Snethlage <50491877+calixtus@users.noreply.github.com>
@koppor koppor removed the status: ready-for-review Pull Requests that are ready to be reviewed by the maintainers label Jun 9, 2021
@koppor koppor marked this pull request as draft June 9, 2021 05:27
@koppor
Copy link
Member Author

koppor commented Jun 9, 2021

Commit 1adbc27 (#7809) partially fixes #7277.

We have the issue that the validation message itself is not shown. @calixtus can you have a look?

@calixtus
Copy link
Member

Seems like something with the visualization is not right. Very strange, since inverse it works...

grafik

grafik

grafik

@koppor
Copy link
Member Author

koppor commented Jun 18, 2021

We need to show the error message in a text field (which is shown if an error message is available)

koppor and others added 2 commits June 21, 2021 22:29
Co-authored-by: Carl Christian Snethlage <50491877+calixtus@users.noreply.github.com>
Co-authored-by: Christoph <siedlerkiller@gmail.com>
Co-authored-by: Carl Christian Snethlage <50491877+calixtus@users.noreply.github.com>
Co-authored-by: Christoph <siedlerkiller@gmail.com>
@Siedlerchr Siedlerchr marked this pull request as ready for review June 21, 2021 20:35
@Siedlerchr Siedlerchr added the status: ready-for-review Pull Requests that are ready to be reviewed by the maintainers label Jun 21, 2021
@calixtus calixtus merged commit bf09514 into main Jun 21, 2021
@calixtus calixtus deleted the fix-7411 branch June 21, 2021 20:48
@koppor koppor removed the status: ready-for-review Pull Requests that are ready to be reviewed by the maintainers label Jun 21, 2021
if (stateManager.getActiveDatabase().isEmpty()) {
dialogService.notify(Localization.lang("Please open or start a new library before searching"));
return;
}

SearchBasedFetcher activeFetcher = getSelectedFetcher();
Globals.getTelemetryClient().ifPresent(client -> client.trackEvent("search", Map.of("fetcher", activeFetcher.getName()), Map.of()));
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

here as well, and please no globals...

@calixtus
Copy link
Member

I've got a small cleanup branch for the preferences dialog in the pipeline, that will be made a PR in the next days addressing the remaining issues.

Siedlerchr added a commit that referenced this pull request Jun 22, 2021
* upstream/main:
  New Crowdin updates (#7843)
  Refactoring and addition of unit tests (#7597)
  CLI option to write XMP metadata to pdfs (#7814)
  Add query validation for web search (#7809)
  change eclipse default output dir (#7842)
  Bump lucene-queryparser from 8.8.2 to 8.9.0 (#7835)
  Bump libreoffice from 7.1.3 to 7.1.4 (#7836)
  Bump postgresql from 42.2.21 to 42.2.22 (#7839)
  Bump org.eclipse.jgit (#7838)
  Bump byte-buddy-parent from 1.11.2 to 1.11.5 (#7837)
  Bump unoloader from 7.1.3 to 7.1.4 (#7841)
  Ms Office Export patent author as inventor (#7831)
  Abbreviation toggle within the JournalEditorViewModel now ignores curly braces (issue #7773) (#7807)
Siedlerchr added a commit that referenced this pull request Jun 30, 2021
* upstream/main: (26 commits)
  Add unit test to four test classes (#7651)
  Fix IEEE test (#7852)
  New Crowdin updates (#7859)
  Fix markdown syntax of ADRs
  add missing l10n (#7857)
  New Crowdin updates (#7847)
  Bump mockito-core from 3.11.1 to 3.11.2 (#7856)
  Bump checkstyle from 8.43 to 8.44 (#7855)
  Fix for issue #4652: Add Find Unlinked Files Filter based on Date (#7846)
  Fix for entering a backslash in the custom entry preview dialog (#7851)
  Fixed INSPIREFetcherTest
  Fixed TitleFetcherTest
  Ignore baeldung.com and tldrlegal.com from out link checks
  New Crowdin updates (#7845)
  New Crowdin updates (#7843)
  Refactoring and addition of unit tests (#7597)
  CLI option to write XMP metadata to pdfs (#7814)
  Add query validation for web search (#7809)
  change eclipse default output dir (#7842)
  Bump lucene-queryparser from 8.8.2 to 8.9.0 (#7835)
  ...
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add best query syntax validation
4 participants