Skip to content
Merged
Show file tree
Hide file tree
Changes from 17 commits
Commits
Show all changes
26 commits
Select commit Hold shift + click to select a range
21ddede
feat: added banner message for survey report
Asespinel Oct 31, 2023
d02d5f1
refactor: changed variable names
Asespinel Nov 1, 2023
4530e02
fix: fixed styles on admin templates
Asespinel Nov 2, 2023
c42c3c3
refactor: changed script location to survey report block
Asespinel Nov 2, 2023
9778c83
chore: removed whitespaces and renamed the context processor files
Asespinel Nov 3, 2023
79effe9
feat: added banner message for survey report
Asespinel Oct 31, 2023
415bcee
refactor: changed variable names
Asespinel Nov 1, 2023
a2c98dc
fix: fixed styles on admin templates
Asespinel Nov 2, 2023
9a6ef63
refactor: changed script location to survey report block
Asespinel Nov 2, 2023
2b65458
chore: removed whitespaces and renamed the context processor files
Asespinel Nov 3, 2023
789d631
chore: added docstring for admin_extra_context method
Asespinel Nov 16, 2023
aab11ce
chore: fix merge conflict
Asespinel Nov 16, 2023
519e4da
fix: fixed docstring position
Asespinel Nov 16, 2023
a47b175
chore: increased query count for unit test
Asespinel Nov 17, 2023
ba47acc
fix: add a send_report_to_external_api mock
MaferMazu Nov 28, 2023
a8cdee5
fix: increase query counts
MaferMazu Nov 28, 2023
237515a
Merge branch 'master' into ase/survey-report-message
MaferMazu Nov 29, 2023
1087363
refactor: separated survey report template from admin and deleted bas…
Asespinel Dec 13, 2023
28e3fb5
chore: check if the request is inside an admin page to reduce query c…
Asespinel Dec 15, 2023
c4f65ec
fix: fixed query Count in override performance test
Asespinel Dec 15, 2023
8c56953
fix: fixed query counts on other test files
Asespinel Dec 15, 2023
a32f3e4
fix: fixed query number in test_views
Asespinel Dec 15, 2023
5d6c531
chore: updated query number
Asespinel Dec 15, 2023
c129b80
revert: reverted query count
Asespinel Dec 15, 2023
718b6f5
refactor: changed months variable into a configurable setting
Asespinel Dec 15, 2023
4cc231c
chore: added blank line on context processors file
Asespinel Dec 15, 2023
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -234,7 +234,7 @@ class TestFieldOverrideSplitPerformance(FieldOverridePerformanceTestCase):
__test__ = True

# TODO: decrease query count as part of REVO-28
QUERY_COUNT = 32
QUERY_COUNT = 33

