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.

Sign up for GitHub

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

Jump to bottom

[Fix #4265] Porting frontend docsearch to work with new API #4340

Merged
merged 4 commits into from Jul 16, 2018
Merged

[Fix #4265] Porting frontend docsearch to work with new API #4340

Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
7c682a6
Porting frontend docsearch to work with new API
safwanrahman Jul 8, 2018
30f7e74
fixup lint
safwanrahman Jul 8, 2018
1fa4db6
renaming document to doc`
safwanrahman Jul 9, 2018
46d72f9
remove $ sign
safwanrahman Jul 9, 2018
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
71 changes: 35 additions & 36 deletions readthedocs/core/static-src/core/js/doc-embed/search.js
View file
Open in desktop
Expand Up @@ -23,44 +23,44 @@ function attach_elastic_search_query(data) {

search_url.href = api_host;
search_url.pathname = '/api/v2/docsearch/';
search_url.search = '?q=' + $.urlencode(query) + '&project=' + project +
'&version=' + version + '&language=' + language;
search_url.search = '?query=' + $.urlencode(query) + '&project=' + project +
'&version=' + version + '&language=' + language;

search_def
.then(function (results) {
var hits = results.hits || {};
var hit_list = hits.hits || [];
.then(function (data) {
var hit_list = data.results || [];
var total_count = data.count || 0;

if (hit_list.length) {
for (var n in hit_list) {
var hit = hit_list[n];
var fields = hit.fields || {};
for (var i = 0; i < hit_list.length; i += 1) {
var doc = hit_list[i];
var highlight = doc.highlight;
var list_item = $('<li style="display: none;"></li>');
var item_url = document.createElement('a');
var highlight = hit.highlight;

item_url.href += fields.link +
DOCUMENTATION_OPTIONS.FILE_SUFFIX;
item_url.search = '?highlight=' + $.urlencode(query);

// Result list elements
list_item.append(
$('<a />')
.attr('href', item_url)
.html(fields.title)
);
// fields.project is returned as an array
if (fields.project.indexOf(project) === -1) {
list_item.append(
$('<span>')
.text(" (from project " + fields.project + ")")
);

// Creating the result from elements
var link = doc.link + DOCUMENTATION_OPTIONS.FILE_SUFFIX +
'?highlight=' + $.urlencode(query);

var item = $('<a>', {'href': link});
item.html(doc.title);
list_item.append(item);

// If the document is from subproject, add extra information
if (doc.project !== project) {
var text = " (from project " + doc.project + ")";
var extra = $('<span>', {'text': text});

list_item.append(extra);
}
if (highlight.content.length) {
var content = $('<div class="context">')
.html(xss(highlight.content[0]));
content.find('em').addClass('highlighted');
list_item.append(content);

// Show highlighted texts
if (highlight.content) {
Copy link
Member

Choose a reason for hiding this comment

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

Do we not need to check length here? I don't know JS well enough, but that's what we were doing before, so probably worth keeping it explicit.

Copy link
Member Author

Choose a reason for hiding this comment

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

In the previous API, the content was provided as a blank list if there are no match in content. but our current API do not provide anything in the content key if there are not anything in the content. So checking the length may raise error.

var content_text = xss(highlight.content[0]);
var contents = $('<div class="context">');

contents.html(content_text);
contents.find('em').addClass('highlighted');
list_item.append(contents);
}

Search.output.append(list_item);
Expand All @@ -74,7 +74,7 @@ function attach_elastic_search_query(data) {
}
else {
Search.status.text(
_('Search finished, found %s page(s) matching the search query.').replace('%s', hit_list.length)
_('Search finished, found %s page(s) matching the search query.').replace('%s', total_count)
);
}
})
Expand All @@ -96,11 +96,10 @@ function attach_elastic_search_query(data) {
withCredentials: true,
},
complete: function (resp, status_code) {
if (typeof (resp.responseJSON) === 'undefined' ||
typeof (resp.responseJSON.results) === 'undefined') {
if (status_code !== 'success' || resp.responseJSON.count === 0) {
return search_def.reject();
}
return search_def.resolve(resp.responseJSON.results);
return search_def.resolve(resp.responseJSON);
}
})
.error(function (resp, status_code, error) {
Expand Down