Closed (fixed)
Project:
Project issue tracking
Version:
5.x-2.x-dev
Component:
Issues
Priority:
Normal
Category:
Bug report
Assigned:
Reporter:
Created:
4 May 2008 at 01:48 UTC
Updated:
9 Nov 2008 at 18:52 UTC
Jump to comment: Most recent file
Comments
Comment #1
hunmonk commented!isset($assigned_choices)will always pass. second, if we're trying to make sure that the submitted user is a valid assignee for the new project, won't the FAPI choice-checker take care of that?// Build the HTML output for the rid select.-- this code comment looks wrong for building the assigned selectfunction project_issue_update_project($pid, $cid = NULL, $rid = NULL, $assigned_uid = NULL, $issue_nid = NULL)-- the order of the args there seems goofy to me. i think it would be less confusing if we reorganized those instead of just tacking the new args on the end. perhaps pid, nid, cid, rid, uid? just some order that makes intuitive sense...Comment #2
hunmonk commentedattached patch:
project_issue_update_project()to something i think is more sensible.project_issue_update_project()node_load()for loading the issue inproject_issue_update_project()to make sure it checks that the passed nid belongs to a project issue. also changed thenode_load()for the project loading for consistency.tested, and this all seems to be working beautifully now. gonna leave it CNR for a bit in case anybody else wants to jab at it...
Comment #3
dwwWhy are we doing this:
in light of this: #311369: Project loading could use the static cache ?
Is the idea that the static cache doesn't help us in this AJAX menu handler, since we're only looking up a single project per "page load" anyway?
Otherwise, patch looks good visually. I haven't tested it yet.
Comment #4
hunmonk commentedno, the idea is that we have no real validation on the pid, we're just trusting the url that the AJAX request sends.
and also what you said about single page loads. :)
Comment #5
aclight commentedThis patch fixes the doxygen of project_issue_update_project() to start with a one line summary and also fixes the @param assigned_uid part. In addition, removes a blank line from the start of the function.
Comment #6
aclight commentedwith updated comment
Comment #7
aclight commentedunified diff this time
Comment #8
dwwReviewed and tested. Please commit to DRUPAL-5--2 and HEAD. Thanks!
Comment #9
hunmonk commentedcommitted to 5.x-2.x, HEAD, deployed on d.o
Comment #10
Anonymous (not verified) commentedAutomatically closed -- issue fixed for two weeks with no activity.