Summary

Add status reports to sonar.

Review Request #7574 — Created Aug. 8, 2015 and submitted Aug. 9, 2015, 11:35 p.m.

Information

Owner

david

Repository

student-sonar

Branch

master

Bugs

Depends On

Commit

4c04f3e...

Reviewers

Groups

student-sonar

People

Description

This is a very large change which adds a big new status reports feature to
sonar. This contains a lot of individual pieces, along with a number of
refactors to clean things up.

At a high level, the new features are:

A "My Status Reports" page which is aimed towards students. This shows a list
of the due dates that apply to them. These are color coded/labeled as either
"Submitted", "Past Due", "Due Soon", or nothing. Clicking on a due date will
open up the editor view.
An "All Status Reports" view, which is only seen by admins. This shows all of
the due dates for all groups. Users are sorted into two columns: "Submitted"
and "Not Submitted". Any dates which are in the past are color-coded either
red or green depending on whether any status reports are missing.
A status report editor view. This is pre-populated with the example text from
our welcome packet. The editor can be flipped to a "preview" mode, and has a
neato fullscreen mode that presents a side-by-side view.
A status report reading view, which just shows the rendered markdown text.
Links to the new status reports from the user detail view.

Various things have been refactored, mostly related to the "ready" state of the
application. Things seem to be migrating towards a steady state here so I'll
probably follow up later with a new view class that handles all of this. There
was also some very confusing issues with the parsing of the "me" user, so I've
switched that out to just pre-populate the userID for the current user and then
match that up once all the users are fetched from the API.

Testing Done

Exercised all of the functionality a ton.

Files

Issues

