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

The API search filtered all notes when showCompletedTodos is set to false #5007

Closed
JackGruber opened this issue May 24, 2021 · 4 comments · Fixed by #5066
Closed

The API search filtered all notes when showCompletedTodos is set to false #5007

JackGruber opened this issue May 24, 2021 · 4 comments · Fixed by #5066
Labels
api Joplin API bug It's a bug high High priority issues

Comments

@JackGruber
Copy link
Contributor

JackGruber commented May 24, 2021

When searching for notes via the API and the showCompletedTodos is set to false and the fields is_todo and todo_completed should not be returned then all notes are filternd.

overviewNotes = await joplin.data.get(["search"], {
  query: "joplin",
  fields: "id, title, body",
});

Environment

Joplin 2.0.1 (dev, win32)

Client ID: 3786010df4424355b5ac92d208168602
Sync Version: 2
Profile Version: 37
Keychain Supported: Yes

Revision: 68b5169 (test)

Steps to reproduce

  1. Set Show completed to-dos to disable
  2. Run a search query in a plugin or API without the todo fields
    http://localhost:27583/search?query=joplin&type=note&token=
  3. No search results returned
  4. Set Show completed to-dos to enable
  5. run query again and the search results are returned

Describe what you expected to happen

  1. The API search does not consider the option showCompletedTodos and always returns all matches
    or
  2. The API search does consider the option showCompletedTodos and filterd the notes accordingly

@laurent22 what do you think is the best option for the API search to handel this behavior?
I think for the API is almost the option 1 the best?

The behavior occurs since #4951

filteredNotes = sortedNotes.filter(note => note.is_todo === 0 || (note.is_todo === 1 && note.todo_completed === 0));

@JackGruber JackGruber added the bug It's a bug label May 24, 2021
@laurent22 laurent22 added api Joplin API high High priority issues labels May 24, 2021
@laurent22
Copy link
Owner

Hmm, in fact what's returned by the API should not be affected by the settings, so I think it was a mistake to add this "showCompletedTodos" condition to SearchEngineUtils. Or maybe it's a mistake to use SearchEngineUtils in routes/search.ts

Actually we have a filter iscomplete:0/1, so is it not sufficient to filter based on whether a to-do is completed or not? Why do we need this support for showCompletedTodos in search?

@JackGruber
Copy link
Contributor Author

JackGruber commented May 27, 2021

The PR was not meant to filter notes based on the option, but to hide the notes after a search.

In the BaseApplication.ts and refreshSearch.js the SearchEngineUtils.notesForQuery is used for selecting the notes.
Therefore I thought this function is only used for displaying the notes, but the other place this function is used is routes/search.ts.

@laurent22
Copy link
Owner

Right, on the UI side it does make sense. So I guess the bug could be that it's used in routes/search.ts. Maybe some option could be added like "applyUserSettings", and if "false" we don't filter based on "showCompletedTodos". Would that fix the original issue?

@JackGruber
Copy link
Contributor Author

Yes, I prepare fix.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
api Joplin API bug It's a bug high High priority issues
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants