Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: add institution info for network/users.php page #159

Draft
wants to merge 6 commits into
base: dev
Choose a base branch
from

Conversation

richard015ar
Copy link
Contributor

@richard015ar richard015ar commented Apr 17, 2024

Issue: #42

This PR modifies the WP network user list in order to add an "Institution" sortable column with the institution name. If the user displayed in the list does not belong to any institution, it displays Unnasigned text.

Testing case

  • As a super admin / network manager, go to Users > Network Users.
  • Make sure the Institution column is displayed after the Email one with the correct value.
  • Sort by Institution column and make sure the ordering is correct.
  • Log in as an Institutional Manager and make sure only the users belonging to the current institution are displayed.
  • Edit / delete users and make sure those actions are performed as expected.
  • Try to edit or delete a non-allowed user by going to <NETWORK_URL>/network/user-edit.php?user_id=<USER_ID_FROM_OTHER_INSTITUTION>. You should not be allowed to do that.
  • Make sure the Super Admin filter at the top of the table is not available.
  • The totals at the top of the table must match with the total number of users.

@@ -65,7 +65,7 @@ public function handlePagesPermissions($institution, $institutionalManagers, $in
*/

add_filter('can_edit_network', function ($canEdit) use ($allowedBooks) {
if (is_network_admin() && !in_array($_REQUEST['id'], $allowedBooks)) {
if (is_network_admin() && isset($_REQUEST['id']) && !in_array($_REQUEST['id'], $allowedBooks)) {
Copy link
Contributor Author

@richard015ar richard015ar Apr 17, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I got PHP Warning: Undefined array key "id" in /app/web/app/plugins/pressbooks-multi-institution/src/Services/PermissionsManager.php on line 68 when going to user.php (standard list) as an IM.

@codecov-commenter
Copy link

codecov-commenter commented Apr 17, 2024

Codecov Report

Merging #159 (39be638) into dev (168af33) will decrease coverage by 0.05%.
The diff coverage is 77.19%.

Additional details and impacted files
@@             Coverage Diff              @@
##                dev     #159      +/-   ##
============================================
- Coverage     82.63%   82.58%   -0.05%     
- Complexity      289      307      +18     
============================================
  Files            32       33       +1     
  Lines          1589     1642      +53     
============================================
+ Hits           1313     1356      +43     
- Misses          276      286      +10     

@richard015ar richard015ar changed the title Feat/open source users list feat: add institution info for network/users.php page Apr 17, 2024
@richard015ar richard015ar marked this pull request as ready for review April 18, 2024 12:44
@@ -188,8 +188,10 @@ private function currentUserHasAccess(string $currentPageParam, array $allowedBo
$institutionalUsers = apply_filters('pb_institutional_users', []);

if ($currentPageParam === 'pb_network_analytics_userlist' || $pagenow === 'users.php' || $pagenow === 'user-edit.php') {
if (isset($_REQUEST['user_id']) && in_array($_REQUEST['user_id'], $institutionalUsers)) {
$isAccessAllowed = true;
$isAccessAllowed = true;
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

we can go to users.php list without having any $_REQUEST value as it has in edit/delete actions. In that case, we want to allow it.

@richard015ar richard015ar marked this pull request as draft April 22, 2024 18:08
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants