From 16c3aa9f9f99460bd30f1de70593e91a9121725f Mon Sep 17 00:00:00 2001
From: AddisonDunn <dunnaddisonw@gmail.com>
Date: Wed, 24 Apr 2024 17:10:51 -0400
Subject: [PATCH 1/3] warn users about unrestricted roles being assinable by a
 restricted role

---
 .../hqwebapp/static/users/scss/roles.scss     |  3 +++
 corehq/apps/users/static/users/js/roles.js    | 13 ++++++++---
 .../users/partials/edit_role_modal.html       | 23 +++++++++++++++++--
 3 files changed, 34 insertions(+), 5 deletions(-)
 create mode 100644 corehq/apps/hqwebapp/static/users/scss/roles.scss

diff --git a/corehq/apps/hqwebapp/static/users/scss/roles.scss b/corehq/apps/hqwebapp/static/users/scss/roles.scss
new file mode 100644
index 000000000000..32dbfa1a8806
--- /dev/null
+++ b/corehq/apps/hqwebapp/static/users/scss/roles.scss
@@ -0,0 +1,3 @@
+.role-modal-warning { // Used in combination with Bootstrap's 'help-block'/'alert' classes
+    margin-bottom: 0px;
+}
diff --git a/corehq/apps/users/static/users/js/roles.js b/corehq/apps/users/static/users/js/roles.js
index 0799446bfedc..a951e266b18d 100644
--- a/corehq/apps/users/static/users/js/roles.js
+++ b/corehq/apps/users/static/users/js/roles.js
@@ -1,3 +1,5 @@
+'use strict';
+
 hqDefine('users/js/roles',[
     'jquery',
     'underscore',
@@ -77,7 +79,6 @@ hqDefine('users/js/roles',[
     };
 
     var RolesViewModel = function (o) {
-        'use strict';
         var self, root;
         self = root = {};
 
@@ -131,14 +132,14 @@ hqDefine('users/js/roles',[
                         };
                     }),
                 };
-
                 data.manageRoleAssignments = {
                     all: data.is_non_admin_editable,
                     specific: ko.utils.arrayMap(o.nonAdminRoles, function (role) {
                         return {
                             path: role._id,
                             name: role.name,
-                            value: data.assignable_by.indexOf(role._id) !== -1,
+                            value: ko.observable(data.assignable_by.indexOf(role._id) !== -1),
+                            access_all_locations: role.permissions.access_all_locations,
                         };
                     }),
                 };
@@ -167,6 +168,12 @@ hqDefine('users/js/roles',[
                 };
                 self.preventRoleDelete = data.preventRoleDelete;
                 self.hasUnpermittedLocationRestriction = data.has_unpermitted_location_restriction || false;
+                self.restrictRoleChecked = ko.computed(function () {
+                    return data.manageRoleAssignments.specific.some(role => role.value() && !role.access_all_locations);
+                });
+                self.showRestrictedLocationRoleAssignmentWarning = ko.computed(function () {
+                    return self.permissions.access_all_locations() && self.restrictRoleChecked();
+                });
                 if (self.hasUnpermittedLocationRestriction) {
                     self.permissions.access_all_locations(true);
                 }
diff --git a/corehq/apps/users/templates/users/partials/edit_role_modal.html b/corehq/apps/users/templates/users/partials/edit_role_modal.html
index cc6b3319b2b3..5a0cdc507269 100644
--- a/corehq/apps/users/templates/users/partials/edit_role_modal.html
+++ b/corehq/apps/users/templates/users/partials/edit_role_modal.html
@@ -1,6 +1,13 @@
 {% load i18n %}
 {% load hq_shared_tags %}
+{% load compress %}
 
+{% compress css %}
+  <link type="text/less"
+        rel="stylesheet"
+        media="all"
+        href="{% static 'users/scss/roles.scss' %}" />
+{% endcompress %}
 
 <div data-bind="modal: roleBeingEdited" tabindex="-1" role="dialog" class="modal fade">
   <div data-bind="with: roleBeingEdited" class="modal-dialog">
@@ -304,7 +311,7 @@ <h4 class="modal-title" data-bind="text: modalTitle"></h4>
                     {% trans "Allow role to access data from all locations." %}
                   </label>
                 </div>
-                <div class="help-block alert alert-warning"
+                <div class="help-block alert alert-warning role-modal-warning"
                      data-bind="visible: !hasUnpermittedLocationRestriction && !permissions.access_all_locations()">
                   {% blocktrans %}
                     Make sure any users assigned this role also have a location assigned to them.
@@ -318,6 +325,12 @@ <h4 class="modal-title" data-bind="text: modalTitle"></h4>
                     full organization access for the assigned users.
                   {% endblocktrans %}
                 </div>
+                <div class="help-block alert alert-warning role-modal-warning" data-bind="visible: permissions.access_all_locations() && restrictRoleChecked()">
+                  {% blocktrans %}
+                    With the current configuration, this role will have full organization access and roles that can assign it will not.
+                    This is not reccomended.
+                  {% endblocktrans %}
+                </div>
               </div>
             </div>
             <div class="form-group">
@@ -454,7 +467,7 @@ <h4 class="modal-title" data-bind="text: modalTitle"></h4>
                   </div>
                   <div class="panel-body"
                        data-bind="foreach: manageRoleAssignments.specific, slideVisible: !manageRoleAssignments.all()">
-                    <div class="form-check">
+                    <div class="form-check" data-bind="css: {'bg-danger': !access_all_locations() && $parent.permissions.access_all_locations()}">
                       <input type="checkbox"
                              data-bind="checked: value, attr: { 'id': 'role-' + path() + '-checkbox' }" />
                       <label data-bind="attr: { 'for': 'role-' + path() + '-checkbox' }">
@@ -462,6 +475,12 @@ <h4 class="modal-title" data-bind="text: modalTitle"></h4>
                       </label>
                     </div>
                   </div>
+                  <div class="help-block alert alert-danger role-modal-warning" data-bind="visible: showRestrictedLocationRoleAssignmentWarning">
+                    {% blocktrans %}
+                      If the highlighted roles are selected, this role will have full organization access and be assignable
+                      by a role that does not. This is not reccomended configuration.
+                    {% endblocktrans %}
+                  </div>
                 </div>
               </div>
             </div>

From 757d8bb7948445a76ae78eaab5547503e9518de1 Mon Sep 17 00:00:00 2001
From: AddisonDunn <dunnaddisonw@gmail.com>
Date: Thu, 25 Apr 2024 11:06:49 -0400
Subject: [PATCH 2/3] use proper Bootstrap classes

---
 .../hqwebapp/static/users/scss/roles.scss     |  3 --
 corehq/apps/users/static/users/js/roles.js    |  7 +++++
 .../users/partials/edit_role_modal.html       | 31 +++++++------------
 3 files changed, 19 insertions(+), 22 deletions(-)
 delete mode 100644 corehq/apps/hqwebapp/static/users/scss/roles.scss

diff --git a/corehq/apps/hqwebapp/static/users/scss/roles.scss b/corehq/apps/hqwebapp/static/users/scss/roles.scss
deleted file mode 100644
index 32dbfa1a8806..000000000000
--- a/corehq/apps/hqwebapp/static/users/scss/roles.scss
+++ /dev/null
@@ -1,3 +0,0 @@
-.role-modal-warning { // Used in combination with Bootstrap's 'help-block'/'alert' classes
-    margin-bottom: 0px;
-}
diff --git a/corehq/apps/users/static/users/js/roles.js b/corehq/apps/users/static/users/js/roles.js
index a951e266b18d..629c2615a855 100644
--- a/corehq/apps/users/static/users/js/roles.js
+++ b/corehq/apps/users/static/users/js/roles.js
@@ -168,12 +168,19 @@ hqDefine('users/js/roles',[
                 };
                 self.preventRoleDelete = data.preventRoleDelete;
                 self.hasUnpermittedLocationRestriction = data.has_unpermitted_location_restriction || false;
+
                 self.restrictRoleChecked = ko.computed(function () {
                     return data.manageRoleAssignments.specific.some(role => role.value() && !role.access_all_locations);
                 });
                 self.showRestrictedLocationRoleAssignmentWarning = ko.computed(function () {
                     return self.permissions.access_all_locations() && self.restrictRoleChecked();
                 });
+                self.cantAccessAllLocations = ko.computed(function () {
+                    return !self.hasUnpermittedLocationRestriction && !self.permissions.access_all_locations();
+                });
+                self.unrestrictedButRestrictedRoleCanAssign = ko.computed(function () {
+                    return self.permissions.access_all_locations() && self.restrictRoleChecked();
+                });
                 if (self.hasUnpermittedLocationRestriction) {
                     self.permissions.access_all_locations(true);
                 }
diff --git a/corehq/apps/users/templates/users/partials/edit_role_modal.html b/corehq/apps/users/templates/users/partials/edit_role_modal.html
index 5a0cdc507269..0b4ef1c895ea 100644
--- a/corehq/apps/users/templates/users/partials/edit_role_modal.html
+++ b/corehq/apps/users/templates/users/partials/edit_role_modal.html
@@ -1,13 +1,5 @@
 {% load i18n %}
 {% load hq_shared_tags %}
-{% load compress %}
-
-{% compress css %}
-  <link type="text/less"
-        rel="stylesheet"
-        media="all"
-        href="{% static 'users/scss/roles.scss' %}" />
-{% endcompress %}
 
 <div data-bind="modal: roleBeingEdited" tabindex="-1" role="dialog" class="modal fade">
   <div data-bind="with: roleBeingEdited" class="modal-dialog">
@@ -296,7 +288,8 @@ <h4 class="modal-title" data-bind="text: modalTitle"></h4>
               </div>
             </div>
             <div class="form-group"
-                 data-bind="css: { 'has-error': hasUnpermittedLocationRestriction },
+                 data-bind="css: { 'has-error': hasUnpermittedLocationRestriction || unrestrictedButRestrictedRoleCanAssign,
+                                   'has-warning': cantAccessAllLocations },
                             visible: $root.canRestrictAccessByLocation || hasUnpermittedLocationRestriction">
               <label class="col-sm-4 control-label">
                 {% trans "Full Organization Access" %}
@@ -311,8 +304,8 @@ <h4 class="modal-title" data-bind="text: modalTitle"></h4>
                     {% trans "Allow role to access data from all locations." %}
                   </label>
                 </div>
-                <div class="help-block alert alert-warning role-modal-warning"
-                     data-bind="visible: !hasUnpermittedLocationRestriction && !permissions.access_all_locations()">
+                <div class="help-block"
+                     data-bind="visible: cantAccessAllLocations">
                   {% blocktrans %}
                     Make sure any users assigned this role also have a location assigned to them.
                     Users without assigned locations will not be permitted to log in.
@@ -325,7 +318,7 @@ <h4 class="modal-title" data-bind="text: modalTitle"></h4>
                     full organization access for the assigned users.
                   {% endblocktrans %}
                 </div>
-                <div class="help-block alert alert-warning role-modal-warning" data-bind="visible: permissions.access_all_locations() && restrictRoleChecked()">
+                <div class="help-block" data-bind="visible: unrestrictedButRestrictedRoleCanAssign">
                   {% blocktrans %}
                     With the current configuration, this role will have full organization access and roles that can assign it will not.
                     This is not reccomended.
@@ -460,7 +453,7 @@ <h4 class="modal-title" data-bind="text: modalTitle"></h4>
               <label class="col-sm-4 control-label">
                 {% trans "Manage Role Assignment" %}
               </label>
-              <div class="col-sm-8 controls">
+              <div class="col-sm-8 controls", data-bind="css: { 'has-error': showRestrictedLocationRoleAssignmentWarning }">
                 <div class="panel panel-default">
                   <div class="panel-heading">
                     {% trans "Select which other roles can assign this role:" %}
@@ -475,12 +468,12 @@ <h4 class="modal-title" data-bind="text: modalTitle"></h4>
                       </label>
                     </div>
                   </div>
-                  <div class="help-block alert alert-danger role-modal-warning" data-bind="visible: showRestrictedLocationRoleAssignmentWarning">
-                    {% blocktrans %}
-                      If the highlighted roles are selected, this role will have full organization access and be assignable
-                      by a role that does not. This is not reccomended configuration.
-                    {% endblocktrans %}
-                  </div>
+                </div>
+                <div class="help-block" data-bind="visible: showRestrictedLocationRoleAssignmentWarning">
+                  {% blocktrans %}
+                    If the highlighted roles are selected, this role will have full organization access and be assignable
+                    by a role that does not. This is not reccomended configuration.
+                  {% endblocktrans %}
                 </div>
               </div>
             </div>

From ea3a593bdbaccd7fea13abcfad4f17557dd37229 Mon Sep 17 00:00:00 2001
From: AddisonDunn <dunnaddisonw@gmail.com>
Date: Thu, 25 Apr 2024 17:06:04 -0400
Subject: [PATCH 3/3] spell recommend correctly

---
 .../apps/users/templates/users/partials/edit_role_modal.html  | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/corehq/apps/users/templates/users/partials/edit_role_modal.html b/corehq/apps/users/templates/users/partials/edit_role_modal.html
index 0b4ef1c895ea..f134eb1ca630 100644
--- a/corehq/apps/users/templates/users/partials/edit_role_modal.html
+++ b/corehq/apps/users/templates/users/partials/edit_role_modal.html
@@ -321,7 +321,7 @@ <h4 class="modal-title" data-bind="text: modalTitle"></h4>
                 <div class="help-block" data-bind="visible: unrestrictedButRestrictedRoleCanAssign">
                   {% blocktrans %}
                     With the current configuration, this role will have full organization access and roles that can assign it will not.
-                    This is not reccomended.
+                    This is not recommended.
                   {% endblocktrans %}
                 </div>
               </div>
@@ -472,7 +472,7 @@ <h4 class="modal-title" data-bind="text: modalTitle"></h4>
                 <div class="help-block" data-bind="visible: showRestrictedLocationRoleAssignmentWarning">
                   {% blocktrans %}
                     If the highlighted roles are selected, this role will have full organization access and be assignable
-                    by a role that does not. This is not reccomended configuration.
+                    by a role that does not. This is not recommended configuration.
                   {% endblocktrans %}
                 </div>
               </div>