Skip to content

Commit ed7f9ac

Browse files
dkotterdkotter
authored
Merge pull request #692 from 10up/enhancement/691
Removed "Enable role-based access" and "Enable user-based access" settings options from the feature settings and set to be enabled by default.
2 parents 67a0165 + c05789c commit ed7f9ac

10 files changed

+120
-194
lines changed

‎includes/Classifai/Admin/UserProfile.php

+11-16
Original file line numberDiff line numberDiff line change
@@ -147,8 +147,6 @@ public function get_allowed_features( int $user_id ): array {
147147
continue;
148148
}
149149

150-
$role_based_access_enabled = isset( $settings['role_based_access'] ) && 1 === (int) $settings['role_based_access'];
151-
$user_based_access_enabled = isset( $settings['user_based_access'] ) && 1 === (int) $settings['user_based_access'];
152150
$user_based_opt_out_enabled = isset( $settings['user_based_opt_out'] ) && 1 === (int) $settings['user_based_opt_out'];
153151

154152
// Bail if user opt-out is not enabled.
@@ -158,25 +156,22 @@ public function get_allowed_features( int $user_id ): array {
158156

159157
// Check if user has access to the feature by role.
160158
$allowed_roles = $settings['roles'] ?? [];
161-
if ( $role_based_access_enabled ) {
162-
// For super admins that don't have a specific role on a site, treat them as admins.
163-
if ( is_multisite() && is_super_admin( $user_id ) && empty( $user_roles ) ) {
164-
$user_roles = [ 'administrator' ];
165-
}
166-
167-
if (
168-
! empty( $allowed_roles ) &&
169-
! empty( array_intersect( $user_roles, $allowed_roles ) )
170-
) {
171-
$allowed_features[ $feature_class::ID ] = $feature_class->get_label();
172-
continue;
173-
}
159+
// For super admins that don't have a specific role on a site, treat them as admins.
160+
if ( is_multisite() && is_super_admin( $user_id ) && empty( $user_roles ) ) {
161+
$user_roles = [ 'administrator' ];
162+
}
163+
164+
if (
165+
! empty( $allowed_roles ) &&
166+
! empty( array_intersect( $user_roles, $allowed_roles ) )
167+
) {
168+
$allowed_features[ $feature_class::ID ] = $feature_class->get_label();
169+
continue;
174170
}
175171

176172
// Check if user has access to the feature.
177173
$allowed_users = $settings['users'] ?? [];
178174
if (
179-
$user_based_access_enabled &&
180175
! empty( $allowed_users ) &&
181176
in_array( $user_id, $allowed_users, true )
182177
) {

‎includes/Classifai/Features/Feature.php

+10-72
Original file line numberDiff line numberDiff line change
@@ -178,9 +178,7 @@ public function add_custom_settings_fields() {
178178
protected function get_default_settings(): array {
179179
$shared_defaults = [
180180
'status' => '0',
181-
'role_based_access' => '1',
182181
'roles' => array_combine( array_keys( $this->roles ), array_keys( $this->roles ) ),
183-
'user_based_access' => 'no',
184182
'users' => [],
185183
'user_based_opt_out' => 'no',
186184
];
@@ -222,25 +220,13 @@ public function sanitize_settings( array $settings ): array {
222220
$new_settings['status'] = $settings['status'] ?? $current_settings['status'];
223221
$new_settings['provider'] = isset( $settings['provider'] ) ? sanitize_text_field( $settings['provider'] ) : $current_settings['provider'];
224222

225-
if ( empty( $settings['role_based_access'] ) || 1 !== (int) $settings['role_based_access'] ) {
226-
$new_settings['role_based_access'] = 'no';
227-
} else {
228-
$new_settings['role_based_access'] = '1';
229-
}
230-
231223
// Allowed roles.
232224
if ( isset( $settings['roles'] ) && is_array( $settings['roles'] ) ) {
233225
$new_settings['roles'] = array_map( 'sanitize_text_field', $settings['roles'] );
234226
} else {
235227
$new_settings['roles'] = $current_settings['roles'];
236228
}
237229

238-
if ( empty( $settings['user_based_access'] ) || 1 !== (int) $settings['user_based_access'] ) {
239-
$new_settings['user_based_access'] = 'no';
240-
} else {
241-
$new_settings['user_based_access'] = '1';
242-
}
243-
244230
// Allowed users.
245231
if ( isset( $settings['users'] ) && ! empty( $settings['users'] ) ) {
246232
if ( is_array( $settings['users'] ) ) {
@@ -448,27 +434,6 @@ public function reset_settings() {
448434
protected function add_access_control_fields() {
449435
$settings = $this->get_settings();
450436

451-
add_settings_field(
452-
'role_based_access',
453-
esc_html__( 'Enable role-based access', 'classifai' ),
454-
[ $this, 'render_input' ],
455-
$this->get_option_name(),
456-
$this->get_option_name() . '_section',
457-
[
458-
'label_for' => 'role_based_access',
459-
'input_type' => 'checkbox',
460-
'default_value' => $settings['role_based_access'],
461-
'description' => __( 'Enables ability to select which roles can access this feature.', 'classifai' ),
462-
'class' => 'classifai-role-based-access',
463-
]
464-
);
465-
466-
// Add hidden class if role-based access is disabled.
467-
$class = 'allowed_roles_row';
468-
if ( ! isset( $settings['role_based_access'] ) || '1' !== $settings['role_based_access'] ) {
469-
$class .= ' hidden';
470-
}
471-
472437
add_settings_field(
473438
'roles',
474439
esc_html__( 'Allowed roles', 'classifai' ),
@@ -480,31 +445,10 @@ protected function add_access_control_fields() {
480445
'options' => $this->roles,
481446
'default_values' => $settings['roles'],
482447
'description' => __( 'Choose which roles are allowed to access this feature.', 'classifai' ),
483-
'class' => $class,
448+
'class' => 'allowed_roles_row',
484449
]
485450
);
486451

487-
add_settings_field(
488-
'user_based_access',
489-
esc_html__( 'Enable user-based access', 'classifai' ),
490-
[ $this, 'render_input' ],
491-
$this->get_option_name(),
492-
$this->get_option_name() . '_section',
493-
[
494-
'label_for' => 'user_based_access',
495-
'input_type' => 'checkbox',
496-
'default_value' => $settings['user_based_access'],
497-
'description' => __( 'Enables ability to select which users can access this feature.', 'classifai' ),
498-
'class' => 'classifai-user-based-access',
499-
]
500-
);
501-
502-
// Add hidden class if user-based access is disabled.
503-
$users_class = 'allowed_users_row';
504-
if ( ! isset( $settings['user_based_access'] ) || '1' !== $settings['user_based_access'] ) {
505-
$users_class .= ' hidden';
506-
}
507-
508452
add_settings_field(
509453
'users',
510454
esc_html__( 'Allowed users', 'classifai' ),
@@ -515,7 +459,7 @@ protected function add_access_control_fields() {
515459
'label_for' => 'users',
516460
'default_value' => $settings['users'],
517461
'description' => __( 'Users who have access to this feature.', 'classifai' ),
518-
'class' => $users_class,
462+
'class' => 'allowed_users_row',
519463
]
520464
);
521465

@@ -930,26 +874,22 @@ public function has_access(): bool {
930874
$feature_roles = $settings['roles'] ?? [];
931875
$feature_users = array_map( 'absint', $settings['users'] ?? [] );
932876

933-
$role_based_access_enabled = isset( $settings['role_based_access'] ) && 1 === (int) $settings['role_based_access'];
934-
$user_based_access_enabled = isset( $settings['user_based_access'] ) && 1 === (int) $settings['user_based_access'];
935877
$user_based_opt_out_enabled = isset( $settings['user_based_opt_out'] ) && 1 === (int) $settings['user_based_opt_out'];
936878

937879
/*
938-
* Checks if Role-based access is enabled and user role has access to the feature.
880+
* Checks if the user role has access to the feature.
939881
*/
940-
if ( $role_based_access_enabled ) {
941-
// For super admins that don't have a specific role on a site, treat them as admins.
942-
if ( is_multisite() && is_super_admin( $user_id ) && empty( $user_roles ) ) {
943-
$user_roles = [ 'administrator' ];
944-
}
945-
946-
$access = ( ! empty( $feature_roles ) && ! empty( array_intersect( $user_roles, $feature_roles ) ) );
882+
// For super admins that don't have a specific role on a site, treat them as admins.
883+
if ( is_multisite() && is_super_admin( $user_id ) && empty( $user_roles ) ) {
884+
$user_roles = [ 'administrator' ];
947885
}
948886

887+
$access = ( ! empty( $feature_roles ) && ! empty( array_intersect( $user_roles, $feature_roles ) ) );
888+
949889
/*
950-
* Checks if User-based access is enabled and user has access to the feature.
890+
* Checks if has access to the feature.
951891
*/
952-
if ( ! $access && $user_based_access_enabled ) {
892+
if ( ! $access ) {
953893
$access = ( ! empty( $feature_users ) && ! empty( in_array( $user_id, $feature_users, true ) ) );
954894
}
955895

@@ -1160,9 +1100,7 @@ function ( $role ) {
11601100
$common_debug_info = [
11611101
__( 'Authenticated', 'classifai' ) => self::get_debug_value_text( $this->is_configured() ),
11621102
__( 'Status', 'classifai' ) => self::get_debug_value_text( $feature_settings['status'], 1 ),
1163-
__( 'Role-based access', 'classifai' ) => self::get_debug_value_text( $feature_settings['role_based_access'], 1 ),
11641103
__( 'Allowed roles (titles)', 'classifai' ) => implode( ', ', $roles ?? [] ),
1165-
__( 'User-based access', 'classifai' ) => self::get_debug_value_text( $feature_settings['user_based_access'], 1 ),
11661104
__( 'Allowed users (titles)', 'classifai' ) => implode( ', ', $feature_settings['users'] ?? [] ),
11671105
__( 'User based opt-out', 'classifai' ) => self::get_debug_value_text( $feature_settings['user_based_opt_out'], 1 ),
11681106
__( 'Provider', 'classifai' ) => $feature_settings['provider'],

‎src/js/admin.js

-54
Original file line numberDiff line numberDiff line change
@@ -78,60 +78,6 @@ document.addEventListener( 'DOMContentLoaded', function () {
7878
} );
7979
} )();
8080

81-
// Role and user based access.
82-
document.addEventListener( 'DOMContentLoaded', function () {
83-
function toogleAllowedRolesRow( e ) {
84-
const checkbox = e.target;
85-
const parentTr = checkbox.closest( 'tr.classifai-role-based-access' );
86-
const allowedRoles = parentTr.nextElementSibling.classList.contains(
87-
'allowed_roles_row'
88-
)
89-
? parentTr.nextElementSibling
90-
: null;
91-
if ( checkbox.checked ) {
92-
allowedRoles.classList.remove( 'hidden' );
93-
} else {
94-
allowedRoles.classList.add( 'hidden' );
95-
}
96-
}
97-
98-
function toogleAllowedUsersRow( e ) {
99-
const checkbox = e.target;
100-
const parentTr = checkbox.closest( 'tr.classifai-user-based-access' );
101-
const allowedUsers = parentTr.nextElementSibling.classList.contains(
102-
'allowed_users_row'
103-
)
104-
? parentTr.nextElementSibling
105-
: null;
106-
if ( checkbox.checked ) {
107-
allowedUsers.classList.remove( 'hidden' );
108-
} else {
109-
allowedUsers.classList.add( 'hidden' );
110-
}
111-
}
112-
113-
const roleBasedAccessCheckBoxes = document.querySelectorAll(
114-
'tr.classifai-role-based-access input[type="checkbox"]'
115-
);
116-
const userBasedAccessCheckBoxes = document.querySelectorAll(
117-
'tr.classifai-user-based-access input[type="checkbox"]'
118-
);
119-
120-
if ( roleBasedAccessCheckBoxes ) {
121-
roleBasedAccessCheckBoxes.forEach( function ( e ) {
122-
e.addEventListener( 'change', toogleAllowedRolesRow );
123-
e.dispatchEvent( new Event( 'change' ) );
124-
} );
125-
}
126-
127-
if ( userBasedAccessCheckBoxes ) {
128-
userBasedAccessCheckBoxes.forEach( function ( e ) {
129-
e.addEventListener( 'change', toogleAllowedUsersRow );
130-
e.dispatchEvent( new Event( 'change' ) );
131-
} );
132-
}
133-
} );
134-
13581
// User Selector.
13682
( () => {
13783
const userSearches = document.querySelectorAll(

‎tests/Classifai/Providers/Azure/ComputerVisionTest.php

-2
Original file line numberDiff line numberDiff line change
@@ -72,9 +72,7 @@ public function test_no_computer_vision_option_set() {
7272
$defaults,
7373
[
7474
'status' => '0',
75-
'role_based_access' => '1',
7675
'roles' => [],
77-
'user_based_access' => 'no',
7876
'users' => [],
7977
'user_based_opt_out' => 'no',
8078
'descriptive_text_fields' => [

‎tests/cypress/integration/admin/common-feature-fields.test.js

-17
Original file line numberDiff line numberDiff line change
@@ -37,16 +37,6 @@ describe('Common Feature Fields', () => {
3737
'name',
3838
`classifai_${ feature }[status]`
3939
);
40-
cy.get( '#role_based_access' ).should(
41-
'have.attr',
42-
'name',
43-
`classifai_${ feature }[role_based_access]`
44-
);
45-
cy.get( '#user_based_access' ).should(
46-
'have.attr',
47-
'name',
48-
`classifai_${ feature }[user_based_access]`
49-
);
5040
cy.get( '#user_based_opt_out' ).should(
5141
'have.attr',
5242
'name',
@@ -57,7 +47,6 @@ describe('Common Feature Fields', () => {
5747
'name',
5848
`classifai_${ feature }[provider]`
5949
);
60-
cy.get( '#role_based_access' ).check();
6150

6251
for ( const role of allowedRoles ) {
6352
if (
@@ -79,14 +68,8 @@ describe('Common Feature Fields', () => {
7968
);
8069
}
8170

82-
cy.get( '#role_based_access' ).uncheck();
83-
cy.get( '.allowed_roles_row' ).should( 'not.be.visible' );
84-
85-
cy.get( '#user_based_access' ).check();
8671
cy.get( '.allowed_users_row' ).should( 'be.visible' );
8772

88-
cy.get( '#user_based_access' ).uncheck();
89-
cy.get( '.allowed_users_row' ).should( 'not.be.visible' );
9073
} );
9174
} );
9275
} );

‎tests/cypress/integration/language-processing/classify-content-ibm-watson.test.js

+30-10
Original file line numberDiff line numberDiff line change
@@ -467,7 +467,6 @@ describe( '[Language processing] Classify content (IBM Watson - NLU) Tests', ()
467467
cy.visit(
468468
'/wp-admin/tools.php?page=classifai&tab=language_processing&feature=feature_classification'
469469
);
470-
cy.get( '#role_based_access' ).check();
471470
cy.get(
472471
'#classifai_feature_classification_roles_administrator'
473472
).uncheck();
@@ -482,7 +481,6 @@ describe( '[Language processing] Classify content (IBM Watson - NLU) Tests', ()
482481
cy.visit(
483482
'/wp-admin/tools.php?page=classifai&tab=language_processing&provider=watson_nlu'
484483
);
485-
cy.get( '#role_based_access' ).check();
486484
cy.get(
487485
'#classifai_feature_classification_roles_administrator'
488486
).check();
@@ -499,8 +497,15 @@ describe( '[Language processing] Classify content (IBM Watson - NLU) Tests', ()
499497
cy.visit(
500498
'/wp-admin/tools.php?page=classifai&tab=language_processing&provider=watson_nlu'
501499
);
502-
cy.get( '#role_based_access' ).uncheck();
503-
cy.get( '#user_based_access' ).uncheck();
500+
501+
// Disable access for all roles.
502+
cy.get( '.allowed_roles_row input[type="checkbox"]' ).uncheck( {
503+
multiple: true,
504+
} );
505+
506+
// Disable access for all users.
507+
cy.disableFeatureForUsers();
508+
504509
cy.get( '#submit' ).click();
505510
cy.get( '.notice' ).contains( 'Settings saved.' );
506511

@@ -511,8 +516,12 @@ describe( '[Language processing] Classify content (IBM Watson - NLU) Tests', ()
511516
cy.visit(
512517
'/wp-admin/tools.php?page=classifai&tab=language_processing&provider=watson_nlu'
513518
);
514-
cy.get( '#role_based_access' ).uncheck();
515-
cy.get( '#user_based_access' ).check();
519+
520+
// Disable access for all roles.
521+
cy.get( '.allowed_roles_row input[type="checkbox"]' ).uncheck( {
522+
multiple: true,
523+
} );
524+
516525
cy.get( 'body' ).then( ( $body ) => {
517526
if (
518527
$body.find(
@@ -543,8 +552,14 @@ describe( '[Language processing] Classify content (IBM Watson - NLU) Tests', ()
543552
cy.visit(
544553
'/wp-admin/tools.php?page=classifai&tab=language_processing&provider=watson_nlu'
545554
);
546-
cy.get( '#role_based_access' ).check();
547-
cy.get( '#user_based_access' ).uncheck();
555+
556+
// Enable access for all roles.
557+
cy.get( '.allowed_roles_row input[type="checkbox"]' ).check( {
558+
multiple: true,
559+
} );
560+
561+
// Disable access for all users.
562+
cy.disableFeatureForUsers();
548563

549564
cy.get( '#submit' ).click();
550565
cy.get( '.notice' ).contains( 'Settings saved.' );
@@ -555,8 +570,13 @@ describe( '[Language processing] Classify content (IBM Watson - NLU) Tests', ()
555570
cy.visit(
556571
'/wp-admin/tools.php?page=classifai&tab=language_processing&provider=watson_nlu'
557572
);
558-
cy.get( '#role_based_access' ).check();
559-
cy.get( '#user_based_access' ).check();
573+
// Enable access for all roles.
574+
cy.get( '.allowed_roles_row input[type="checkbox"]' ).check( {
575+
multiple: true,
576+
} );
577+
578+
// Disable access for all users.
579+
cy.disableFeatureForUsers();
560580
cy.get( '#user_based_opt_out' ).check();
561581

562582
cy.get( '#submit' ).click();

0 commit comments

Comments
 (0)