3.4 updates#21
Conversation
ctgraham
left a comment
ctgraham
left a comment
There was a problem hiding this comment.
Looks good. One potentially unintentional change, low priority.
|
@ctgraham and/or @wopsononock I just wanted another quick onceover to make sure these are good to merge the pull request |
ctgraham
left a comment
ctgraham
left a comment
There was a problem hiding this comment.
Looks like _getBackwardsCompatibleContext() is now just cruft and can be removed.
| case 'settings': | ||
| $contextID = (!is_null($this->_getBackwardsCompatibleContext()) ? $this->_getBackwardsCompatibleContext()->getId() : PKPApplication::CONTEXT_SITE); | ||
| $context = Application::get()->getRequest()->getContext(); | ||
| $contextID = $context->getId(); |
There was a problem hiding this comment.
There is some risk to this change.
Previously, we checked to see if getContext() returned null, and if so, substituted PKPApplication::CONTEXT_SITE for the context id. The current code presumes that $context must not be null.
This can be checked by trying to manage the plugin from the Site level, rather than from the journal level. Or, we could re-introduce the if statement presumptively.
2 participants
No description provided.