Milimetric has submitted this change and it was merged.
Change subject: finished csv output by timeseries
......................................................................
finished csv output by timeseries
Change-Id: I39c0cb3ab9c02ac4d43624a69ea7c06a3692514c
---
M tests/fixtures.py
M tests/test_controllers/test_reports.py
M wikimetrics/configurables.py
M wikimetrics/controllers/home.py
M wikimetrics/controllers/reports.py
M wikimetrics/models/cohort.py
M wikimetrics/models/cohort_user.py
M wikimetrics/models/report_nodes/run_report.py
M wikimetrics/templates/report.html
M wikimetrics/utils.py
10 files changed, 161 insertions(+), 34 deletions(-)
Approvals:
Milimetric: Verified; Looks good to me, approved
diff --git a/tests/fixtures.py b/tests/fixtures.py
index 4cb62ef..ba00360 100644
--- a/tests/fixtures.py
+++ b/tests/fixtures.py
@@ -51,6 +51,7 @@
revisions_per_editor=0,
revision_timestamps=[],
revision_lengths=[],
+ owner_user_id=None,
):
"""
Parameters
@@ -63,6 +64,7 @@
revision_lengths : two dimensional array indexed same as
above OR
a single integer so all revisions will
have the same length
+ owner_user_id : record in the User table that owns this
cohort
Returns
Nothing but creates the following, to be accessed in a test:
@@ -137,6 +139,22 @@
revision.rev_parent_id = ordered_revisions[i - 1].rev_id
self.mwSession.commit()
+
+ # establish ownership for this cohort
+ if not owner_user_id:
+ #owner_user = User(username='test cohort owner')
+ #self.session.add(owner_user)
+ #self.session.commit()
+ #owner_user_id = owner_user.id
+ # TODO: break dependency on old complicated setUp
+ owner_user_id = self.test_web_user_id
+
+ self.session.add(CohortUser(
+ user_id=owner_user_id,
+ cohort_id=self.cohort.id,
+ role=CohortUserRole.OWNER,
+ ))
+ self.session.commit()
# create test data for metric PagesCreated
@nottest
diff --git a/tests/test_controllers/test_reports.py
b/tests/test_controllers/test_reports.py
index c9453c9..8aac0ce 100644
--- a/tests/test_controllers/test_reports.py
+++ b/tests/test_controllers/test_reports.py
@@ -16,6 +16,19 @@
class ReportsControllerTest(WebTest):
+ def setUp(self):
+ WebTest.setUp(self)
+ self.create_test_cohort(
+ editor_count=2,
+ revisions_per_editor=4,
+ revision_timestamps=[
+ [20130101000001, 20130201010000, 20130201010100,
20130301020100],
+ [20130101000001, 20130201010000, 20130201010100,
20130301020100]
+ ],
+ revision_lengths=10,
+ owner_user_id=self.test_web_user_id,
+ )
+
def test_index(self):
response = self.app.get('/reports/', follow_redirects=True)
assert_true(
@@ -63,7 +76,7 @@
desired_responses = [{
'name': 'Edits - test',
'cohort': {
- 'id': self.test_cohort_id,
+ 'id': self.cohort.id,
},
'metric': {
'name': 'NamespaceEdits',
@@ -141,13 +154,13 @@
desired_responses = [{
'name': 'Edits - test',
'cohort': {
- 'id': self.test_cohort_id,
+ 'id': self.cohort.id,
},
'metric': {
'name': 'NamespaceEdits',
'namespaces': [0, 1, 2],
- 'start_date': '2013-06-01 00:00:00',
- 'end_date': '2013-09-01 00:00:00',
+ 'start_date': '2013-01-01 00:00:00',
+ 'end_date': '2013-05-01 00:00:00',
'individualResults': False,
'aggregateResults': True,
'aggregateSum': False,
@@ -172,7 +185,7 @@
# Check the csv result
response = self.app.get('/reports/result/{0}.csv'.format(result_key))
- assert_true(response.data.find('Average') >= 0)
+ assert_true(response.data.find('Average,4.0') >= 0)
# Testing to see if the parameters are also in the CSV
# (related to Mingle 1089)
@@ -187,13 +200,13 @@
desired_responses = [{
'name': 'Edits - test',
'cohort': {
- 'id': self.test_cohort_id,
+ 'id': self.cohort.id,
},
'metric': {
'name': 'NamespaceEdits',
'namespaces': [0, 1, 2],
- 'start_date': '2013-06-01 00:00:00',
- 'end_date': '2013-09-01 00:00:00',
+ 'start_date': '2013-01-01 00:00:00',
+ 'end_date': '2013-04-01 00:00:00',
'individualResults': False,
'aggregateResults': True,
'aggregateSum': True,
@@ -218,20 +231,20 @@
# Check the csv result
response = self.app.get('/reports/result/{0}.csv'.format(result_key))
- assert_true(response.data.find('Sum') >= 0)
+ assert_true(response.data.find('Sum,8.0') >= 0)
def test_report_result_std_dev_only_csv(self):
# Make the request
desired_responses = [{
'name': 'Edits - test',
'cohort': {
- 'id': self.test_cohort_id,
+ 'id': self.cohort.id,
},
'metric': {
'name': 'NamespaceEdits',
'namespaces': [0, 1, 2],
- 'start_date': '2013-06-01 00:00:00',
- 'end_date': '2013-09-01 00:00:00',
+ 'start_date': '2013-01-01 00:00:00',
+ 'end_date': '2013-04-01 00:00:00',
'individualResults': False,
'aggregateResults': True,
'aggregateSum': False,
@@ -257,3 +270,62 @@
# Check the csv result
response = self.app.get('/reports/result/{0}.csv'.format(result_key))
assert_true(response.data.find('Standard Deviation') >= 0)
+
+ def test_report_result_timeseries_csv(self):
+ # Make the request
+ desired_responses = [{
+ 'name': 'Edits - test',
+ 'cohort': {
+ 'id': self.cohort.id,
+ },
+ 'metric': {
+ 'name': 'NamespaceEdits',
+ 'timeseries': 'month',
+ 'namespaces': [0, 1, 2],
+ 'start_date': '2013-01-01 00:00:00',
+ 'end_date': '2013-05-01 00:00:00',
+ 'individualResults': True,
+ 'aggregateResults': True,
+ 'aggregateSum': False,
+ 'aggregateAverage': True,
+ 'aggregateStandardDeviation': False,
+ },
+ }]
+ json_to_post = json.dumps(desired_responses)
+
+ response = self.app.post('/reports/create/', data=dict(
+ responses=json_to_post
+ ))
+
+ # Wait a second for the task to get processed
+ time.sleep(1)
+
+ # Check that the task has been created
+ response = self.app.get('/reports/list/')
+ parsed = json.loads(response.data)
+ result_key = parsed['reports'][-1]['result_key']
+ task, report = get_celery_task(result_key)
+
+ # Check the csv result
+ response = self.app.get('/reports/result/{0}.csv'.format(result_key))
+ assert_true(response.data.find(
+ 'user_id,submetric,'
+ '2013-01-01 00:00:00,2013-02-01 00:00:00,'
+ '2013-03-01 00:00:00,2013-04-01 00:00:00'
+ ) >= 0)
+ assert_true(response.data.find(
+ '{0},edits,1,2,1,0'.format(self.editors[0].user_id)
+ ) >= 0)
+ assert_true(response.data.find(
+ '{0},edits,1,2,1,0'.format(self.editors[1].user_id)
+ ) >= 0)
+ assert_true(response.data.find(
+ 'Average,edits,1.0,2.0,1.0,0.0'
+ ) >= 0)
+
+ # Testing to see if the parameters are also in the CSV
+ assert_true(response.data.find('parameters') >= 0)
+ assert_true(response.data.find('start_date') >= 0)
+ assert_true(response.data.find('end_date') >= 0)
+ assert_true(response.data.find('namespaces') >= 0)
+ assert_true(response.data.find('metric/cohort') >= 0)
diff --git a/wikimetrics/configurables.py b/wikimetrics/configurables.py
index 2cebd01..bd6733d 100644
--- a/wikimetrics/configurables.py
+++ b/wikimetrics/configurables.py
@@ -100,12 +100,12 @@
"""
cmd = ['git', 'log', '--date', 'relative', "--pretty=format:'%an %ar %h'",
'-n', '1']
p = subprocess.Popen(cmd, shell=False, stdout=subprocess.PIPE)
- version, err = p.communicate()
+ version, err = p.communicate()
if err is not None:
version = 'Unknown version'
cmd = ['git', 'log', '--date', 'relative', "--pretty=format:%h", '-n', '1']
p = subprocess.Popen(cmd, shell=False, stdout=subprocess.PIPE)
- latest, err = p.communicate()
+ latest, err = p.communicate()
if err is not None:
latest = 'unknown'
diff --git a/wikimetrics/controllers/home.py b/wikimetrics/controllers/home.py
index 86b10dc..dcdf088 100644
--- a/wikimetrics/controllers/home.py
+++ b/wikimetrics/controllers/home.py
@@ -40,6 +40,7 @@
def override_url_for():
return dict(url_for=dated_url_for)
+
def dated_url_for(endpoint, **values):
"""
Cache-busting version of url_for, works only for static files
diff --git a/wikimetrics/controllers/reports.py
b/wikimetrics/controllers/reports.py
index 63c1fc9..e2797f0 100644
--- a/wikimetrics/controllers/reports.py
+++ b/wikimetrics/controllers/reports.py
@@ -107,13 +107,12 @@
if celery_task.ready():
task_result = get_celery_task_result(celery_task, pj)
- parameters = json.loads(pj.parameters)
+ p = json.loads(pj.parameters)
- if 'timeseries' in parameters\
- and parameters['timeseries'] != TimeseriesChoices.NONE:
- csv_io = get_timeseries_csv(task_result, pj, parameters)
+ if 'timeseries' in p and p['timeseries'] != TimeseriesChoices.NONE:
+ csv_io = get_timeseries_csv(task_result, pj, p)
else:
- csv_io = get_simple_csv(task_result, pj, parameters)
+ csv_io = get_simple_csv(task_result, pj, p)
res = Response(csv_io.getvalue(), mimetype='text/csv')
res.headers['Content-Disposition'] =\
diff --git a/wikimetrics/models/cohort.py b/wikimetrics/models/cohort.py
index 2fea4d6..5bb919e 100644
--- a/wikimetrics/models/cohort.py
+++ b/wikimetrics/models/cohort.py
@@ -1,9 +1,13 @@
import itertools
from operator import itemgetter
from sqlalchemy import Column, Integer, Boolean, DateTime, String, func
+
+from wikimetrics.utils import Unauthorized
from wikimetrics.configurables import db
from wikiuser import WikiUser
from cohort_wikiuser import CohortWikiUser
+from cohort_user import CohortUser, CohortUserRole
+from user import User
__all__ = ['Cohort']
@@ -80,3 +84,33 @@
#return ((project, (r[0] for r in users)) for project, users in groups)
for project, users in groups:
yield project or self.default_project, (r[0] for r in users)
+
+ @staticmethod
+ def get_safely(db_session, user_id, cohort_id):
+ """
+ Gets a cohort but first checks permissions on it
+
+ Parameters
+ db_session : the database session to query
+ user_id : the user that should have access to the cohort
+ cohort_id : the cohort to get
+
+ Returns
+ If found, a Cohort instance with id == cohort_id
+
+ Raises
+ NoResultFound : cohort did not exist
+ Unauthorized : user_id is not allowed to access this cohort
+ """
+ cohort, role = db_session.query(Cohort, CohortUser.role)\
+ .join(CohortUser)\
+ .join(User)\
+ .filter(User.id == user_id)\
+ .filter(Cohort.id == cohort_id)\
+ .filter(Cohort.enabled)\
+ .one()
+
+ if role in CohortUserRole.SAFE_ROLES:
+ return cohort
+ else:
+ raise Unauthorized('You are not allowed to use this cohort')
diff --git a/wikimetrics/models/cohort_user.py
b/wikimetrics/models/cohort_user.py
index a5fbf91..42b31b0 100644
--- a/wikimetrics/models/cohort_user.py
+++ b/wikimetrics/models/cohort_user.py
@@ -10,6 +10,7 @@
class CohortUserRole(object):
OWNER = 'OWNER'
VIEWER = 'VIEWER'
+ SAFE_ROLES = [OWNER, VIEWER]
class CohortUser(db.WikimetricsBase):
diff --git a/wikimetrics/models/report_nodes/run_report.py
b/wikimetrics/models/report_nodes/run_report.py
index 8328399..e7cb01c 100644
--- a/wikimetrics/models/report_nodes/run_report.py
+++ b/wikimetrics/models/report_nodes/run_report.py
@@ -1,3 +1,5 @@
+from sqlalchemy.orm.exc import NoResultFound
+
from wikimetrics.configurables import db
from wikimetrics.models.cohort_user import CohortUserRole
from wikimetrics.models.cohort import Cohort
@@ -36,20 +38,13 @@
children = []
metric_names = []
cohort_names = []
- allowed_roles = [CohortUserRole.OWNER, CohortUserRole.VIEWER]
+
for cohort_metric_dict in desired_responses:
# get cohort
cohort_dict = cohort_metric_dict['cohort']
db_session = db.get_session()
- cohort = db_session.query(Cohort)\
- .join(CohortUser)\
- .join(User)\
- .filter(User.id == self.user_id)\
- .filter(Cohort.id == cohort_dict['id'])\
- .filter(Cohort.enabled)\
- .filter(CohortUser.role.in_(allowed_roles))\
- .one()
+ cohort = Cohort.get_safely(db_session, self.user_id,
cohort_dict['id'])
db_session.close()
# construct metric
diff --git a/wikimetrics/templates/report.html
b/wikimetrics/templates/report.html
index 107c1d1..3938bdf 100644
--- a/wikimetrics/templates/report.html
+++ b/wikimetrics/templates/report.html
@@ -82,12 +82,12 @@
<input type="checkbox" data-bind="checked:
aggregateAverage, attr: {id: tabId() + '-a-avg'}"/>
</div>
</div>
- <div class="control-group">
- <label class="control-label" data-bind="attr:
{for: tabId() + '-a-std'}">Standard Deviation</label>
- <div class="controls">
- <input type="checkbox" data-bind="checked:
aggregateStandardDeviation, attr: {id: tabId() + '-a-std'}"/>
- </div>
- </div>
+ <!--<div class="control-group">-->
+ <!--<label class="control-label"
data-bind="attr: {for: tabId() + '-a-std'}">Standard Deviation</label>-->
+ <!--<div class="controls">-->
+ <!--<input type="checkbox"
data-bind="checked: aggregateStandardDeviation, attr: {id: tabId() +
'-a-std'}"/>-->
+ <!--</div>-->
+ <!--</div>-->
</div>
</div>
</div>
diff --git a/wikimetrics/utils.py b/wikimetrics/utils.py
index 738226d..8e7f2e6 100644
--- a/wikimetrics/utils.py
+++ b/wikimetrics/utils.py
@@ -109,3 +109,10 @@
uniques[key] = o
return uniques.values()
+
+
+class Unauthorized(Exception):
+ """
+ Different exception type to separate "unauthorized" errors from the rest
+ """
+ pass
--
To view, visit https://gerrit.wikimedia.org/r/86179
To unsubscribe, visit https://gerrit.wikimedia.org/r/settings
Gerrit-MessageType: merged
Gerrit-Change-Id: I39c0cb3ab9c02ac4d43624a69ea7c06a3692514c
Gerrit-PatchSet: 2
Gerrit-Project: analytics/wikimetrics
Gerrit-Branch: master
Gerrit-Owner: Milimetric <[email protected]>
Gerrit-Reviewer: Milimetric <[email protected]>
_______________________________________________
MediaWiki-commits mailing list
[email protected]
https://lists.wikimedia.org/mailman/listinfo/mediawiki-commits
