Skip to content

Commit

Permalink
Remove punctuation from search queries, and escape the query string c…
Browse files Browse the repository at this point in the history
…orrectly.
  • Loading branch information
seanlip committed Apr 28, 2016
1 parent 86405ad commit a6f2635
Show file tree
Hide file tree
Showing 10 changed files with 50 additions and 39 deletions.
5 changes: 0 additions & 5 deletions core/controllers/base.py
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,6 @@
import sys
import time
import traceback
import urllib
import urlparse

import jinja2
Expand Down Expand Up @@ -315,10 +314,6 @@ def __init__(self, request, response): # pylint: disable=super-init-not-called
else:
self.payload = None

def unescape_state_name(self, escaped_state_name):
"""Unescape a state name that is encoded with encodeURIComponent."""
return urllib.unquote(escaped_state_name).decode('utf-8')

def dispatch(self):
"""Overrides dispatch method in webapp2 superclass."""
# If the request is to the old demo server, redirect it permanently to
Expand Down
6 changes: 3 additions & 3 deletions core/controllers/editor.py
Original file line number Diff line number Diff line change
Expand Up @@ -142,7 +142,7 @@ def test_editor(self, exploration_id, escaped_state_name=None, **kwargs):
if not escaped_state_name:
return handler(self, exploration_id, **kwargs)

state_name = self.unescape_state_name(escaped_state_name)
state_name = utils.unescape_encoded_uri_component(escaped_state_name)
if state_name not in exploration.states:
logging.error('Could not find state: %s' % state_name)
logging.error('Available states: %s' % exploration.states.keys())
Expand Down Expand Up @@ -579,7 +579,7 @@ def get(self, exploration_id, escaped_state_name):
except:
raise self.PageNotFoundException

state_name = self.unescape_state_name(escaped_state_name)
state_name = utils.unescape_encoded_uri_component(escaped_state_name)
if state_name not in exploration.states:
# If trying to access a non-existing state, there is no training
# data associated with it.
Expand Down Expand Up @@ -821,7 +821,7 @@ def get(self, exploration_id, escaped_state_name):
except:
raise self.PageNotFoundException

state_name = self.unescape_state_name(escaped_state_name)
state_name = utils.unescape_encoded_uri_component(escaped_state_name)
if state_name not in exploration.states:
logging.error('Could not find state: %s' % state_name)
logging.error('Available states: %s' % exploration.states.keys())
Expand Down
25 changes: 18 additions & 7 deletions core/controllers/galleries.py
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@

import json
import logging
import string

from core.controllers import base
from core.domain import collection_domain
Expand Down Expand Up @@ -79,7 +80,7 @@ def get_matching_exploration_dicts(query_string, search_cursor):
"""Given a query string and a search cursor, returns a list of exploration
dicts that satisfy the search query.
"""
exp_ids, search_cursor = (
exp_ids, new_search_cursor = (
exp_services.get_exploration_ids_matching_query(
query_string, cursor=search_cursor))

Expand All @@ -92,7 +93,7 @@ def get_matching_exploration_dicts(query_string, search_cursor):
'%s explorations were fetched to load the gallery page. '
'You may be running up against the default query limits.'
% feconf.DEFAULT_QUERY_LIMIT)
return explorations_list
return explorations_list, new_search_cursor


class GalleryPage(base.BaseHandler):
Expand All @@ -115,6 +116,7 @@ def get(self):
})
self.render_template('galleries/gallery.html')


class GallerySearchPage(base.BaseHandler):
"""The exploration gallery search page."""

Expand Down Expand Up @@ -163,25 +165,34 @@ def get(self):
self.render_json(self.values)


class GalleryHandler(base.BaseHandler):
"""Provides data for the exploration gallery page."""
class SearchHandler(base.BaseHandler):
"""Provides data for exploration search results."""

def get(self):
"""Handles GET requests."""
query_string = self.request.get('q')
query_string = utils.unescape_encoded_uri_component(
self.request.get('q'))

