Skip to content

Commit

Permalink
Remove {{BEFORE_END_HEAD_TAG_HOOK}} from Base.html (oppia#6835)
Browse files Browse the repository at this point in the history
* remove before end head tag hook

* work on lint

* work on review changes and some refactoring

* work on failing test

* address review comments

* failing tests

* work on backend tests

* address review comments
  • Loading branch information
jameesjohn authored and vojtechjelinek committed Jun 5, 2019
1 parent add9318 commit 56f26d2
Show file tree
Hide file tree
Showing 8 changed files with 74 additions and 41 deletions.
38 changes: 10 additions & 28 deletions core/controllers/admin_test.py
Original file line number Diff line number Diff line change
Expand Up @@ -14,8 +14,8 @@

"""Tests for the admin page."""

from core.controllers import base
from core.domain import collection_services
from core.domain import config_domain
from core.domain import exp_domain
from core.domain import exp_services
from core.domain import search_services
Expand Down Expand Up @@ -58,51 +58,33 @@ def test_change_configuration_property(self):
self.login(self.ADMIN_EMAIL, is_super_admin=True)
response = self.get_html_response('/admin')
csrf_token = self.get_csrf_token_from_response(response)
new_config_value = True

response_dict = self.get_json('/adminhandler')
response_config_properties = response_dict['config_properties']
self.assertDictContainsSubset({
'value': '',
}, response_config_properties[base.BEFORE_END_HEAD_TAG_HOOK.name])
'value': False,
}, response_config_properties[
config_domain.IS_IMPROVEMENTS_TAB_ENABLED.name])

payload = {
'action': 'save_config_properties',
'new_config_property_values': {
base.BEFORE_END_HEAD_TAG_HOOK.name: (
self.UNICODE_TEST_STRING),
config_domain.IS_IMPROVEMENTS_TAB_ENABLED.name: (
new_config_value),
}
}
self.post_json('/adminhandler', payload, csrf_token=csrf_token)

response_dict = self.get_json('/adminhandler')
response_config_properties = response_dict['config_properties']
self.assertDictContainsSubset({
'value': self.UNICODE_TEST_STRING,
}, response_config_properties[base.BEFORE_END_HEAD_TAG_HOOK.name])
'value': new_config_value,
}, response_config_properties[
config_domain.IS_IMPROVEMENTS_TAB_ENABLED.name])

self.logout()

def test_change_about_page_config_property(self):
"""Test that config property values are changed correctly."""
new_config_value = 'new_config_value'

response = self.get_html_response('/about')
self.assertNotIn(new_config_value, response.body)

self.login(self.ADMIN_EMAIL, is_super_admin=True)
response = self.get_html_response('/admin')
csrf_token = self.get_csrf_token_from_response(response)
self.post_json(
'/adminhandler', {
'action': 'save_config_properties',
'new_config_property_values': {
base.BEFORE_END_HEAD_TAG_HOOK.name: new_config_value
}
}, csrf_token=csrf_token)

response = self.get_html_response('/about')
self.assertIn(new_config_value, response.body)


class GenerateDummyExplorationsTest(test_utils.GenericTestBase):
"""Test the conditions for generation of dummy explorations."""
Expand Down
26 changes: 15 additions & 11 deletions core/controllers/base.py
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,6 @@
import utils

from google.appengine.api import users
import jinja2
import webapp2

app_identity_services = models.Registry.import_app_identity_services()
Expand All @@ -50,14 +49,6 @@
'oppia_csrf_secret', {'type': 'unicode'},
'Text used to encrypt CSRF tokens.', DEFAULT_CSRF_SECRET)

BEFORE_END_HEAD_TAG_HOOK = config_domain.ConfigProperty(
'before_end_head_tag_hook', {
'type': 'unicode',
'ui_config': {
'rows': 7,
},
},
'Code to insert just before the closing </head> tag in all pages.', '')


def _clear_login_cookies(response_headers):
Expand Down Expand Up @@ -308,8 +299,6 @@ def render_template(self, filepath, iframe_restriction='DENY'):
scheme, netloc, path, _, _ = urlparse.urlsplit(self.request.uri)