Description	From	Last Updated
Leftover from debugging?	brennie	Aug. 9, 2015, 11:51 a.m.
You're inconsistent with spacing between top-level definitions.	brennie	Aug. 9, 2015, 11:51 a.m.
You can use submitted.forEach here.	brennie	Aug. 9, 2015, 11:52 a.m.
Leftover from debugging?	brennie	Aug. 9, 2015, 11:52 a.m.
Use an arrow function.	brennie	Aug. 9, 2015, 11:54 a.m.
Use an arrow function.	brennie	Aug. 9, 2015, 11:54 a.m.
Use an arrow function.	brennie	Aug. 9, 2015, 11:55 a.m.
Do we care about line width here?	brennie	Aug. 9, 2015, 11:56 a.m.
This is indented strangely. I would expect the body to be indented relative to the function keyword.	brennie	Aug. 9, 2015, 11:56 a.m.
Same here.	brennie	Aug. 9, 2015, 11:57 a.m.
Shouldn't this be this.get('users').get(window.userID) ?	brennie	Aug. 9, 2015, 11:57 a.m.
Blank line between these.	brennie	Aug. 9, 2015, 11:58 a.m.
Shouldn't this use <%- -> ?	brennie	Aug. 9, 2015, noon
Should 200 be a constant somewhere?	brennie	Aug. 9, 2015, 12:01 p.m.
If this is listening for resize events, maybe we want to _.debounce instead of _.throttle ?	brennie	Aug. 9, 2015, 12:01 p.m.
Use an arrow function.	brennie	Aug. 9, 2015, 1:17 p.m.
Should this use <%- ?	brennie	Aug. 9, 2015, 12:02 p.m.
Assuming dueDates is an array, you can use dueDates.each((dueDate) => { // ... });	brennie	Aug. 9, 2015, 12:03 p.m.
I'm not very familiar with mongo/mongoose, but shouldn't this be a Date ? Or is this a foreign key-esque pointer …	brennie	Aug. 9, 2015, 12:03 p.m.
Blank line between these.	brennie	Aug. 9, 2015, 10:51 p.m.

Tool: PEP8 Style Checker
Ignored Files:
    package.json
    lib/frontend/config.js
    lib/frontend/application.js
    lib/frontend/user-detail-view.js
    lib/frontend/util.js
    lib/api.js
    views/student-list.handlebars
    lib/frontend/status-report-editor-view.js
    lib/frontend/all-status-reports-view.js
    lib/frontend/sidebar-view.js
    lib/frontend/calendar-view.js
    lib/frontend/application-view.js
    lib/frontend/status-report-checker-view.js
    lib/schema.js
    lib/frontend/my-status-reports-view.js
    css/style.less
    lib/frontend/status-report-reading-view.js
    lib/routes.js
    lib/frontend/models.js



Tool: Pyflakes
Ignored Files:
    package.json
    lib/frontend/config.js
    lib/frontend/application.js
    lib/frontend/user-detail-view.js
    lib/frontend/util.js
    lib/api.js
    views/student-list.handlebars
    lib/frontend/status-report-editor-view.js
    lib/frontend/all-status-reports-view.js
    lib/frontend/sidebar-view.js
    lib/frontend/calendar-view.js
    lib/frontend/application-view.js
    lib/frontend/status-report-checker-view.js
    lib/schema.js
    lib/frontend/my-status-reports-view.js
    css/style.less
    lib/frontend/status-report-reading-view.js
    lib/routes.js
    lib/frontend/models.js

```
This looks awesome :D
```
lib/frontend/all-status-reports-view.js (Diff revision 1)
The issue has been resolved. Show all issues
```
Leftover from debugging?
```

lib/frontend/all-status-reports-view.js (Diff revision 1)

The issue has been resolved. Show all issues

You're inconsistent with spacing between top-level definitions.

lib/frontend/all-status-reports-view.js (Diff revision 1)

The issue has been resolved. Show all issues

You can use submitted.forEach here.

lib/frontend/all-status-reports-view.js (Diff revision 1)
The issue has been resolved. Show all issues
```
Leftover from debugging?
```
lib/frontend/all-status-reports-view.js (Diff revision 1)
The issue has been resolved. Show all issues
```
Use an arrow function.
```
lib/frontend/all-status-reports-view.js (Diff revision 1)
The issue has been resolved. Show all issues
```
Use an arrow function.
```
lib/frontend/all-status-reports-view.js (Diff revision 1)
The issue has been resolved. Show all issues
```
Use an arrow function.
```
lib/frontend/all-status-reports-view.js (Diff revision 1)
The issue has been dropped. Show all issues
```
Do we care about line width here?
```
1. david Aug. 9, 2015, 12:03 p.m.
  I think I'm going to make an executive decision for this codebase and say "no".

lib/frontend/application-view.js (Diff revision 1)

The issue has been resolved. Show all issues

This is indented strangely. I would expect the body to be indented relative to the function keyword.

lib/frontend/application-view.js (Diff revision 1)

The issue has been resolved. Show all issues

Same here.

lib/frontend/application.js (Diff revision 1)
The issue has been resolved. Show all issues
```
Shouldn't this be this.get('users').get(window.userID) ?
```
lib/frontend/my-status-reports-view.js (Diff revision 1)
```
Missing a period :)
```
lib/frontend/my-status-reports-view.js (Diff revision 1)
The issue has been resolved. Show all issues
```
Blank line between these.
```
lib/frontend/status-report-editor-view.js (Diff revision 1)
The issue has been resolved. Show all issues
```
Shouldn't this use <%- -> ?
```
1. david Aug. 9, 2015, 12:03 p.m.
  Arr, I always get those backwards.
lib/frontend/status-report-editor-view.js (Diff revision 1)
The issue has been resolved. Show all issues
```
Should 200 be a constant somewhere?
```

lib/frontend/status-report-editor-view.js (Diff revision 1)

The issue has been resolved. Show all issues

If this is listening for resize events, maybe we want to _.debounce instead of _.throttle ?

lib/frontend/status-report-editor-view.js (Diff revision 1)

The issue has been dropped. Show all issues

Use an arrow function.

david Aug. 9, 2015, 12:03 p.m.

This is multiple statements so an arrow function isn't appropriate.

brennie Aug. 9, 2015, 1:14 p.m.

I still feel this should be a multistatement arrow function.

lib/frontend/status-report-reading-view.js (Diff revision 1)
The issue has been resolved. Show all issues
```
Should this use <%- ?
```

lib/frontend/user-detail-view.js (Diff revision 1)

The issue has been resolved. Show all issues

Assuming dueDates is an array, you can use

dueDates.each((dueDate) => {
    // ...
});

lib/schema.js (Diff revision 1)

The issue has been resolved. Show all issues

I'm not very familiar with mongo/mongoose, but shouldn't this be a Date ? Or is this a foreign key-esque pointer to a CalendarEntry. If so, can we document it as such?

Commit:

1ad5bee1241353a587c5ed7a75f39590a1703ee2

7e90bb7955154bd7ab407c85c6538d9802691d21

Diff:

Revision 2 (+1134 -185)

Show changes

	package.json
	css/style.less
	lib/api.js
	lib/routes.js
	lib/schema.js
	lib/frontend/all-status-reports-view.js
	lib/frontend/application.js
	lib/frontend/application-view.js
	12 more

Tool: Pyflakes
Ignored Files:
    package.json
    lib/frontend/config.js
    lib/frontend/application.js
    lib/frontend/user-detail-view.js
    lib/frontend/util.js
    lib/api.js
    views/student-list.handlebars
    lib/frontend/status-report-editor-view.js
    lib/frontend/all-status-reports-view.js
    lib/frontend/sidebar-view.js
    lib/frontend/calendar-view.js
    lib/frontend/application-view.js
    lib/frontend/confirm.js
    lib/frontend/status-report-checker-view.js
    lib/schema.js
    lib/frontend/my-status-reports-view.js
    css/style.less
    lib/frontend/status-report-reading-view.js
    lib/routes.js
    lib/frontend/models.js



Tool: PEP8 Style Checker
Ignored Files:
    package.json
    lib/frontend/config.js
    lib/frontend/application.js
    lib/frontend/user-detail-view.js
    lib/frontend/util.js
    lib/api.js
    views/student-list.handlebars
    lib/frontend/status-report-editor-view.js
    lib/frontend/all-status-reports-view.js
    lib/frontend/sidebar-view.js
    lib/frontend/calendar-view.js
    lib/frontend/application-view.js
    lib/frontend/confirm.js
    lib/frontend/status-report-checker-view.js
    lib/schema.js
    lib/frontend/my-status-reports-view.js
    css/style.less
    lib/frontend/status-report-reading-view.js
    lib/routes.js
    lib/frontend/models.js

Change Summary:

Use Array.forEach and fat arrow functions everywhere that's appropriate.

Commit:

7e90bb7955154bd7ab407c85c6538d9802691d21

4c04f3e78a01fc16e628c4a3d330b55d73bfa570

Diff:

Revision 3 (+1162 -227)

Show changes

	package.json
	css/style.less
	lib/api.js
	lib/routes.js
	lib/schema.js
	lib/frontend/all-status-reports-view.js
	lib/frontend/application.js
	lib/frontend/application-view.js
	14 more

Tool: PEP8 Style Checker
Ignored Files:
    lib/frontend/user-detail-view.js
    lib/frontend/confirm.js
    lib/frontend/config.js
    lib/frontend/application-view.js
    css/style.less
    lib/routes.js
    lib/frontend/models.js
    package.json
    lib/frontend/my-status-reports-view.js
    lib/frontend/sidebar-view.js
    lib/schema.js
    lib/frontend/util.js
    lib/frontend/collection-view.js
    lib/frontend/status-report-editor-view.js
    lib/frontend/application.js
    lib/frontend/header-view.js
    lib/frontend/calendar-view.js
    lib/api.js
    views/student-list.handlebars
    lib/frontend/all-status-reports-view.js
    lib/frontend/status-report-checker-view.js
    lib/frontend/status-report-reading-view.js



Tool: Pyflakes
Ignored Files:
    lib/frontend/user-detail-view.js
    lib/frontend/confirm.js
    lib/frontend/config.js
    lib/frontend/application-view.js
    css/style.less
    lib/routes.js
    lib/frontend/models.js
    package.json
    lib/frontend/my-status-reports-view.js
    lib/frontend/sidebar-view.js
    lib/schema.js
    lib/frontend/util.js
    lib/frontend/collection-view.js
    lib/frontend/status-report-editor-view.js
    lib/frontend/application.js
    lib/frontend/header-view.js
    lib/frontend/calendar-view.js
    lib/api.js
    views/student-list.handlebars
    lib/frontend/all-status-reports-view.js
    lib/frontend/status-report-checker-view.js
    lib/frontend/status-report-reading-view.js

Ship it!

lib/frontend/all-status-reports-view.js (Diff revision 3)
The issue has been resolved. Show all issues
```
Blank line between these.
```

Status:: Completed
Change Summary:: Pushed to master (ee02373)