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

MBS-13035: Better "no results" message for CDTOC release search #2916

Draft
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

reosarevok
Copy link
Member

Implement MBS-13035

Problem

The release search when trying to attach a CDTOC is limited to the 300 releases best matching the entered name and only from those does it look for mediums with the right number of tracks. This can be confusing: see MBS-12971 where the editor knew the release existed, and did not understand why the search was not returning it.

Solution

This changes the message to a more concrete one that specifies users should be as specific as possible, and also directs them to an indexed search they can refine if needed.

Testing

Manually to ensure the search link works properly.

The search is limited to the 300 releases best matching the entered name
and only from those does it look for mediums with the right
number of tracks. This can be confusing (MBS-12971).
The new message specifies that users should be as specific as possible,
and also directs them to an indexed search they can refine if needed.
@reosarevok reosarevok added QoL Non-urgent quality of life improvements Frontend-only PRs that only change display components labels Apr 19, 2023
Copy link
Contributor

@yvanzo yvanzo left a comment

Choose a reason for hiding this comment

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

This is conflicting with your own (currently draft but almost finished) pull request #2642 which converts this template to React.

It might also be simpler to create an advanced search link with escaped title using JavaScript as proposed in comment.

Comment on lines -64 to +66
l('No results found. Try refining your search query.');
l('No results found. For performance issues, this search checks only a limited amount of releases closest to your entered query. To ensure a better result, search for the full release title or as close to it as possible. You can also {search_link|search manually} and paste the link to the release here.', { search_link => { href => search_link, target => '_blank' } });
Copy link
Contributor

Choose a reason for hiding this comment

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

For readability and to limit i18n breakage and redundancy, I suggest to keep the initial paragraph (for the short answer), and to add the new text in a separate paragraph (for the long answer).

About the added text itself now:

  1. The first added sentence doesn’t really makes clear that the title is searched at first, and that other search conditions are tested on these limited amount of intermediate results.
  2. The second added sentence is more about trying to work around the explained limitation than actually ensuring a better result.
  3. The third and last added sentence provides a link for a search for release by name. It is not clearly worded. Would it be possible to pass some other conditions such as CD format and whatnot? See for example: https://musicbrainz.org/search?query=release%3A%28%22Never+Gonna+Give+You+Up%22%29+AND+format%3ACD&type=release&limit=25&method=advanced

Copy link
Member Author

Choose a reason for hiding this comment

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

For 3. we could, but the system doesn't map entirely great right now, because a) we don't always know it's a CD (other formats also support discids, including no format set) and b) the main thing we could use to search (number of tracks) clashes because we want number of cdtoc tracks (without data tracks) but indexed search just has total number :)

@reosarevok reosarevok marked this pull request as draft May 17, 2023 17:37
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Frontend-only PRs that only change display components QoL Non-urgent quality of life improvements
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants