510034 - Implement QAC Stats Tab

Status: RESOLVED FIXED

(Other Applications Graveyard :: QA Companion, enhancement)

Description

[Aaron Train [:aaronmt]]

Attachment: v0.1

Currently, Litmus web services provide a very basic way to garner stats for a designated user through JSON get requests, (i.e., json.cgi?user_stats=username). 

AFAIK, publicly accessible stats at https://litmus.mozilla.org/stats.cgi would need to be parsed which is messy. 

In regards to future Litmus redesign, this can be improved.

In the meantime, as a basic enhancement to QAC we could add a fourth tab that utilizes the aforementioned basic web services.

In this bug, I would like to garner ideas to make the stats unique and fun. The figures to work with are monthly,weekly and overall test submission counts

Current implementation (0.1) merely just displays the figures.

As an idea, what do you think about a very basic earnings/ranking system based on the submission tally's - we rank them with some kind of icon or star or some sort.

Any ideas more than welcome.

See 0.1 screenshot in attachment

Comment 1

[Aaron Train [:aaronmt]]

Ideally end-goal implementation would do fancy things with numbers regarding platform/branch runs with comparisons to other users (i.e, bar graphs, and all that fancy stuff)

Comment 2

[Aaron Train [:aaronmt]]

as well, ideally these stats would also hint the user at areas with low test coverage as explored by the user.

Comment 3

[Aaron Train [:aaronmt]]

Attachment: v0.2

Idea of v0.2 was to introduce recent bug filings by the current user in the past four weeks. These bugs are listed in descending order of most recent first with each hyper-linked to the associated bug.

As well introduced is the Protovis JS graphics toolkit for graphing. Currently I don't have much data to work with (i.e, monthly submits, weekly submits and all time submits [as per litmus json limitation and implementation]) and don't want to manifest a negative feedback loop by introducing too much to the user.

Any ideas more than welcome

Comment 4

[Aaron Train [:aaronmt]]

Attachment: v0.25

- Added query for today's results
- Added query for weekly results
- Added query for recommended testing pulling in 
a) Recently failed results
b) Recently unclear/broken results

- Removed protovis graph library 
- Removed graphs (we just don't have relevant information at the moment to make some charts for (couldn't think of anything useful for the user))

Heather/Clint/Other's let me know what you think from the screenshot

Comment 5

[Aaron Train [:aaronmt]]

Attachment: v1.0 - implementation based on screenshot v0.25

v0.25 - implementation based on screen shot (v0.25)

Up for review :)

Comment 6

[Aaron Train [:aaronmt]]

Attachment: unstar.png

Comment 7

[Aaron Train [:aaronmt]]

Attachment: star.png

Comment 8

[cmtalbert]

Comment on attachment 396585 [details] [diff] [review]
v1.0 - implementation based on screenshot v0.25 

This is a really good first pass.  It needs a little more work, though, I think.


>Index: chrome/locale/en-US/qa.properties
>===================================================================
>--- chrome/locale/en-US/qa.properties	(revision 50094)
>+++ chrome/locale/en-US/qa.properties	(working copy)
>@@ -69,3 +69,7 @@
> qa.firstrun.accountCreated = Account Created. You are now logged into Litmus and can begin testing.
> qa.firstrun.email.invalid = Invalid E-Mail.
> qa.preferences.error = Error: Problem fetching data from Litmus.
>+qa.stats.bugs.error = Error: There was a problem fetching data Bugzilla
>+qa.stats.subtitle = Personal stats for
Don't do this ^^ It makes things really hard to translate.  Instead use our substitution method like this: qa.stats.subtitle = Personal stats for %S

>Index: chrome/content/tabs/stats.js
>+   
>+   getCurrentUser : function(){
>+    var credentials = new Array(
>+        qaLogin.getLogin('chrome://qa/content/qa.xul'),
>+        qaLogin.getLogin('https://litmus.mozilla.org'));
>+    
>+    var currentUsername = "";
>+     
>+    if(credentials[0]) {
>+     document.getElementById("qa-user-stats-cur").value = qaMain.bundle.getString("qa.stats.subtitle") + " " + credentials[0].username;
So, here to use the substitution method, (with the %S in the propery file, you would call formatStringFromName instead of "getString" on the bundle object

>+    }
>+    else if(credentials[1]) {
>+     document.getElementById("qa-user-stats-cur").value = credentials[1].username;
>+     currentUsername =  credentials[1].username;
Why don't we set the qa.stats.subtitle in this case?  It seems like we'd still want to set that text, right?

The star/unstar code below seems really convoluted.  Why couldn't you do something like:
Step 1: Draw 10 stars ( or some number that looks good)
Step 1: calculate the number of gold stars to draw based on the following formula:
Total number of gold stars = total tests / tests to make a star 

You're also creating different scales for each category, which is probably what we want but it's really hard to follow.  Can we just create an explicit rate for each segment: 
* 10 tests/week = 1 week gold star
* 50 tests/month = 1 month gold star
* 100 tests/alltime = 1 all time gold star

Then we can more easily deal with this without hardcoding a bunch of ranges.
>+       
>+    if(monthly.value >= 50 && monthly.value < 100){
>+     monthlyGroup.appendChild(qaStats.makeStar());
>+     for(i=0;i<3;i++) monthlyGroup.appendChild(qaStats.makeUnStar());
>+    }
>+    else if(monthly.value >= 100 && monthly.value < 250){
>+     for(i=0;i<2;i++) monthlyGroup.appendChild(qaStats.makeStar());
>+     for(i=0;i<2;i++) monthlyGroup.appendChild(qaStats.makeUnStar());
>+    }
>+    else if(monthly.value >= 250 && monthly.value <= 500){
>+     for(i=0;i<4;i++) monthlyGroup.appendChild(qaStats.makeStar());
>+    }else {
>+     for(i=0;i<4;i++) monthlyGroup.appendChild(qaStats.makeUnStar()); 
>+    }
>+    
>+    if(weekly.value >= 25 && weekly.value < 50){
>+     weeklyGroup.appendChild(qaStats.makeStar());
>+     for(i=0;i<3;i++) weeklyGroup.appendChild(qaStats.makeUnStar());
>+    }
>+    else if(weekly.value >= 50 && weekly.value < 75){
>+     for(i=0;i<2;i++) weeklyGroup.appendChild(qaStats.makeStar());
>+     for(i=0;i<2;i++) weeklyGroup.appendChild(qaStats.makeUnStar());
>+    }
>+    else if(weekly.value >= 75 && weekly.value <= 100){
>+     for(i=0;i<4;i++) weeklyGroup.appendChild(qaStats.makeStar());
>+    }else{
>+     for(i=0;i<4;i++) weeklyGroup.appendChild(qaStats.makeUnStar());
>+    }
>+    
>+    if(alltime.value >= 250 && alltime.value < 500){
>+     allTimeGroup.appendChild(qaStats.makeStar());
>+     for(i=0;i<3;i++) allTimeGroup.appendChild(qaStats.makeUnStar());
>+    }
>+    else if(alltime.value >= 500 && alltime.value < 750){
>+     for(i=0;i<2;i++) allTimeGroup.appendChild(qaStats.makeStar());
>+     for(i=0;i<2;i++) allTimeGroup.appendChild(qaStats.makeUnStar());
>+    }
>+    else if(alltime.value >= 750 && alltime.value <= 1000){
>+     for(i=0;i<4;i++) allTimeGroup.appendChild(qaStats.makeStar());
>+    }else{
>+     for(i=0;i<2;i++) allTimeGroup.appendChild(qaStats.makeUnStar());
>+    }
>+   },
>+   
>+   
>+   
>+   generateRecentBugListing : function(bugs){   
>+    var menu = document.getElementById("qa-user-stats-buglist");
>+    while(menu.getRowCount()) menu.removeItemAt(0);
>+     
>+    for(i=0;i<bugs.length;i++){
>+      var row = document.createElement("listitem");
>+      row.value = bugs[i].id;
>+
>+      var id = document.createElement("listcell");
>+      id.setAttribute("label",bugs[i].id);
>+
>+      var name = document.createElement("listcell");
>+      name.setAttribute("label",bugs[i].summary); 
>+      name.setAttribute("flex","1");
>+      name.setAttribute("style","color: #0067ac; font-weight: bold");
Why can't we set this in a CSS page instead of doing it on the fly here?  That way it will be easy to find if we want to change it later.  We can simply set a class on each of these cells and then put a single rule in the CSS sheet that applies to that class rather than calling this code on every single listcell we create.

Comment 9

[cmtalbert]

(In reply to comment #8)

> You're also creating different scales for each category, which is probably what
> we want but it's really hard to follow.  Can we just create an explicit rate
> for each segment: 
> * 10 tests/week = 1 week gold star
> * 50 tests/month = 1 month gold star
> * 100 tests/alltime = 1 all time gold star
Sorry for the spam, I should have noted that these are just suggestions and aren't necessarily the rates that we want to set.  I don't care what the rates are, I really want us to explicitly call them out in the code and make the code easier to follow/extend here.

Comment 10

[Aaron Train [:aaronmt]]

Suggestions are good. I can use the listed scales as example and reduce the code into six statements. How about this? 

var starGroup = 10 // Max ranking per category

// Draw rankings gold stars
    
for(i=0;i<Math.floor(monthly.value / 50);i++)
 monthlyGroup.appendChild(qaStats.makeStar())
    
for(i=0;i<Math.floor(weekly.value / 10);i++)
 weeklyGroup.appendChild(qaStats.makeStar())
     
for(i=0;i<Math.floor(alltime.value / 100);i++)
 allTimeGroup.appendChild(qaStats.makeStar())

// Draw rankings grey unstars (underachieved)
    
for(i=0;i<(starGroup - Math.floor(monthly.value / 50));i++)
 monthlyGroup.appendChild(qaStats.makeUnStar());
    
for(i=0;i<(starGroup - Math.floor(weekly.value / 10));i++)
 weeklyGroup.appendChild(qaStats.makeUnStar());
      
for(i=0;i<(starGroup - Math.floor(alltime.value / 100));i++)
 allTimeGroup.appendChild(qaStats.makeUnStar());

Comment 11

[Aaron Train [:aaronmt]]

^ or I can initially draw 10 and remove and replace

Comment 12

[Aaron Train [:aaronmt]]

Attachment: v1.1

Fixes from comment #8 and comment #9 with my inclusion of comment #10

Comment 13

[Aaron Train [:aaronmt]]

Doh, ^ inclusion of comment #11

Comment 14

[cmtalbert]

Comment on attachment 396802 [details] [diff] [review]
v1.1

Awesome! r=ctalbert

Comment 15

[cmtalbert]

rev=50381 Checked in