Skip to content

Commit

Permalink
[bvl_feedback] Remove redundant bvlFeedbackPossible method
Browse files Browse the repository at this point in the history
The middleware makes a call to NDB_BVL_Feedback::bvlFeedbackPossible
to determine whether the feedback icon should be displayed. This
function checks the test_name (based on the page passed by the middleware) and user permission to filter out who should not see the
icon. However, permission is already checked by the middleware, and
the test_name check breaks the loading of the icon for instruments
since the page passed by the middleware is the NDB_BVL_Instrument
(sub)class, not the "instruments" module.

The middleware checks for the existence of a "getFeedbackPanel"
method on the page (which is more robust, and exists on the instrument class),
so the test name check is unnecessary and only breaks instrument feedback.
The result is that the template variable can just be set to "true" if we're in
the if statement without loss of functionality, and the static method
(which is unused anywhere else) can be removed.

Resolves #7665
  • Loading branch information
driusan committed Oct 29, 2021
1 parent 2846a34 commit aec6684
Show file tree
Hide file tree
Showing 2 changed files with 1 addition and 33 deletions.
30 changes: 0 additions & 30 deletions php/libraries/NDB_BVL_Feedback.class.inc
Original file line number Diff line number Diff line change
Expand Up @@ -1049,34 +1049,4 @@ class NDB_BVL_Feedback
$record = $db->pselectOne($query, $param);
return $record;
}

/**
* This function is currently used to check whether or not to render
* the bvl_feedback_icon in main.tpl. It checks if the user has the
* bvl feedback permission first, if the user has the permissions it
* then checks if the user is on the timepoint_list, instrument_list
* or on any instrument page.
*
* Returns false if the user doesn't have the appropriate permissions
* or is not on one of the appropriate pages to return bvl_feedback.
*
* @param string $test_name The name of the test to be checked.
*
* @return bool
* @throws ConfigurationException
* @throws Exception
* @author Evan McIlroy <evanmcilroy@gmail.com>
*/
static function bvlFeedbackPossible(string $test_name): bool
{
if (\User::singleton()->hasPermission('bvl_feedback')) {
if (($test_name === "timepoint_list")
|| ($test_name === "instrument_list")
|| ($test_name === "instruments")
) {
return true;
}
}
return false; //returning false when user is not on appropriate study site
}
}
4 changes: 1 addition & 3 deletions src/Middleware/UserPageDecorationMiddleware.php
Original file line number Diff line number Diff line change
Expand Up @@ -154,9 +154,7 @@ function ($a, $b) {
$sessionID
);

$tpl_data['bvl_feedback'] = \NDB_BVL_Feedback::bvlFeedbackPossible(
$this->PageName
);
$tpl_data['bvl_feedback'] = true;
}

// This shouldn't exist. (And if it does, it shouldn't reference
Expand Down

0 comments on commit aec6684

Please sign in to comment.