TEST_DATA = {
('no_overrides', 1, True, False): (QUERY_COUNT, 2),
Expand Down
10 changes: 5 additions & 5 deletions lms/djangoapps/courseware/tests/test_views.py
Original file line number Diff line number Diff line change
Expand Up @@ -358,7 +358,7 @@ def test_index_query_counts(self):
self.client.login(username=self.user.username, password=self.user_password)
CourseEnrollment.enroll(self.user, course.id)

with self.assertNumQueries(177, table_ignorelist=QUERY_COUNT_TABLE_IGNORELIST):
with self.assertNumQueries(178, table_ignorelist=QUERY_COUNT_TABLE_IGNORELIST):
with check_mongo_calls(3):
url = reverse(
'courseware_section',
Expand Down Expand Up @@ -1491,8 +1491,8 @@ def test_view_certificate_link(self):
self.assertContains(resp, "earned a certificate for this course.")

@ddt.data(
(True, 53),
(False, 53),
(True, 54),
(False, 54),
)
@ddt.unpack
def test_progress_queries_paced_courses(self, self_paced, query_count):
Expand All @@ -1507,13 +1507,13 @@ def test_progress_queries(self):
ContentTypeGatingConfig.objects.create(enabled=True, enabled_as_of=datetime(2018, 1, 1))
self.setup_course()
with self.assertNumQueries(
53, table_ignorelist=QUERY_COUNT_TABLE_IGNORELIST
54, table_ignorelist=QUERY_COUNT_TABLE_IGNORELIST
), check_mongo_calls(2):
self._get_progress_page()

for _ in range(2):
with self.assertNumQueries(
37, table_ignorelist=QUERY_COUNT_TABLE_IGNORELIST
38, table_ignorelist=QUERY_COUNT_TABLE_IGNORELIST
), check_mongo_calls(2):
self._get_progress_page()

Expand Down
7 changes: 6 additions & 1 deletion lms/envs/common.py
Original file line number Diff line number Diff line change
Expand Up @@ -1332,7 +1332,12 @@ def _make_mako_template_dirs(settings):
'openedx.core.djangoapps.site_configuration.context_processors.configuration_context',

# Mobile App processor (Detects if request is from the mobile app)
'lms.djangoapps.mobile_api.context_processor.is_from_mobile_app'
'lms.djangoapps.mobile_api.context_processor.is_from_mobile_app',

# Context processor necesarry for the survey report message appear on the admin site
'openedx.features.survey_report.context_processors.admin_extra_context'


]

# Django templating
Expand Down
125 changes: 125 additions & 0 deletions lms/templates/admin/base.html
Original file line number Diff line number Diff line change
@@ -0,0 +1,125 @@
{% load i18n static %}<!DOCTYPE html>
Comment thread
Asespinel marked this conversation as resolved.
Resolved
{% get_current_language as LANGUAGE_CODE %}{% get_current_language_bidi as LANGUAGE_BIDI %}
<html lang="{{ LANGUAGE_CODE|default:"en-us" }}" dir="{{ LANGUAGE_BIDI|yesno:'rtl,ltr,auto' }}">
<head>
<title>{% block title %}{% endblock %}</title>
<link rel="stylesheet" href="{% block stylesheet %}{% static "admin/css/base.css" %}{% endblock %}">
{% block dark-mode-vars %}
<link rel="stylesheet" href="{% static "admin/css/dark_mode.css" %}">
<script src="{% static "admin/js/theme.js" %}" defer></script>
{% endblock %}
{% if not is_popup and is_nav_sidebar_enabled %}
<link rel="stylesheet" href="{% static "admin/css/nav_sidebar.css" %}">
<script src="{% static 'admin/js/nav_sidebar.js' %}" defer></script>
{% endif %}
{% block extrastyle %}{% endblock %}
{% if LANGUAGE_BIDI %}<link rel="stylesheet" href="{% block stylesheet_rtl %}{% static "admin/css/rtl.css" %}{% endblock %}">{% endif %}
{% block extrahead %}{% endblock %}
{% block responsive %}
<meta name="viewport" content="width=device-width, initial-scale=1.0">
<link rel="stylesheet" href="{% static "admin/css/responsive.css" %}">
{% if LANGUAGE_BIDI %}<link rel="stylesheet" href="{% static "admin/css/responsive_rtl.css" %}">{% endif %}
{% endblock %}
{% block blockbots %}<meta name="robots" content="NONE,NOARCHIVE">{% endblock %}
<script src="https://code.jquery.com/jquery-3.6.0.min.js"></script>
</head>

<body class="{% if is_popup %}popup {% endif %}{% block bodyclass %}{% endblock %}"
data-admin-utc-offset="{% now "Z" %}">
<!-- Container -->
<div id="container">

{% if not is_popup %}
<!-- Header -->
{% block header %}
<div id="header">
<div id="branding">
{% block branding %}{% endblock %}
</div>
{% block usertools %}
{% if has_permission %}
<div id="user-tools">
{% block welcome-msg %}
{% translate 'Welcome,' %}
<strong>{% firstof user.get_short_name user.get_username %}</strong>.
{% endblock %}
{% block userlinks %}
{% if site_url %}
<a href="{{ site_url }}">{% translate 'View site' %}</a> /
{% endif %}
{% if user.is_active and user.is_staff %}
{% url 'django-admindocs-docroot' as docsroot %}
{% if docsroot %}
<a href="{{ docsroot }}">{% translate 'Documentation' %}</a> /
{% endif %}
{% endif %}
{% if user.has_usable_password %}
<a href="{% url 'admin:password_change' %}">{% translate 'Change password' %}</a> /
{% endif %}
<form id="logout-form" method="post" action="{% url 'admin:logout' %}">
{% csrf_token %}
<button type="submit">{% translate 'Log out' %}</button>
</form>
{% include "admin/color_theme_toggle.html" %}
{% endblock %}
</div>
{% endif %}
{% endblock %}
{% block nav-global %}{% endblock %}
</div>
{% endblock %}
<!-- END Header -->
{% block nav-breadcrumbs %}
<nav aria-label="{% translate 'Breadcrumbs' %}">
{% block breadcrumbs %}
<div class="breadcrumbs">
<a href="{% url 'admin:index' %}">{% translate 'Home' %}</a>
{% if title %} &rsaquo; {{ title }}{% endif %}
</div>
{% endblock %}
</nav>
{% endblock %}
{% endif %}
{% block survey_report_banner %}{% endblock %}
Comment thread
Asespinel marked this conversation as resolved.
Resolved
<div class="main" id="main">
{% if not is_popup and is_nav_sidebar_enabled %}
{% block nav-sidebar %}
{% include "admin/nav_sidebar.html" %}
{% endblock %}
{% endif %}
<main id="content-start" class="content" tabindex="-1">
{% block messages %}
{% if messages %}
<ul class="messagelist">{% for message in messages %}
<li{% if message.tags %} class="{{ message.tags }}"{% endif %}>{{ message|capfirst }}</li>
{% endfor %}</ul>
{% endif %}
{% endblock messages %}
<!-- Content -->
<div id="content" class="{% block coltype %}colM{% endblock %}">
{% block pretitle %}{% endblock %}
{% block content_title %}{% if title %}<h1>{{ title }}</h1>{% endif %}{% endblock %}
{% block content_subtitle %}{% if subtitle %}<h2>{{ subtitle }}</h2>{% endif %}{% endblock %}
{% block content %}
{% block object-tools %}{% endblock %}
{{ content }}
{% endblock %}
{% block sidebar %}{% endblock %}
<br class="clear">
</div>
<!-- END Content -->
{% block footer %}<div id="footer"></div>{% endblock %}
</main>
</div>
</div>
<!-- END Container -->

<!-- SVGs -->
<svg xmlns="http://www.w3.org/2000/svg" class="base-svgs">
<symbol viewBox="0 0 24 24" width="1rem" height="1rem" id="icon-auto"><path d="M0 0h24v24H0z" fill="currentColor"/><path d="M12 22C6.477 22 2 17.523 2 12S6.477 2 12 2s10 4.477 10 10-4.477 10-10 10zm0-2V4a8 8 0 1 0 0 16z"/></symbol>
<symbol viewBox="0 0 24 24" width="1rem" height="1rem" id="icon-moon"><path d="M0 0h24v24H0z" fill="currentColor"/><path d="M10 7a7 7 0 0 0 12 4.9v.1c0 5.523-4.477 10-10 10S2 17.523 2 12 6.477 2 12 2h.1A6.979 6.979 0 0 0 10 7zm-6 5a8 8 0 0 0 15.062 3.762A9 9 0 0 1 8.238 4.938 7.999 7.999 0 0 0 4 12z"/></symbol>
<symbol viewBox="0 0 24 24" width="1rem" height="1rem" id="icon-sun"><path d="M0 0h24v24H0z" fill="currentColor"/><path d="M12 18a6 6 0 1 1 0-12 6 6 0 0 1 0 12zm0-2a4 4 0 1 0 0-8 4 4 0 0 0 0 8zM11 1h2v3h-2V1zm0 19h2v3h-2v-3zM3.515 4.929l1.414-1.414L7.05 5.636 5.636 7.05 3.515 4.93zM16.95 18.364l1.414-1.414 2.121 2.121-1.414 1.414-2.121-2.121zm2.121-14.85l1.414 1.415-2.121 2.121-1.414-1.414 2.121-2.121zM5.636 16.95l1.414 1.414-2.121 2.121-1.414-1.414 2.121-2.121zM23 11v2h-3v-2h3zM4 11v2H1v-2h3z"/></symbol>
</svg>
<!-- END SVGs -->
</body>
</html>
75 changes: 75 additions & 0 deletions lms/templates/admin/base_site.html
Original file line number Diff line number Diff line change
Expand Up @@ -20,3 +20,78 @@ <h1 id="site-name"><a href="{% url 'admin:index' %}">{{ site_header|default:_('D


{% endblock %}

{% block survey_report_banner %}
Comment thread
Asespinel marked this conversation as resolved.
Resolved
{% if show_survey_report_banner %}
<div id="originalContent" style="border: 3px solid #06405d; margin-bottom: 50px; rgb(0 0 0 / 18%) 0px 3px 5px;">
<div style="background-color: #06405d;padding: 17px 37px;">
<h1 style="margin: 0; color: #FFFF; font-weight: 600;">Join the Open edX Data Sharing Initiative and shape the future of learning</h1>
</div>
<div style="padding: 17px 37px;">
<p>The Open edX Project relies on the collective strength of its community to be a thriving platform for online education.</p>
<p>Open edX is a dynamic ecosystem and it is used in diverse learning environments. By sharing anonymized reports of aggregated data, you can contribute to the collective knowledge of the community. This data can help us all understand the reach of our project, make better decisions and ultimately support innovation in lifelong learning and advance next generation learning experience platforms.</p>
<p>We invite you to join the Open edX Data Sharing Initiative by sharing an anonymized reports of aggregated data from your institution's usage of the platform. The report data will be sent to Axim Collaborative, the non-profit behind the Open edX project.</p>
<p>If you agree and want to send a report you can click the button below. You can always send reports and see the status of reports you have sent in the past at <a href="/admin/survey_report/surveyreport/">admin/survey_report/surveyreport/</a> .</p>
</div>
<div style="display: flex; justify-content: flex-end; padding: 0 37px 17px;">
<button id="dismissButton" type="button" style="background-color:var(--close-button-bg); color: var(--button-fg); border: none; border-radius: 4px; padding: 10px 20px; margin-right: 10px; cursor: pointer;">Dismiss</button>
<form id='survey_report_form' method="POST" action="/survey_report/generate_report" style="margin: 0; padding: 0;">
{% csrf_token %}
<button type="submit" style="background-color: #377D4D; color: var(--button-fg); border: none; border-radius: 4px; padding: 10px 20px; cursor: pointer;">Send Report</button>
</form>
</div>
</div>
<div id="thankYouMessage" style="display: none; background-color: var(--darkened-bg); padding: 20px 40px; margin-bottom: 30px;box-shadow: rgb(0 0 0 / 18%) 0px 3px 5px;">
<div style="display: flex; align-items: center;">
<svg xmlns="http://www.w3.org/2000/svg" width="30" height="30" viewBox="0 0 24 24">
<g fill="#377D4D"><path d="M22 12c0 5.523-4.477 10-10 10S2 17.523 2 12S6.477 2 12 2s10 4.477 10 10Z"></path>
<path d="M16.03 8.97a.75.75 0 0 1 0 1.06l-5 5a.75.75 0 0 1-1.06 0l-2-2a.75.75 0 1 1 1.06-1.06l1.47 1.47l2.235-2.236L14.97 8.97a.75.75 0 0 1 1.06 0Z" fill="#FFF"></path>
</g>
</svg>
<span style="font-size: 16px; margin-left: 15px;">Thank you for your collaboration and support! Your contribution is greatly appreciated and will help us continue to improve.</span>
</div>
</div>
{% endif %}

<!-- The original content of the block -->
<script>
$(document).ready(function(){
$('#dismissButton').click(function() {
$('#originalContent').slideUp('slow', function() {
// If you want to do something after the slide-up, do it here.
// For example, you can hide the entire div:
// $(this).hide();
});
});
// When the form is submitted
$("#survey_report_form").submit(function(event){
event.preventDefault(); // Prevent the form from submitting traditionally

// Make the AJAX request
$.ajax({
url: $(this).attr("action"),
type: $(this).attr("method"),
data: $(this).serialize(),
success: function(response){
// Hide the original content block
$("#originalContent").slideUp(400, function() {
//$(this).css('display', 'none');
// Show the thank-you message block with slide down effect
$("#thankYouMessage").slideDown(400, function() {
// Wait for 3 seconds (3000 milliseconds) and then slide up the thank-you message
setTimeout(function() {
$("#thankYouMessage").slideUp(400);
}, 3000);
});
});
},
error: function(error){
// Handle any errors
console.error("Error sending report:", error);
}
});
});
});
</script>
Comment thread
Asespinel marked this conversation as resolved.
Resolved

{% endblock %}
Original file line number Diff line number Diff line change
Expand Up @@ -49,7 +49,7 @@ def test_queries(self):

# Fetch the view and verify that the query counts haven't changed
# TODO: decrease query count as part of REVO-28
with self.assertNumQueries(52, table_ignorelist=QUERY_COUNT_TABLE_IGNORELIST):
with self.assertNumQueries(53, table_ignorelist=QUERY_COUNT_TABLE_IGNORELIST):
with check_mongo_calls(3):
url = course_updates_url(self.course)
self.client.get(url)
1 change: 1 addition & 0 deletions openedx/features/survey_report/api.py
Original file line number Diff line number Diff line change
Expand Up @@ -53,6 +53,7 @@ def generate_report() -> None:
data = get_report_data()
data["state"] = SURVEY_REPORT_GENERATED
update_report(survey_report.id, data)
send_report_to_external_api(survey_report.id)
except (Exception, ) as update_report_error:
update_report(survey_report.id, {"state": SURVEY_REPORT_ERROR})
raise Exception(update_report_error) from update_report_error
Expand Down
31 changes: 31 additions & 0 deletions openedx/features/survey_report/context_processors.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,31 @@
"""
This is the survey report contex_processor modules

This is meant to determine the visibility of the survey report banner
across all admin pages in case a survey report has not been generated

"""

from datetime import datetime
from dateutil.relativedelta import relativedelta # for months test
from .models import SurveyReport


def admin_extra_context(request):

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Random thought (completely optional): Is it easy to check if this request is for an admin view and return {'show_survey_report_banner': False} if it's not an admin view (without having to check the database)? In that case, you might be able to remove the changes that require bumping the number of queries in the other tests.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

I implemented the change requested with this change we should revert back the query counts by 1 in all the tests? Or just on particular ones?

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

We will know for sure once the tests run completely but my guess is that for all of them.

"""
This function sends extra context to every admin site

The current treshhold to show the banner is one month but this can be redefined in the future

"""
months = 1

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

We should move this to a setting so that it is configurable and also put a better default. Since we would like to get reports about twice a year, we could set the default to 6.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Thanks for the feedback ioll change it to 6

try:
latest_report = SurveyReport.objects.latest('created_at')
months_treshhold = datetime.today().date() - relativedelta(months=months) # Calculate date one month ago
show_survey_report_banner = latest_report.created_at.date() <= months_treshhold
except SurveyReport.DoesNotExist:
show_survey_report_banner = False

return {
'show_survey_report_banner': show_survey_report_banner,
}
Original file line number Diff line number Diff line change
Expand Up @@ -16,8 +16,9 @@ class GenerateReportTest(TestCase):
Test for generate_report command.
"""

@mock.patch('openedx.features.survey_report.api.send_report_to_external_api')
@mock.patch('openedx.features.survey_report.api.get_report_data')
def test_generate_report(self, mock_get_report_data):
def test_generate_report(self, mock_get_report_data, mock_send_report):
"""
Test that generate_report command creates a survey report.
"""
Expand All @@ -30,6 +31,7 @@ def test_generate_report(self, mock_get_report_data):
'extra_data': {'extra': 'data'},
}
mock_get_report_data.return_value = report_test_data
mock_send_report.return_value = None
out = StringIO()
call_command('generate_report', no_send=True, stdout=out)

Expand Down