*8319* Allow assistant roles to access the submission archive #46
Conversation
|
|
||
| if ($contextId && is_int($contextId)) | ||
| $params[] = (int) $contextId; | ||
| if ($status && is_int($status)) $status = (int) $status; |
There was a problem hiding this comment.
If $status is now optional, there should be a default (null) in the parameter list. Also, I'm starting to work through Doxygen and improve our code self-documentation, so it's a good time to start adding missing parameter descriptions when working with code.
There was a problem hiding this comment.
Status is not optional. I can add the descriptions.
There was a problem hiding this comment.
Looking again, I can see this addresses my concern about int casts. I still prefer to have the int casts inline with the SQL if possible -- it's easy to tell at a glance that it's being done. But the "if ($status)" part of the check looks like it's only relevant to optional parameters -- is it needed here if $status is not optional?
There was a problem hiding this comment.
I will move the int casts into the SQL and remove the first part of the conditional.
|
OTOH I don't think this works for reviewers; they aren't tracked using stage_assignments, but rather with review_assignments. |
|
I see. I didn't know that reviewer assignments are tracked in review_assignments. I will incorporate this too. |
|
I incorporated the requested changes. |
*8319* Allow assistant roles to access the submission archive
See: http://pkp.sfu.ca/bugzilla/show_bug.cgi?id=8319