# Remove all punctuation from the query string, and replace it with
# spaces. See http://stackoverflow.com/a/266162 and
# http://stackoverflow.com/a/11693937
remove_punctuation_map = dict(
(ord(char), None) for char in string.punctuation)
query_string = query_string.translate(remove_punctuation_map)

if self.request.get('category'):
query_string += ' category=%s' % self.request.get('category')
if self.request.get('language_code'):
query_string += ' language_code=%s' % self.request.get(
'language_code')
search_cursor = self.request.get('cursor', None)

explorations_list = get_matching_exploration_dicts(
explorations_list, new_search_cursor = get_matching_exploration_dicts(
query_string, search_cursor)

self.values.update({
'explorations_list': explorations_list,
'search_cursor': search_cursor,
'search_cursor': new_search_cursor,
})

self.render_json(self.values)
Expand Down
2 changes: 0 additions & 2 deletions core/templates/dev/head/components/SearchResultsDirective.js
Original file line number Diff line number Diff line change
Expand Up @@ -16,8 +16,6 @@
* @fileoverview Data and controllers for the Oppia gallery search page.
*/

oppia.constant('GALLERY_DATA_URL', '/galleryhandler/data');

oppia.directive('searchResults', [function() {
return {
restrict: 'E',
Expand Down
10 changes: 4 additions & 6 deletions core/templates/dev/head/galleries/Gallery.js
Original file line number Diff line number Diff line change
Expand Up @@ -32,18 +32,16 @@ angular.module('template/carousel/carousel.html', []).run([
'</div>\n');
}]);

oppia.constant('GALLERY_DATA_URL', '/galleryhandler/data');

oppia.controller('Gallery', [
'$scope', '$http', '$rootScope', '$modal', '$window', '$timeout',
'ExplorationCreationButtonService', 'oppiaDatetimeFormatter',
'oppiaDebouncer', 'urlService', 'GALLERY_DATA_URL', 'CATEGORY_LIST',
'searchService', 'siteAnalyticsService',
'oppiaDebouncer', 'urlService', 'CATEGORY_LIST', 'searchService',
'siteAnalyticsService',
function(
$scope, $http, $rootScope, $modal, $window, $timeout,
ExplorationCreationButtonService, oppiaDatetimeFormatter,
oppiaDebouncer, urlService, GALLERY_DATA_URL, CATEGORY_LIST,
searchService, siteAnalyticsService) {
oppiaDebouncer, urlService, CATEGORY_LIST, searchService,
siteAnalyticsService) {
$rootScope.loadingMessage = 'Loading';

// Below is the width of each tile (width + margins), which can be found
Expand Down
2 changes: 1 addition & 1 deletion core/templates/dev/head/galleries/GallerySpec.js
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@ describe('Gallery controller', function() {

beforeEach(inject(function(_$httpBackend_, $rootScope, $controller) {
$httpBackend = _$httpBackend_;
$httpBackend.expectGET('/galleryhandler/data').respond({
$httpBackend.expectGET('/searchhandler/data').respond({
allow_yaml_file_upload: false,
explorations_list: [{
id: '3',
Expand Down
28 changes: 16 additions & 12 deletions core/templates/dev/head/services/searchService.js
Original file line number Diff line number Diff line change
Expand Up @@ -16,11 +16,11 @@
* @fileoverview search service for activityTilesInfinityGrid
*/

oppia.constant('GALLERY_DATA_URL', '/galleryhandler/data');
oppia.constant('SEARCH_DATA_URL', '/searchhandler/data');

oppia.factory('searchService', [
'$http', '$rootScope', 'GALLERY_DATA_URL',
function($http, $rootScope, GALLERY_DATA_URL) {
'$http', '$rootScope', 'SEARCH_DATA_URL',
function($http, $rootScope, SEARCH_DATA_URL) {
var _lastQuery = null;
var _lastSelectedCategories = {};
var _lastSelectedLanguageCodes = {};
Expand Down Expand Up @@ -86,20 +86,23 @@ oppia.factory('searchService', [
var _isCurrentlyFetchingResults = false;
var numSearchesInProgress = 0;

var getQueryUrl = function(searchUrlQueryString) {
return SEARCH_DATA_URL + '?q=' + searchUrlQueryString;
};

return {
getSearchUrlQueryString: function(searchQuery, selectedCategories,
selectedLanguageCodes) {
return encodeURI(searchQuery) +
return encodeURIComponent(searchQuery) +
_getSuffixForQuery(selectedCategories, selectedLanguageCodes);
},
// Note that an empty query results in all explorations being shown.
executeSearchQuery: function(
searchQuery, selectedCategories, selectedLanguageCodes,
successCallback) {
console.log(searchQuery);
var queryUrl = GALLERY_DATA_URL + '?q=' + this.getSearchUrlQueryString(
searchQuery, selectedCategories, selectedLanguageCodes
);
var queryUrl = getQueryUrl(
this.getSearchUrlQueryString(
searchQuery, selectedCategories, selectedLanguageCodes));

_isCurrentlyFetchingResults = true;
numSearchesInProgress++;
Expand Down Expand Up @@ -133,7 +136,8 @@ oppia.factory('searchService', [
},
// The following takes in the url search component as an argument and the
// selectionDetails. It will update selectionDetails with the relevant
// fields that were extracted from the url. Returns the search query.
// fields that were extracted from the url. It returns the unencoded search
// query string.
updateSearchFieldsBasedOnUrlQuery: function(urlComponent,
selectionDetails) {
var urlQuery = urlComponent.substring('?q='.length);
Expand Down Expand Up @@ -172,9 +176,9 @@ oppia.factory('searchService', [
return;
}

var queryUrl = GALLERY_DATA_URL + '?q=' + this.getSearchUrlQueryString(
_lastQuery, _lastSelectedCategories, _lastSelectedLanguageCodes
);
var queryUrl = getQueryUrl(
this.getSearchUrlQueryString(
_lastQuery, _lastSelectedCategories, _lastSelectedLanguageCodes));

if (_searchCursor) {
queryUrl += '&cursor=' + _searchCursor;
Expand Down
2 changes: 1 addition & 1 deletion feconf.py
Original file line number Diff line number Diff line change
Expand Up @@ -361,7 +361,7 @@ def get_empty_ratings():
FEEDBACK_STATS_URL_PREFIX = '/feedbackstatshandler'
FEEDBACK_THREAD_URL_PREFIX = '/threadhandler'
FEEDBACK_THREADLIST_URL_PREFIX = '/threadlisthandler'
GALLERY_DATA_URL = '/galleryhandler/data'
GALLERY_SEARCH_DATA_URL = '/searchhandler/data'
GALLERY_SEARCH_URL = '/search/find'
GALLERY_URL = '/gallery'
LEARN_GALLERY_URL = '/learn'
Expand Down
4 changes: 2 additions & 2 deletions main.py
Original file line number Diff line number Diff line change
Expand Up @@ -187,8 +187,8 @@ def ui_access_wrapper(self, *args, **kwargs):
get_redirect_route(
r'%s' % feconf.GALLERY_URL, galleries.GalleryPage, 'gallery_page'),
get_redirect_route(
r'%s' % feconf.GALLERY_DATA_URL, galleries.GalleryHandler,
'gallery_handler'),
r'%s' % feconf.GALLERY_SEARCH_DATA_URL, galleries.SearchHandler,
'gallery_search_handler'),
get_redirect_route(
r'%s' % feconf.GALLERY_SEARCH_URL,
galleries.GallerySearchPage, 'gallery_search'),
Expand Down
5 changes: 5 additions & 0 deletions utils.py
Original file line number Diff line number Diff line change
Expand Up @@ -445,3 +445,8 @@ def get_all_language_codes_and_names():
'code': lc['code'],
'name': _get_short_language_description(lc['description']),
} for lc in feconf.ALL_LANGUAGE_CODES]


def unescape_encoded_uri_component(escaped_string):
"""Unescape a string that is encoded with encodeURIComponent."""
return urllib.unquote(escaped_string).decode('utf-8')

0 comments on commit a6f2635

Please sign in to comment.