Skip to content

Comments

Overdue tasks page#89

Open
FrankieFabuloso wants to merge 7 commits intomasterfrom
overdueTasksPage
Open

Overdue tasks page#89
FrankieFabuloso wants to merge 7 commits intomasterfrom
overdueTasksPage

Conversation

@FrankieFabuloso
Copy link
Contributor

Fixes 76 .

Overview

Adds view page for overdue tasks linked from admin dasheboard.

Data Model / DB Schema Changes

N/A

Environment / Configuration Changes

N/A

Notes

N/A

}),
credentials: 'same-origin'
}
fetch('api/task/all', options)
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

import { fetchURL, toStandardDate } from '../../../common/utils'
import moment from 'moment'

export default class OverdueTaskDashboard extends Component {
Copy link
Collaborator

@punitrathore punitrathore Mar 2, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think we're overloading the use of the name Dashboard. Please change to OverdueTask


router.get('/all', (request, response, next) => {
Promise.all([task.getAll(), user.findAll() ])
.then(results => response.json(results))
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Instead of returning all the tasks & users, can you write a new SQL query that fetches the overdue tasks(joined with the user's github handle)? There is too much work happening on the front end that can just be reduced to a SQL query

findEmailByUUID( user_id ){
for( let task of this.state.tasks ){
if( task.user_id == user_id ){
if(task.email !== null) {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

these 2 if statements can be combined

}


findEmailByUUID( user_id ){
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

why does this function exist? Since you are now fetching the task, and the associated user of the task(by joining the task and user table), the email should be a property on the task object.

@jaredatron
Copy link

Is this still waiting on a review?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants