search

YaleSTC / shifts

Application to easily track shifts, reports, and payforms for employees.
MIT License
23 stars 18 forks source link

Task links on active_tasks page are broken #263

Closed njlxyaoxinwei closed 10 years ago

njlxyaoxinwei commented 10 years ago

The links to complete the active tasks on the /active_tasks page are not working. Also the page takes very long (around 15 seconds) to load.

njlxyaoxinwei commented 10 years ago

Breakdown of the issue

  1. Right now completing the task is realized by make_entry action. It is defined in routes.rb as a member route. However, all links to this route passes as parameter an array of one single task_id, and make_entry as defined in the tasks_controller does not use params[:id]. This confusion is why the links aren't working. There are two ways to fix this:
    • Define make_entry as a collection route
    • Or alternatively, get rid of the array parameter and modify the make_entry action using params[:id]
  2. In tasks_controller, when user is not on shift, the /active_tasks page fails because of the last filter in computing @tasks requires either an existing @report or @shift.
  3. In tasks_controller, active_tasks action (and all_allowed_tasks method) has a very slow Database query: LocGroup.all.select{ |lg| lg.users.include?(current_user) } which is slowing the action down significantly.
njlxyaoxinwei commented 10 years ago

branch 263_active_tasks fixes the first two problems by:

  1. Making make_entry a member route and modify the action
  2. If not on shift, the last filter in both active_tasks action and all_allowed_tasks method checks the day_in_week of the weekly tasks against the day of week of the current time (Time.now).

For the slow loading time, I replaced the old query with the more intuitive current_user.loc_groups, reducing the loading time of the page to around 3 seconds. However, despite their seeming equivalence, it turns out that they are not exactly the same.

I ran a script that compares results of the two expressions for every user in the anonymized database in issue #213. For 243 out of the total 341 users, the two expression return the same array of location groups. For the remaining 97 users, the only difference between the two results is that Bass Media location group is in user.loc_groups but not the other expression. I suspect that this might be an error in the database? I also tried adding a new user and the two expressions have the same result for the new user. @caseywatts