activity: Make activity page with graphs on issues #27

nalinbhardwaj · 2017-12-06T21:32:14Z

Build scraper that scrapes issues.json from gh-board and produces a JSON file just containing stats. This JSON file is used for displaying a static page with graphs, which can be toggled from a dropdown to display past year/month or week's data.
Example page:

~~I haven't worked with Django before, so please let me know if something is wrong, or there are better ways to do what I do.~~ I've since removed all uses of Django, and am deploying it individually since I couldn't get it to work with the rest of the codebase neatly(getting some errors, although it worked fine individually)

The data is processed in python, and not directly in frontend to prevent browser overload(issues.json is > 27 MB)
Closes #25

TODO: Once all changes are approved, make orderly commits with proper fragmentation, description etc. (done)

gitmate-bot · 2017-12-06T21:32:30Z

activity/scraper.py

+import datetime
+from dateutil import parser, relativedelta
+
+issuesurl = "https://coala.github.io/gh-board/issues.json"


The line contains the keyword 'coala'.

Origin: KeywordBear, Section: generalization.

gitmate-bot · 2017-12-06T21:32:30Z

activity/index.html

+<html>
+
+<head>
+    <title>Coala Community Activity</title>


The line contains the keyword 'Coala'.

Origin: KeywordBear, Section: generalization.

So, should I remove the word 'coala' or what?

We don't allow "coala" here, the organization name must be dynamic.

By the way, coala is always written with lower case 'c', but in this repo, it's should be removed altogether.

We can keep this as Organization Community Activity for now, later after the model patch, this can be made dynamic.
Same for wherever you've used coala

@Monal5031 okay I’ll do that, but I can’t remove it from the issuesurl since that’s a url to fetch data from? Should I add that to ignored files?

# Ignore KeywordBear

gitmate-bot · 2017-12-06T21:32:31Z

activity/index.html

+</head>
+
+<body>
+    <h1>Coala Activity</h1>


The line contains the keyword 'Coala'.

Origin: KeywordBear, Section: generalization.

gitmate-bot · 2017-12-06T21:32:32Z

activity/index.html