values.update({
'BEFORE_END_HEAD_TAG_HOOK': jinja2.utils.Markup(
BEFORE_END_HEAD_TAG_HOOK.value),
'DEV_MODE': constants.DEV_MODE,
'DOMAIN_URL': '%s://%s' % (scheme, netloc),
'ACTIVITY_STATUS_PRIVATE': (
Expand Down Expand Up @@ -586,3 +575,18 @@ def is_csrf_token_valid(cls, user_id, token):
return False
except Exception:
return False


class GoogleAnalyticsHandler(BaseHandler):
"""Returns Google Analytics variables."""

GET_HANDLER_ERROR_RETURN_TYPE = feconf.HANDLER_TYPE_JSON

def get(self):
"""Respond to a get request for the variables."""
google_analytics_data = {
'analytics_id': feconf.ANALYTICS_ID,
'site_name_for_analytics': feconf.SITE_NAME_FOR_ANALYTICS
}

self.render_json(google_analytics_data)
19 changes: 18 additions & 1 deletion core/controllers/base_test.py
Original file line number Diff line number Diff line change
Expand Up @@ -671,7 +671,8 @@ def test_every_method_has_decorator(self):
# Following handler are present in base.py where acl_decorators
# cannot be imported.
if (handler.__name__ == 'LogoutPage' or
handler.__name__ == 'Error404Handler'):
handler.__name__ == 'Error404Handler' or
handler.__name__ == 'GoogleAnalyticsHandler'):
continue

if handler.get != base.BaseHandler.get:
Expand Down Expand Up @@ -888,3 +889,19 @@ def test_no_error_is_raised_on_opening_new_tab_after_signup(self):
)

self.get_html_response('/about')


class GoogleAnalyticsHandlerTests(test_utils.GenericTestBase):

def test_analytics_handler_returns_required_values(self):

with self.swap(feconf, 'ANALYTICS_ID', 'test123'):
with self.swap(
feconf, 'SITE_NAME_FOR_ANALYTICS', 'Oppia foundation'):
response = self.get_json(feconf.GOOGLE_ANALYTICS_DATA_URL)
self.assertEqual(
response, {
'analytics_id': 'test123',
'site_name_for_analytics': 'Oppia foundation'
}
)
1 change: 1 addition & 0 deletions core/domain/config_domain.py
Original file line number Diff line number Diff line change
Expand Up @@ -79,6 +79,7 @@ class ConfigProperty(object):
- banned_usernames.
- banner_alt_text.
- before_end_body_tag_hook.
- before_end_head_tag_hook.
- carousel_slides_config.
- collection_editor_whitelist.
- contact_email_address.
Expand Down
1 change: 0 additions & 1 deletion core/templates/dev/head/pages/base.html
Original file line number Diff line number Diff line change
Expand Up @@ -91,7 +91,6 @@
{% include 'pages/header_js_libs.html' %}
{% endblock header_js %}

{{BEFORE_END_HEAD_TAG_HOOK}}
</head>

<body>
Expand Down
21 changes: 21 additions & 0 deletions core/templates/dev/head/pages/header_js_libs.html
Original file line number Diff line number Diff line change
Expand Up @@ -13,3 +13,24 @@
<script src="/third_party/static/headroom-js-0.9.4/headroom.min.js"></script>
<script src="/third_party/static/headroom-js-0.9.4/angular.headroom.min.js"></script>
<script src="/third_party/static/angular-drag-and-drop-lists-2.1.0/angular-drag-and-drop-lists.min.js"></script>

<script>
(function(i,s,o,g,r,a,m){i['GoogleAnalyticsObject']=r;i[r]=i[r]||function(){
(i[r].q=i[r].q||[]).push(arguments)},i[r].l=1*new Date();a=s.createElement(o),
m=s.getElementsByTagName(o)[0];a.async=1;a.src=g;m.parentNode.insertBefore(a,m)
})(window,document,'script','https://www.google-analytics.com/analytics.js','ga');

(function() {
fetch('/google_analytics').then(function(response) {
return response.text()
}).then(function(response) {
response = JSON.parse(response.substring(4));
ga('create', response.analytics_id, 'auto', {allowLinker: true});
ga('set', 'anonymizeIp', true);
ga('set', 'forceSSL', true);
ga('require', 'linker');
ga('linker:autoLink', [response.site_name_for_analytics]);
ga('send', 'pageview');
})
})();
</script>
5 changes: 5 additions & 0 deletions feconf.py
Original file line number Diff line number Diff line change
Expand Up @@ -612,6 +612,7 @@ def get_empty_ratings():
FEEDBACK_THREAD_VIEW_EVENT_URL = '/feedbackhandler/thread_view_event'
FLAG_EXPLORATION_URL_PREFIX = '/flagexplorationhandler'
FRACTIONS_LANDING_PAGE_URL = '/fractions'
GOOGLE_ANALYTICS_DATA_URL = '/google_analytics'
TOPIC_LANDING_PAGE_URL = '/learn/<subject>/<topic>'
LEARNER_DASHBOARD_URL = '/learner_dashboard'
LEARNER_DASHBOARD_DATA_URL = '/learnerdashboardhandler/data'
Expand Down Expand Up @@ -952,3 +953,7 @@ def get_empty_ratings():
AVAILABLE_LANDING_PAGES = {
'maths': ['fractions', 'ratios']
}

# Data required for Google Analytics.
ANALYTICS_ID = ''
SITE_NAME_FOR_ANALYTICS = ''
4 changes: 4 additions & 0 deletions main.py
Original file line number Diff line number Diff line change
Expand Up @@ -665,6 +665,10 @@ def ui_access_wrapper(self, *args, **kwargs):
get_redirect_route(
r'/resolveissuehandler/<exploration_id>', editor.ResolveIssueHandler),

get_redirect_route(
r'%s' % feconf.GOOGLE_ANALYTICS_DATA_URL,
base.GoogleAnalyticsHandler),

# 404 error handler.
get_redirect_route(r'/<:.*>', base.Error404Handler),
]
Expand Down

0 comments on commit 56f26d2

Please sign in to comment.