Please consider replacing ‘whitelist’ with ‘allow list.’ Thank you.

Tasks

  • Backport to 7.x

Comments

ianchan created an issue. See original summary.

rocketeerbkw’s picture

Title: Terminology updates » Avoid exclusive language
Priority: Major » Normal
Issue tags: +Novice

Thanks, I'll accept patches with this change. I think it only exists in the code comments and on the project page.

anjali rathod’s picture

Assigned: Unassigned » anjali rathod
anjali rathod’s picture

Assigned: anjali rathod » Unassigned
Status: Active » Needs review
StatusFileSize
new3.61 KB

Couldn't get the words with blacklist, made the changes for whitelist. Please review and add any needed comments

guilhermevp’s picture

Status: Needs review » Reviewed & tested by the community
StatusFileSize
new58.32 KB
new16.33 KB

Patch applies cleanly and remove all whitelist related mentions.

nicrodgers’s picture

Status: Reviewed & tested by the community » Needs work

Thanks for your contribution!
I've reviewed the changes, and highlight below some further changes to improve readability. Currently some of the changes don't read correctly, nor are they grammatically correct.

  1. +++ b/README.txt
    @@ -2,7 +2,7 @@ Description
    +IP addresses) to be allowed list which restricts the functionality of users.
    

    this doesn't read properly.

    How about:

    In general, this module can be used to prevent logging in by users unless their IP address is on the allowed list.

  2. +++ b/restrict_by_ip.module
    @@ -2,7 +2,7 @@
    + * Restrict logins or roles to allow list IP addresses.
    

    to IP addresses on the allowed list

  3. +++ b/src/LoginFirewall.php
    @@ -91,7 +91,7 @@ class LoginFirewall implements LoginFirewallInterface {
    +   *   True/False whether the current users IP is in global IP allow listlist.
    

    listlist -> list

  4. +++ b/src/LoginFirewall.php
    @@ -112,7 +112,7 @@ class LoginFirewall implements LoginFirewallInterface {
    +   *   True/False whether the current users IP is in user IP allow listlist.
    

    listlist -> list

  5. +++ b/src/LoginFirewallInterface.php
    @@ -12,7 +12,7 @@ use Drupal\Core\Session\AccountInterface;
    +   * Checks that a user is allowed to login based on IP allow lists.
    

    based on the IP allow list

  6. +++ b/src/RoleFirewallInterface.php
    @@ -15,7 +15,7 @@ namespace Drupal\restrict_by_ip;
    +   * Checks which roles should be removed based on IP allow lists.
    

    based on the IP allow list

  7. +++ b/src/Session/AccountProxy.php
    @@ -13,7 +13,7 @@ use Drupal\restrict_by_ip\RoleFirewallInterface;
    + * on IP allow list. Proxy all other method calls to the original current_user
    

    on the IP allow list.

nicrodgers’s picture

Issue summary: View changes

Updated the issue summary to only reference the removal of 'whitelist' as there is currently no mention of 'blacklist' in the codebase, so nothing needs removing there.

anjali rathod’s picture

Assigned: Unassigned » anjali rathod
anjali rathod’s picture

Assigned: anjali rathod » Unassigned
Status: Needs work » Needs review
StatusFileSize
new6.47 KB

Thanks for the update!
I have made the changes. Please review ! :)

nicrodgers’s picture

Status: Needs review » Needs work

Hi Anjali,

Nearly there I think. Just a few things left that I've spotted:

  1. +++ b/restrict_by_ip.module
    @@ -2,7 +2,7 @@
    + * Restrict logins or roles to allow list IP addresses on the allowed list.
    

    Restrict logins or roles to IP addresses on the allowed list

  2. +++ b/src/Session/AccountProxy.php
    @@ -13,7 +13,7 @@ use Drupal\restrict_by_ip\RoleFirewallInterface;
    + * on the IP allow list. Proxy all other method calls to the original current_user
    

    these lines need reformatting so they are under 80 characters (to comply with drupal coding standards)

  3. +++ b/src/Session/AccountProxy.php
    diff --git a/src/Tests/UiTest.php b/src/Tests/UiTest.php
    diff --git a/tests/src/Unit/UnitTest.php b/tests/src/Unit/UnitTest.php
    

    there seem to be a lot of chmod/mode changes in this latest diff (like this one) that are unrelated and need removing

neslee canil pinto’s picture

Status: Needs work » Needs review
StatusFileSize
new3.9 KB
nicrodgers’s picture

Status: Needs review » Reviewed & tested by the community

Looks good, nice work everyone.

  • rocketeerbkw committed 0092bb67 on 8.x-4.x
    Issue #3211223 by Anjali Rathod, Neslee Canil Pinto, guilhermevp,...

rocketeerbkw’s picture

Version: 8.x-4.x-dev » 7.x-3.x-dev
Issue summary: View changes
Status: Reviewed & tested by the community » Active

Thanks all, I committed the patch and reworked the README. Moving issue to 7.x for backporting.

rocketeerbkw’s picture

darvanen’s picture

Status: Active » Fixed

As Drupal 7 is no longer supported I'm moving this to fixed.

Status: Fixed » Closed (fixed)

Automatically closed - issue fixed for 2 weeks with no activity.