+                responsive: true,
+                title: {
+                    display: true,
+                    text: 'Coala Community Activity'


The line contains the keyword 'Coala'.

Origin: KeywordBear, Section: generalization.

Techievena · 2017-12-07T11:23:54Z

activity/scraper.py

+def diff_week(d1, d2):
+	monday1 = (d1 - datetime.timedelta(days=d1.weekday()))
+	monday2 = (d2 - datetime.timedelta(days=d2.weekday()))
+	return (monday1 - monday2).days / 7


Though I am not sure about this. I think this would be rather // 7. We need the floor value for the number of weeks right.

Monday1-monday2 is guaranteed to be a multiple of 7. Since, it’s a difference in Monday’s of different weeks...

Then maybe return ((d1-d2).days)//7 would be more simple.

The purpose behind doing it this way is to make sure weeks are counted starting from Monday. So if the data is scraped on a Thursday, the current week is Monday-Thursday, and not previous Friday-Thursday, since it doesn’t make much sense mentally to have weeks not start from Monday, and also, it maintains consistency with other options(for example, year counts a month partially if it hasn’t ended yet)
What the code does is basically calculate the number of Mondays between the two dates, which is when regular calendar week changes.

I've left a comment in the code now to be clear(er) about this for future readers.

jayvdb · 2017-12-07T15:33:38Z

.travis.yml

@@ -17,4 +17,9 @@ after_success:
  - mkdir _site public
  - python manage.py collectstatic --noinput
  - python manage.py distill-local public --force
+  - mkdir public/activity
+  - python activity/scraper.py


Move all of this before the django stuff.
Then the json could be loaded by djanjo, but more importantly the static components should all be managed by djanjo's asset system. (e.g. there are django addons that do cache busting)

jayvdb · 2017-12-07T15:37:15Z

activity/index.html

+    var labels = [];
+    var myChart;
+
+    function updateChart() {


this should also be in static JS files, so they can be linted.

pass the document elements into the functions.

jayvdb · 2017-12-07T15:39:11Z

activity/scraper.py

+from dateutil import parser, relativedelta
+
+# URL to grab all issues from
+issuesurl = "https://coala.github.io/gh-board/issues.json"  # Ignore KeywordBear


no, dont ignore the bears, they will eat you :P
derive the org_name from your working environment.
See #1 and #26

jayvdb · 2017-12-07T15:42:22Z

activity/scraper.py

+	return (d1-d2).days
+
+# Get labels for each option
+month_labels = []


you want the python std calendar library, which is unfortunately not well known (and will be necessary if we add i18n).

Also move 80% of the remaining code into a class, even if it isnt well designed, because a class will force you to do some basic design, and then the indent levels will be correct , and it is easier to shuffle code around after that has been done.

jayvdb · 2017-12-07T15:42:33Z

activity/scraper.py

+		if issue["state"] == "closed":
+			data["week"]["closed"][dys] += 1
+
+print(data)


Printed for logging purposes currently, the rest of the codebase doesn't appear to use proper logging so I just printed it to console too :P

jayvdb · 2017-12-07T15:44:42Z

activity/scraper.py

+			data["week"]["closed"][dys] += 1
+
+print(data)
+with open('activity/data.json', 'w') as fp:


os.sep, for proper cross platform support.

jayvdb · 2017-12-07T15:58:21Z

unack 0f2b4a2

gitmate-bot · 2017-12-07T20:02:01Z

static/charts.js

+function setChart(labels, openedData, closedData, type) {
+	var ctx = document.getElementById("canvas");
+
+	curChart = new Chart(ctx, {


'Chart' is not defined.

Origin: JSHintBear, Section: javascript.

gitmate-bot · 2017-12-07T20:02:02Z

static/charts.js

+function updateChart(type) {
+	if(curChart){ curChart.destroy(); }
+
+	$.getJSON({


'$' is not defined.

Origin: JSHintBear, Section: javascript.

gitmate-bot · 2017-12-07T20:03:47Z

static/charts.js

+function setChart(labels, openedData, closedData, type) {
+    var ctx = document.getElementById("canvas");
+
+    curChart = new Chart(ctx, {


'Chart' is not defined.

Origin: JSHintBear, Section: javascript.

There should be a way to tell JSHintBear (or configure jshint) to ignore Chart.

gitmate-bot · 2017-12-07T20:03:48Z

static/charts.js

+function updateChart(type) {
+    if(curChart){ curChart.destroy(); }
+
+    $.getJSON({


'$' is not defined.

Origin: JSHintBear, Section: javascript.

I think you need to configure JSHintBear to accept JQuery's $.

jayvdb · 2017-12-07T20:22:02Z

.travis.yml

@@ -3,6 +3,7 @@ python: 3.6

 env:
  global:
+  - ORG_NAME: "coala"


Not allowed to put the word coala anywhere in this repo; it must work dynamically; when forked to a different org, it must derive the org. I've also now created #29 for the existing use of coala in this file.

jayvdb · 2017-12-07T20:22:40Z

activity/scraper.py

+    print(real_data)
+    with open('static' + os.sep + 'activity-data.json', 'w') as fp:
+        json.dump(real_data, fp)
+


EOL problem here.

jayvdb · 2017-12-07T20:23:51Z

static/charts.js

+function updateChart(type) {
+    if(curChart){ curChart.destroy(); }
+
+    $.getJSON({


I think you need to configure JSHintBear to accept JQuery's $.

jayvdb · 2017-12-07T20:25:11Z

static/charts.js

+function setChart(labels, openedData, closedData, type) {
+    var ctx = document.getElementById("canvas");
+
+    curChart = new Chart(ctx, {


There should be a way to tell JSHintBear (or configure jshint) to ignore Chart.

gitmate-bot · 2017-12-07T23:47:15Z

Comment on ddf9751, file static/charts.js, line 61.

'$' is not defined.

Origin: JSHintBear, Section: javascript.

gitmate-bot · 2017-12-07T23:48:26Z

Comment on ddf9751, file static/charts.js, line 6.

'Chart' is not defined.

Origin: JSHintBear, Section: javascript.

gitmate-bot · 2017-12-07T23:48:27Z

Comment on ddf9751, file static/charts.js, line 61.

'$' is not defined.

Origin: JSHintBear, Section: javascript.

andrewda · 2017-12-08T02:34:09Z

static/charts.js

+function setChart(labels, openedData, closedData, type) {
+    var ctx = document.getElementById("canvas");
+
+    curChart = new Chart(ctx, {  // Ignore JSHintBear


You should be able to add Chart and $ to the globals by adding this to the top of the file:

/* globals $, Chart */

Thanks, worked like a charm. :)

Monal5031 · 2017-12-08T13:07:55Z

orgname.sh

+regex="github.com/[a-z0-9A-Z]*"
+git config -l | grep -o "$regex" | while read -r line ; do
+    echo "${line:11}" | xargs echo -n > org_name.txt
+done


newline here please.

blazeu · 2017-12-08T13:38:37Z

activity/index.html

+
+<body>
+    <h1>Community Activity</h1>
+    <script>loadTimeElements()</script>


Why not put it inside the same script tag below?

Actually, this is a remain of when I was serving the page using django. It's no longer possible to display the time stamp(since it's not generated via django)
I think I'll just remove this for now.

blazeu · 2017-12-08T13:43:09Z

activity/index.html

+    <script src="https://cdnjs.cloudflare.com/ajax/libs/Chart.js/2.7.1/Chart.bundle.js"></script>
+    <script src="https://ajax.googleapis.com/ajax/libs/jquery/1.12.4/jquery.min.js"></script>
+    <script src="/static/timeago.js"></script>
+    <script src="/static/charts.js"></script>


All of the <script> tag can be inside the body (below the content) so it doesn't block content.

updateChart() is called after the content anyway, so it doesn't matter.

blazeu · 2017-12-08T13:44:25Z

activity/index.html

+<head>
+    <title>Community Activity</title>
+    <script src="https://cdnjs.cloudflare.com/ajax/libs/Chart.js/2.7.1/Chart.bundle.js"></script>
+    <script src="https://ajax.googleapis.com/ajax/libs/jquery/1.12.4/jquery.min.js"></script>


Is jquery 3 not compatible with Chart.js?

Wasn't using it because it fails silently on getJSON errors(and I need logging), will change.

I believe it's .fail() in jQuery 3

yep, changing.

blazeu · 2017-12-08T13:46:50Z

static/charts.js

+        },
+        error: function(error_data) {
+            console.log("error");
+            console.log(error_data);


console.error is a bit accurate.

blazeu · 2017-12-08T14:01:14Z

static/charts.js

+    if(curChart){ curChart.destroy(); }
+
+    $.getJSON({
+        url: "/static/activity-data.json",


You can use the url directly $.getJSON("/static/activity-data.json")
and have a callback as the second argument or .done() and .fail()

Read http://api.jquery.com/jquery.getjson/#jqxhr-object

^ But it's optional.

jayvdb · 2017-12-08T14:04:13Z

orgname.sh

+
+regex="github.com/[a-z0-9A-Z]*"
+git config -l | grep -o "$regex" | while read -r line ; do
+    echo "${line:11}" | xargs echo -n > org_name.txt


I think we prefer two indents by default for shell. Maybe see what ShellCheckBear says ;-)

Seems not, .ci/deploy.sh uses 4 as well.

that was a horribly rushed script I mostly copied from other sources and fixed up; without code review; naughty me.

https://github.com/coala/coala-bears/blob/master/.ci/deps.sh
https://github.com/coala/coala/blob/master/.misc/deps.sh
(and others in those directories)

We dont run code linting on them yet. I've created coala/coala#4952 for that. (I'll publish a backlog of new tasks tomorrow)

(this script is so small it doesnt matter; you are right we dont have a convention yet, so you dont need to update this patch to follow a non-existent coding standard)

blazeu · 2017-12-08T14:13:57Z

This is just a comment.

When I check ChartJS, it does not need jQuery.

You need to stop using jQuery only for fetching things in the future :P
You can use the native Fetch API and there's 500 bytes polyfill for older browsers to support it.

For now I think it's ok since we're probably gonna need it for anything else, esp. DOM modification,
although I still prefer the native DOM API, it's getting better these days.

nalinbhardwaj · 2017-12-08T14:41:21Z

@blazeu I'm currently also using .on()(although I could just replace that by an event listener). I think it's okay for it to use jQuery ATM. (I'll switch to a minified version for less loading).

hemangsk · 2017-12-08T16:21:03Z

static/charts.js

+    $.getJSON("/static/activity-data.json",
+        function(data) {
+            var labels = data.year.labels;
+            var openedData = data.year.opened;


L65 and 66 are not needed right, its getting overwritten later inside if else anyway

jayvdb · 2017-12-08T18:08:10Z

activity/scraper.py

+from dateutil import parser, relativedelta
+
+class Scraper():
+    '''


""" for docstrings.

jayvdb · 2017-12-08T18:08:52Z

activity/scraper.py

+    '''
+
+    # Count of months/weeks/days respectively to be scraped in past.
+    month_count = 12


I am 99% sure these constants can be obtained from calendar. If not, they should be CONSTANTS at the top of the module.

hmm, I chose these constants randomly myself. They're not representative of anything, just a number I think is good for display.(so no calendar use here)

jayvdb · 2017-12-08T18:09:11Z

activity/scraper.py

+
+        # Process labels for each option
+        for x in range(self.month_count-1, -1, -1):
+            self.data["year"]["labels"].append(calendar.month_name[(self.date - relativedelta.relativedelta(months=x)).month])


line length; please ensure this is being linted.

jayvdb · 2017-12-08T18:09:40Z

activity/scraper.py

+
+        # Initialise data dicts
+        self.data = {
+            "year" : {


prefer single quotes in code.

jayvdb · 2017-12-08T18:10:28Z

activity/scraper.py

+
+    def get_data(self):
+        """
+        Get data


return type

jayvdb · 2017-12-08T18:10:46Z

activity/scraper.py

+    # URL to grab all issues from
+    org_name = open('org_name.txt').readline()
+
+    issuesurl = "http://" + org_name + ".github.io/gh-board/issues.json"


issues_url

Make bash script to automatically determine the org the repo is for. Uses git config info. Stores org's name to org_name.txt in running folder. Fixes #29 partially

Makes class Scraper to scrape http://coala.github.io/gh-board/issues.json or similar file into a JSON containing only the statistics required. Stores file at "static/activity-data.json" Closes #25

jayvdb

2 spaces for JS is the decision

Make activity/index.html to display the data, and static/charts.js for constructing the charts. Uses scraped file `static/activity-data.json`. Closes #25

done

jayvdb · 2017-12-09T16:17:00Z

ack 6b69490

When the issue data JSON is missing, an exception should occur then, instead of when failing to load the JSON. Related to coala#27

Use templates for static content, including new basic front page. Also remove unnecessary auth processor. Related to coala#27

gitmate-bot added process/pending review size/S difficulty/high initiatives/gci labels Dec 6, 2017

gitmate-bot reviewed Dec 6, 2017

View reviewed changes

jayvdb added process/wip and removed process/pending review labels Dec 7, 2017

Techievena suggested changes Dec 7, 2017

View reviewed changes

gitmate-bot added process/pending review and removed process/wip labels Dec 7, 2017

jayvdb requested changes Dec 7, 2017

View reviewed changes

jayvdb added process/wip and removed process/pending review labels Dec 7, 2017

gitmate-bot added process/pending review size/M and removed process/wip size/S labels Dec 7, 2017

gitmate-bot reviewed Dec 7, 2017

View reviewed changes

jayvdb requested changes Dec 7, 2017

View reviewed changes

jayvdb added process/wip and removed process/pending review labels Dec 7, 2017

gitmate-bot added process/pending review and removed process/wip labels Dec 7, 2017

andrewda reviewed Dec 8, 2017

View reviewed changes

Monal5031 reviewed Dec 8, 2017

View reviewed changes

blazeu requested changes Dec 8, 2017

View reviewed changes

jayvdb reviewed Dec 8, 2017

View reviewed changes

hemangsk previously requested changes Dec 8, 2017

View reviewed changes

jayvdb requested changes Dec 8, 2017

View reviewed changes

jayvdb added process/wip and removed process/pending review labels Dec 8, 2017

gitmate-bot added process/pending review and removed process/wip labels Dec 8, 2017

Nalin Bhardwaj added 2 commits December 9, 2017 04:04

orgname.sh: Make bash script to autodetect org name

b762305

Make bash script to automatically determine the org the repo is for. Uses git config info. Stores org's name to org_name.txt in running folder. Fixes #29 partially

activity/scraper.py: Make scraper for data

51769df

Makes class Scraper to scrape http://coala.github.io/gh-board/issues.json or similar file into a JSON containing only the statistics required. Stores file at "static/activity-data.json" Closes #25

jayvdb requested changes Dec 9, 2017

View reviewed changes

activity: Make frontend to display data

6b69490

Make activity/index.html to display the data, and static/charts.js for constructing the charts. Uses scraped file `static/activity-data.json`. Closes #25

jayvdb approved these changes Dec 9, 2017

View reviewed changes

jayvdb merged commit 6b69490 into coala:master Dec 9, 2017

nalinbhardwaj deleted the activity branch December 9, 2017 16:22

nalinbhardwaj mentioned this pull request Dec 9, 2017

activity/scraper: Fix reqs and indexing issues #34

Merged

jayvdb added a commit to jayvdb/community that referenced this pull request Dec 10, 2017

activity/scraper.py: Error if source data missing

66b7505

When the issue data JSON is missing, an exception should occur then, instead of when failing to load the JSON. Related to coala#27

jayvdb added a commit to jayvdb/community that referenced this pull request Dec 10, 2017

Integrate activity into django

6a835c9

Use templates for static content, including new basic front page. Also remove unnecessary auth processor. Related to coala#27

nalinbhardwaj mentioned this pull request Dec 10, 2017

Remove 'coala' from COMMIT_AUTHOR_EMAIL #29

Closed

gitmate-bot added the status/STALE label Dec 20, 2017

activity: Make activity page with graphs on issues #27

activity: Make activity page with graphs on issues #27

Conversation

nalinbhardwaj commented Dec 6, 2017 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

blazeu Dec 7, 2017 • edited Loading

Choose a reason for hiding this comment

Monal5031 Dec 7, 2017 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

nalinbhardwaj Dec 7, 2017 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jayvdb commented Dec 7, 2017

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

gitmate-bot commented Dec 7, 2017

gitmate-bot commented Dec 7, 2017

gitmate-bot commented Dec 7, 2017

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

nalinbhardwaj Dec 8, 2017 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

blazeu Dec 8, 2017 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

blazeu Dec 8, 2017 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

nalinbhardwaj Dec 8, 2017 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

blazeu commented Dec 8, 2017 • edited Loading

nalinbhardwaj commented Dec 8, 2017

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jayvdb left a comment

Choose a reason for hiding this comment

jayvdb commented Dec 9, 2017

nalinbhardwaj commented Dec 6, 2017 •

edited

Loading

blazeu Dec 7, 2017 •

edited

Loading

Monal5031 Dec 7, 2017 •

edited

Loading

nalinbhardwaj Dec 7, 2017 •

edited

Loading

nalinbhardwaj Dec 8, 2017 •

edited

Loading

blazeu Dec 8, 2017 •

edited

Loading

blazeu Dec 8, 2017 •

edited

Loading

nalinbhardwaj Dec 8, 2017 •

edited

Loading

blazeu commented Dec 8, 2017 •

edited

Loading