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

Removed "Enable role-based access" and "Enable user-based access" settings options from the feature settings and set to be enabled by default. #692

Merged
merged 3 commits into from
Feb 5, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
27 changes: 11 additions & 16 deletions includes/Classifai/Admin/UserProfile.php
Original file line number Diff line number Diff line change
Expand Up @@ -147,8 +147,6 @@ public function get_allowed_features( int $user_id ): array {
continue;
}

$role_based_access_enabled = isset( $settings['role_based_access'] ) && 1 === (int) $settings['role_based_access'];
$user_based_access_enabled = isset( $settings['user_based_access'] ) && 1 === (int) $settings['user_based_access'];
$user_based_opt_out_enabled = isset( $settings['user_based_opt_out'] ) && 1 === (int) $settings['user_based_opt_out'];

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

// Check if user has access to the feature by role.
$allowed_roles = $settings['roles'] ?? [];
if ( $role_based_access_enabled ) {
// For super admins that don't have a specific role on a site, treat them as admins.
if ( is_multisite() && is_super_admin( $user_id ) && empty( $user_roles ) ) {
$user_roles = [ 'administrator' ];
}

if (
! empty( $allowed_roles ) &&
! empty( array_intersect( $user_roles, $allowed_roles ) )
) {
$allowed_features[ $feature_class::ID ] = $feature_class->get_label();
continue;
}
// For super admins that don't have a specific role on a site, treat them as admins.
if ( is_multisite() && is_super_admin( $user_id ) && empty( $user_roles ) ) {
$user_roles = [ 'administrator' ];
}

if (
! empty( $allowed_roles ) &&
! empty( array_intersect( $user_roles, $allowed_roles ) )
) {
$allowed_features[ $feature_class::ID ] = $feature_class->get_label();
continue;
}

// Check if user has access to the feature.
$allowed_users = $settings['users'] ?? [];
if (
$user_based_access_enabled &&
! empty( $allowed_users ) &&
in_array( $user_id, $allowed_users, true )
) {
Expand Down
82 changes: 10 additions & 72 deletions includes/Classifai/Features/Feature.php
Original file line number Diff line number Diff line change
Expand Up @@ -178,9 +178,7 @@ public function add_custom_settings_fields() {
protected function get_default_settings(): array {
$shared_defaults = [
'status' => '0',
'role_based_access' => '1',
'roles' => array_combine( array_keys( $this->roles ), array_keys( $this->roles ) ),
'user_based_access' => 'no',
'users' => [],
'user_based_opt_out' => 'no',
];
Expand Down Expand Up @@ -222,25 +220,13 @@ public function sanitize_settings( array $settings ): array {
$new_settings['status'] = $settings['status'] ?? $current_settings['status'];
$new_settings['provider'] = isset( $settings['provider'] ) ? sanitize_text_field( $settings['provider'] ) : $current_settings['provider'];

if ( empty( $settings['role_based_access'] ) || 1 !== (int) $settings['role_based_access'] ) {
$new_settings['role_based_access'] = 'no';
} else {
$new_settings['role_based_access'] = '1';
}

// Allowed roles.
if ( isset( $settings['roles'] ) && is_array( $settings['roles'] ) ) {
$new_settings['roles'] = array_map( 'sanitize_text_field', $settings['roles'] );
} else {
$new_settings['roles'] = $current_settings['roles'];
}

if ( empty( $settings['user_based_access'] ) || 1 !== (int) $settings['user_based_access'] ) {
$new_settings['user_based_access'] = 'no';
} else {
$new_settings['user_based_access'] = '1';
}

// Allowed users.
if ( isset( $settings['users'] ) && ! empty( $settings['users'] ) ) {
if ( is_array( $settings['users'] ) ) {
Expand Down Expand Up @@ -448,27 +434,6 @@ public function reset_settings() {
protected function add_access_control_fields() {
$settings = $this->get_settings();

add_settings_field(
'role_based_access',
esc_html__( 'Enable role-based access', 'classifai' ),
[ $this, 'render_input' ],
$this->get_option_name(),
$this->get_option_name() . '_section',
[
'label_for' => 'role_based_access',
'input_type' => 'checkbox',
'default_value' => $settings['role_based_access'],
'description' => __( 'Enables ability to select which roles can access this feature.', 'classifai' ),
'class' => 'classifai-role-based-access',
]
);

// Add hidden class if role-based access is disabled.
$class = 'allowed_roles_row';
if ( ! isset( $settings['role_based_access'] ) || '1' !== $settings['role_based_access'] ) {
$class .= ' hidden';
}

add_settings_field(
'roles',
esc_html__( 'Allowed roles', 'classifai' ),
Expand All @@ -480,31 +445,10 @@ protected function add_access_control_fields() {
'options' => $this->roles,
'default_values' => $settings['roles'],
'description' => __( 'Choose which roles are allowed to access this feature.', 'classifai' ),
'class' => $class,
'class' => 'allowed_roles_row',
]
);

add_settings_field(
'user_based_access',
esc_html__( 'Enable user-based access', 'classifai' ),
[ $this, 'render_input' ],
$this->get_option_name(),
$this->get_option_name() . '_section',
[
'label_for' => 'user_based_access',
'input_type' => 'checkbox',
'default_value' => $settings['user_based_access'],
'description' => __( 'Enables ability to select which users can access this feature.', 'classifai' ),
'class' => 'classifai-user-based-access',
]
);

// Add hidden class if user-based access is disabled.
$users_class = 'allowed_users_row';
if ( ! isset( $settings['user_based_access'] ) || '1' !== $settings['user_based_access'] ) {
$users_class .= ' hidden';
}

add_settings_field(
'users',
esc_html__( 'Allowed users', 'classifai' ),
Expand All @@ -515,7 +459,7 @@ protected function add_access_control_fields() {
'label_for' => 'users',
'default_value' => $settings['users'],
'description' => __( 'Users who have access to this feature.', 'classifai' ),
'class' => $users_class,
'class' => 'allowed_users_row',
]
);

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

$role_based_access_enabled = isset( $settings['role_based_access'] ) && 1 === (int) $settings['role_based_access'];
$user_based_access_enabled = isset( $settings['user_based_access'] ) && 1 === (int) $settings['user_based_access'];
$user_based_opt_out_enabled = isset( $settings['user_based_opt_out'] ) && 1 === (int) $settings['user_based_opt_out'];

/*
* Checks if Role-based access is enabled and user role has access to the feature.
* Checks if the user role has access to the feature.
*/
if ( $role_based_access_enabled ) {
// For super admins that don't have a specific role on a site, treat them as admins.
if ( is_multisite() && is_super_admin( $user_id ) && empty( $user_roles ) ) {
$user_roles = [ 'administrator' ];
}

$access = ( ! empty( $feature_roles ) && ! empty( array_intersect( $user_roles, $feature_roles ) ) );
// For super admins that don't have a specific role on a site, treat them as admins.
if ( is_multisite() && is_super_admin( $user_id ) && empty( $user_roles ) ) {
$user_roles = [ 'administrator' ];
}

$access = ( ! empty( $feature_roles ) && ! empty( array_intersect( $user_roles, $feature_roles ) ) );

/*
* Checks if User-based access is enabled and user has access to the feature.
* Checks if has access to the feature.
*/
if ( ! $access && $user_based_access_enabled ) {
if ( ! $access ) {
$access = ( ! empty( $feature_users ) && ! empty( in_array( $user_id, $feature_users, true ) ) );
}

Expand Down Expand Up @@ -1160,9 +1100,7 @@ function ( $role ) {
$common_debug_info = [
__( 'Authenticated', 'classifai' ) => self::get_debug_value_text( $this->is_configured() ),
__( 'Status', 'classifai' ) => self::get_debug_value_text( $feature_settings['status'], 1 ),
__( 'Role-based access', 'classifai' ) => self::get_debug_value_text( $feature_settings['role_based_access'], 1 ),
__( 'Allowed roles (titles)', 'classifai' ) => implode( ', ', $roles ?? [] ),
__( 'User-based access', 'classifai' ) => self::get_debug_value_text( $feature_settings['user_based_access'], 1 ),
__( 'Allowed users (titles)', 'classifai' ) => implode( ', ', $feature_settings['users'] ?? [] ),
__( 'User based opt-out', 'classifai' ) => self::get_debug_value_text( $feature_settings['user_based_opt_out'], 1 ),
__( 'Provider', 'classifai' ) => $feature_settings['provider'],
Expand Down
54 changes: 0 additions & 54 deletions src/js/admin.js
Original file line number Diff line number Diff line change
Expand Up @@ -78,60 +78,6 @@ document.addEventListener( 'DOMContentLoaded', function () {
} );
} )();

// Role and user based access.
document.addEventListener( 'DOMContentLoaded', function () {
function toogleAllowedRolesRow( e ) {
const checkbox = e.target;
const parentTr = checkbox.closest( 'tr.classifai-role-based-access' );
const allowedRoles = parentTr.nextElementSibling.classList.contains(
'allowed_roles_row'
)
? parentTr.nextElementSibling
: null;
if ( checkbox.checked ) {
allowedRoles.classList.remove( 'hidden' );
} else {
allowedRoles.classList.add( 'hidden' );
}
}

function toogleAllowedUsersRow( e ) {
const checkbox = e.target;
const parentTr = checkbox.closest( 'tr.classifai-user-based-access' );
const allowedUsers = parentTr.nextElementSibling.classList.contains(
'allowed_users_row'
)
? parentTr.nextElementSibling
: null;
if ( checkbox.checked ) {
allowedUsers.classList.remove( 'hidden' );
} else {
allowedUsers.classList.add( 'hidden' );
}
}

const roleBasedAccessCheckBoxes = document.querySelectorAll(
'tr.classifai-role-based-access input[type="checkbox"]'
);
const userBasedAccessCheckBoxes = document.querySelectorAll(
'tr.classifai-user-based-access input[type="checkbox"]'
);

if ( roleBasedAccessCheckBoxes ) {
roleBasedAccessCheckBoxes.forEach( function ( e ) {
e.addEventListener( 'change', toogleAllowedRolesRow );
e.dispatchEvent( new Event( 'change' ) );
} );
}

if ( userBasedAccessCheckBoxes ) {
userBasedAccessCheckBoxes.forEach( function ( e ) {
e.addEventListener( 'change', toogleAllowedUsersRow );
e.dispatchEvent( new Event( 'change' ) );
} );
}
} );

// User Selector.
( () => {
const userSearches = document.querySelectorAll(
Expand Down
2 changes: 0 additions & 2 deletions tests/Classifai/Providers/Azure/ComputerVisionTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -72,9 +72,7 @@ public function test_no_computer_vision_option_set() {
$defaults,
[
'status' => '0',
'role_based_access' => '1',
'roles' => [],
'user_based_access' => 'no',
'users' => [],
'user_based_opt_out' => 'no',
'descriptive_text_fields' => [
Expand Down
17 changes: 0 additions & 17 deletions tests/cypress/integration/admin/common-feature-fields.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -37,16 +37,6 @@ describe('Common Feature Fields', () => {
'name',
`classifai_${ feature }[status]`
);
cy.get( '#role_based_access' ).should(
'have.attr',
'name',
`classifai_${ feature }[role_based_access]`
);
cy.get( '#user_based_access' ).should(
'have.attr',
'name',
`classifai_${ feature }[user_based_access]`
);
cy.get( '#user_based_opt_out' ).should(
'have.attr',
'name',
Expand All @@ -57,7 +47,6 @@ describe('Common Feature Fields', () => {
'name',
`classifai_${ feature }[provider]`
);
cy.get( '#role_based_access' ).check();

for ( const role of allowedRoles ) {
if (
Expand All @@ -79,14 +68,8 @@ describe('Common Feature Fields', () => {
);
}

cy.get( '#role_based_access' ).uncheck();
cy.get( '.allowed_roles_row' ).should( 'not.be.visible' );

cy.get( '#user_based_access' ).check();
cy.get( '.allowed_users_row' ).should( 'be.visible' );

cy.get( '#user_based_access' ).uncheck();
cy.get( '.allowed_users_row' ).should( 'not.be.visible' );
} );
} );
} );
Original file line number Diff line number Diff line change
Expand Up @@ -467,7 +467,6 @@ describe( '[Language processing] Classify content (IBM Watson - NLU) Tests', ()
cy.visit(
'/wp-admin/tools.php?page=classifai&tab=language_processing&feature=feature_classification'
);
cy.get( '#role_based_access' ).check();
cy.get(
'#classifai_feature_classification_roles_administrator'
).uncheck();
Expand All @@ -482,7 +481,6 @@ describe( '[Language processing] Classify content (IBM Watson - NLU) Tests', ()
cy.visit(
'/wp-admin/tools.php?page=classifai&tab=language_processing&provider=watson_nlu'
);
cy.get( '#role_based_access' ).check();
cy.get(
'#classifai_feature_classification_roles_administrator'
).check();
Expand All @@ -499,8 +497,15 @@ describe( '[Language processing] Classify content (IBM Watson - NLU) Tests', ()
cy.visit(
'/wp-admin/tools.php?page=classifai&tab=language_processing&provider=watson_nlu'
);
cy.get( '#role_based_access' ).uncheck();
cy.get( '#user_based_access' ).uncheck();

// Disable access for all roles.
cy.get( '.allowed_roles_row input[type="checkbox"]' ).uncheck( {
multiple: true,
} );

// Disable access for all users.
cy.disableFeatureForUsers();

cy.get( '#submit' ).click();
cy.get( '.notice' ).contains( 'Settings saved.' );

Expand All @@ -511,8 +516,12 @@ describe( '[Language processing] Classify content (IBM Watson - NLU) Tests', ()
cy.visit(
'/wp-admin/tools.php?page=classifai&tab=language_processing&provider=watson_nlu'
);
cy.get( '#role_based_access' ).uncheck();
cy.get( '#user_based_access' ).check();

// Disable access for all roles.
cy.get( '.allowed_roles_row input[type="checkbox"]' ).uncheck( {
multiple: true,
} );

cy.get( 'body' ).then( ( $body ) => {
if (
$body.find(
Expand Down Expand Up @@ -543,8 +552,14 @@ describe( '[Language processing] Classify content (IBM Watson - NLU) Tests', ()
cy.visit(
'/wp-admin/tools.php?page=classifai&tab=language_processing&provider=watson_nlu'
);
cy.get( '#role_based_access' ).check();
cy.get( '#user_based_access' ).uncheck();

// Enable access for all roles.
cy.get( '.allowed_roles_row input[type="checkbox"]' ).check( {
multiple: true,
} );

// Disable access for all users.
cy.disableFeatureForUsers();

cy.get( '#submit' ).click();
cy.get( '.notice' ).contains( 'Settings saved.' );
Expand All @@ -555,8 +570,13 @@ describe( '[Language processing] Classify content (IBM Watson - NLU) Tests', ()
cy.visit(
'/wp-admin/tools.php?page=classifai&tab=language_processing&provider=watson_nlu'
);
cy.get( '#role_based_access' ).check();
cy.get( '#user_based_access' ).check();
// Enable access for all roles.
cy.get( '.allowed_roles_row input[type="checkbox"]' ).check( {
multiple: true,
} );

// Disable access for all users.
cy.disableFeatureForUsers();
cy.get( '#user_based_opt_out' ).check();

cy.get( '#submit' ).click();
Expand Down
Loading
Loading