search

maxrossello / redmine_extended_watchers

Grant additional issue and project view permissions to watcher users
GNU General Public License v3.0
44 stars 20 forks source link

Fixed error 500 #10

Closed xaionaro closed 9 years ago

maxrossello commented 9 years ago

Hi, can you please explain the use case and rationale for the fix? Thank you!

xaionaro commented 9 years ago

Well… I'm just using your plugin on my hacky Redmine and got an error 500 while trying to look at an issue. 

  Rendered issues/_action_menu.html.erb (5.4ms)
  Rendered issues/show.html.erb within layouts/base (51.6ms)
Completed 500 Internal Server Error in 442.4ms

ActionView::Template::Error (Mysql::Error: You have an error in your SQL syntax; check the manual that corresponds to your MariaDB server version for the right syntax to use near ',)  OR issues.id IN (44,43,67,70,81,82,91,94,100,102,103,125,126,127,77,134,135,' at line 1: SELECT `issues`.`id` AS t0_r0, `issues`.`tracker_id` AS t0_r1, `issues`.`project_id` AS t0_r2, `issues`.`subject` AS t0_r3, `issues`.`description` AS t0_r4, `issues`.`due_date` AS t0_r5, `issues`.`category_id` AS t0_r6, `issues`.`status_id` AS t0_r7, `issues`.`assigned_to_id` AS t0_r8, `issues`.`priority_id` AS t0_r9, `issues`.`fixed_version_id` AS t0_r10, `issues`.`author_id` AS t0_r11, `issues`.`lock_version` AS t0_r12, `issues`.`created_on` AS t0_r13, `issues`.`updated_on` AS t0_r14, `issues`.`start_date` AS t0_r15, `issues`.`done_ratio` AS t0_r16, `issues`.`estimated_hours` AS t0_r17, `issues`.`parent_id` AS t0_r18, `issues`.`root_id` AS t0_r19, `issues`.`lft` AS t0_r2 [... cutted ...]

So I just found the problem piece ("OR issues.assigned_toid IN (3,,,)_ OR issues.id") in the SQL query, found the code responsible for that and added the quotes [(3,,,) -> ("3","","","")]. After that I was able to see the problem issue (no 500 error anymore).

So I can't explain how to repeat the bug and is this fix really correct. Just it's a recipe that fixed my problem with the plugin, so I decided to share it with the author. Sorry for my uselessness :)

maxrossello commented 9 years ago

Hey, bug fixing is very useful! Don't get me wrong :-)

I just need to guarantee the code against a regression hence would like to replicate the bug. I will have a general try with the patch, then will merge.

Thank you!

maxrossello commented 9 years ago

It's not clear to me why the expression

user.groups.map(&:id)

gives empty array components to you, maybe it could be due to different underlying db, but in any case it seems your patch increases robustness, hence I merged it.

Thank you

xaionaro commented 9 years ago

It's not clear to me why the expression gives empty array components to you, maybe it could be due to different underlying db, but in any case it seems your patch increases robustness, hence I merged it.

I work with DB directly. So I may corrupt some data bypass the Redmine.

Thank you

Thanks you. There's no need to "git rebase" every time if my patch is merged to the upstream :)