Overhaul dashboard, add Todos, and improve typography #422

marcua · 2017-10-15T21:46:15Z

This PR does three things:

Replaces the dashboard cards with tables for Active/Pending/Completed tasks
Adds a Todo, which any collaborator on a project can create/check off, but is assigned to particular tasks/roles on a project
Some CSS improvements, including moving our font stack over to the system font stack. Night and day improvement to our typography.

Great example of a terrible PR in that it does three things at once. This was the result of a B12 hackathon, and I'm getting it out there rather than keeping it under wraps. If reviewing it is too much of a headache, I can break it into smaller PRs.

TODO

Small amount of CSS work on the todo list
Truncate details in table if they get too long
Try to integrate angular-smart-table for sorting/searching/pagination. Can always go in a v2.

marcua · 2017-10-15T22:08:44Z

orchestra/static/orchestra/dashboard/tasktable.html

+    </div>
+  </div>
+</section>
+<!--/section-panel tasks-section -->


marcua · 2017-10-17T03:02:08Z

orchestra/static/orchestra/common/js/orchestra.services.es6.js

@@ -64,14 +64,14 @@ export function orchestraService () {



the only thing that changed in style.css above is the font stack moving over to the system fonts. the diff is large because of whitespace cleanup.

kkamalov · 2017-10-17T07:00:24Z

orchestra/interface_api/project_management/views.py

+    """
+    This function is used by both the project management interface
+    (project admins only) and for providing project information to
+    experts (only to experts associated with a project). We enfoce


kkamalov · 2017-10-17T07:01:07Z

orchestra/interface_api/project_management/views.py

    project_id = load_encoded_json(request.body)['project_id']
+    worker = Worker.objects.get(user=request.user)


get_object_or_404

kkamalov · 2017-10-17T07:02:34Z

orchestra/migrations/0071_auto_20171015_2303.py

@@ -0,0 +1,21 @@
+# -*- coding: utf-8 -*-


maybe put in 1 migration?

kkamalov · 2017-10-17T07:05:37Z

orchestra/static/orchestra/common/js/orchestra.services.es6.js

@@ -64,14 +64,14 @@ export function orchestraService () {

 export function orchestraTasks ($http) {
  'ngAnnotate'
+  const activeState = (task) => task.state === 'in_progress' || task.state === 'returned' || task.state === 'just_added'


['just_added', 'in_progress', 'returned'].indexOf(task.state) !== -1

kkamalov · 2017-10-17T07:06:33Z

orchestra/static/orchestra/common/js/orchestra.services.es6.js

-      }
-      return tasks
-    },
+    allTasks: function () { return this.tasks },


should this function () be replaced with () => below?

i initially did just this to "modernize" this part of the code, but ran into an issue---the object syntax we're using to create these functions on orchestraTasks requires this be bound in a way that function definitions do, and modern () => definitions do not.

i propose we modernize the whole module in a separate PR :)

thisisdhaas · 2017-10-17T15:30:24Z

orchestra/interface_api/project_management/views.py

+    worker = get_object_or_404(Worker, user=request.user)
+    if not (is_project_admin(request.user) or
+            worker.assignments.filter(task__project=project_id).exists()):
+        raise BadRequest('Permission denied')


should be a 403 or 404 on Permission denied, not a 400?

Also looks like you created nice permission classes for this in your PR, can they be reused here?

There's only one exception type for jsonviews: https://github.com/jsocol/django-jsonview/blob/master/jsonview/exceptions.py#L1

permission_classes by default can only be ANDed, not ORed. This project addresses that, and we can consider depending on it in a followon PR

just to win an argument, just return Django's PermissionDenied.

That project looks useful, but I don't really think our security needs are complex enough to merit adding dependencies.

thisisdhaas · 2017-10-17T15:33:13Z

orchestra/models/__init__.py

@@ -14,4 +14,5 @@
 from orchestra.models.core.models import Iteration  # noqa
 from orchestra.models.core.models import TimeEntry  # noqa
 from orchestra.models.core.models import TaskTimer  # noqa
+from orchestra.models.core.models import Todo  # noqa


I feel like you complained about this to me :-)

If you leave this here, the way to get rid of the noqas is to define __all__, which makes some sense if the point of this code is to gather a bunch of symbols in one module. (Though python people fight about it a lot).

did __all__

overall i dislike this model of doing things, but refactors/cleanups on this PR are out of the question :)

Nit: but now you should be able to remove all the # noqas.

thisisdhaas · 2017-10-17T15:36:05Z

orchestra/models/core/mixins.py

@@ -61,6 +61,9 @@ def __str__(self):
            self.phone
        )

+    def formatted_slack_username(self):
+        return '<@{}|{}>'.format(self.slack_user_id, self.slack_username)
+


Deprecated (see https://api.slack.com/changelog/2017-09-the-one-about-usernames), they want you to just pass the user_id and let them figure out the display name.

Updated. Thanks! I didn't know that.

thisisdhaas · 2017-10-17T15:45:42Z

orchestra/todos/views.py

+        return False
+
+
+class TodoList(generics.ListCreateAPIView):


No good viewsets to let you combine the list and detail views?

Not that I could find :P

thisisdhaas · 2017-10-17T15:47:10Z

orchestra/utils/task_lifecycle.py

+                if next_todo:
+                    next_todo_description = next_todo.description
+                num_todos = task_assignment.task.todos.count()
+                should_be_active = ((num_todos == 0) or


why active if there are no todos?

added comment explaining why :)

thisisdhaas · 2017-10-19T03:02:53Z

orchestra/static/orchestra/dashboard/tasktable.html

-                <td>{{task.detail}}</td>
-                <td>{{task.next_todo_description}}</td>
+                <td>{{task.detail|limitTo:50}}{{task.detail.length > 50 ? '...' : ''}}</td>
+                <td>{{task.next_todo_description|limitTo:50}}{{task.next_todo_description.length > 50 ? '...' : ''}}</td>


Is there pagination? Or does this make it impossible to see > 50 tasks? (i.e., what if Kirstin needs to work on task 51?)

this truncates text to 50 characters so the table doesn't wrap

marcua added 15 commits October 10, 2017 01:14

Todo model & bad todo list

b81e9bf

todos subapp

0237419

bad todo directive

b7ccc64

Task cards -> table

09b6c5a

todo list works

6dd0ce8

Add scss

035dee7

Slack notifications

c9b305a

Send to pending state

2e7925d

Active can be pending :)

98cdef2

not this one

2a44818

Enter submits todo list

fecfc72

Reload tasks when visiting dashboard

89299f4

Update typography

b64f307

todo css and lint

3ed8ad2

fix test

037065f

marcua commented Oct 15, 2017

View reviewed changes

marcua added 8 commits October 16, 2017 03:03

Fix permissions on endpoints

8d80148

Test new endpoints

25ec1db

Table size

dc0d7e9

Unused import

7ff2759

description field width

f3e74dc

Display step names, not slugs

eb909a8

Clean up code and fix test

921e92f

self-review

79ffeec

marcua changed the title ~~[WIP] Overhaul dashboard, add TODOs, and improve typography~~ Overhaul dashboard, add TODOs, and improve typography Oct 17, 2017

marcua requested a review from kkamalov October 17, 2017 02:57

marcua changed the title ~~Overhaul dashboard, add TODOs, and improve typography~~ Overhaul dashboard, add Todos, and improve typography Oct 17, 2017

marcua commented Oct 17, 2017

View reviewed changes

kkamalov reviewed Oct 17, 2017

View reviewed changes

Code review

e5ad336

thisisdhaas reviewed Oct 17, 2017

View reviewed changes

marcua added 3 commits October 19, 2017 02:30

Code review and style

cb2665d

Truncate table

2e15f00

nonbreaking space

40f8eaa

thisisdhaas reviewed Oct 19, 2017

View reviewed changes

thisisdhaas approved these changes Oct 19, 2017

View reviewed changes

marcua added 2 commits October 19, 2017 03:26

last code review

8d5193f

compiled files

f9ebce2

marcua merged commit 3a63440 into master Oct 20, 2017

marcua deleted the dashboard-todo-overhaul branch October 20, 2017 13:36

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Overhaul dashboard, add Todos, and improve typography #422

Overhaul dashboard, add Todos, and improve typography #422

marcua commented Oct 15, 2017 •

edited by devChuk

Loading

marcua Oct 15, 2017

marcua Oct 17, 2017

kkamalov Oct 17, 2017

marcua Oct 17, 2017

kkamalov Oct 17, 2017

marcua Oct 17, 2017

kkamalov Oct 17, 2017

marcua Oct 17, 2017

kkamalov Oct 17, 2017

marcua Oct 17, 2017

kkamalov Oct 17, 2017

marcua Oct 17, 2017

thisisdhaas Oct 17, 2017

thisisdhaas Oct 17, 2017

marcua Oct 19, 2017

marcua Oct 19, 2017 •

edited

Loading

thisisdhaas Oct 19, 2017 •

edited

Loading

marcua Oct 19, 2017

thisisdhaas Oct 17, 2017

marcua Oct 19, 2017 •

edited

Loading

thisisdhaas Oct 19, 2017

marcua Oct 19, 2017

thisisdhaas Oct 17, 2017

marcua Oct 19, 2017 •

edited

Loading

thisisdhaas Oct 17, 2017

marcua Oct 19, 2017

thisisdhaas Oct 17, 2017

marcua Oct 19, 2017

thisisdhaas Oct 19, 2017

marcua Oct 19, 2017

		project_id = load_encoded_json(request.body)['project_id']
		worker = Worker.objects.get(user=request.user)

Overhaul dashboard, add Todos, and improve typography #422

Overhaul dashboard, add Todos, and improve typography #422

Conversation

marcua commented Oct 15, 2017 • edited by devChuk 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

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

marcua Oct 19, 2017 • edited Loading

Choose a reason for hiding this comment

thisisdhaas Oct 19, 2017 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

marcua Oct 19, 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

marcua Oct 19, 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

marcua commented Oct 15, 2017 •

edited by devChuk

Loading

marcua Oct 19, 2017 •

edited

Loading

thisisdhaas Oct 19, 2017 •

edited

Loading

marcua Oct 19, 2017 •

edited

Loading

marcua Oct 19, 2017 •

edited

Loading