feat(#9116): update user place field in admin to allow setting multiple places #9128

Benmuiruri · 2024-05-16T17:30:06Z

Description

This is a continuation of #6543

Now, users can be associated with more than one facility. This PR enables the CHT's Admin app to provide a way to associate those facilities with the user.

To get the feature to work, the admin user must enable can_have_multiple_places to some specific roles (e.g Supervisor, community_health_assistant, etc)

Whenever a new user is added:

The dialog displays, and the place field is a multiselect dropdown.
The admin submits the form and CHT validates:
- At least one of the selected roles has can_have_multiple_places
- Selected places are at the same hierarchy level
- The user should be a child of at least one of the selected places.
CHT displays error in form when above validations don't pass successfully.
When editing an existing user, the form should load with the multiselect field prefilled with the user's current place or places. On editing a user the form saves changes correctly

Closes #9116

Code review checklist

Readable: Concise, well named, follows the style guide, documented if necessary.
Documented: Configuration and user documentation on cht-docs
Tested: Unit and/or e2e where appropriate
Internationalised: All user facing text
Backwards compatible: Works with existing data and configuration or includes a migration. Any breaking changes documented in the release notes.

Compose URLs

If Build CI hasn't passed, these may 404:

License

The software is provided under AGPL-3.0. Contributions to this project are accepted under the same license.

latin-panda · 2024-05-23T08:52:52Z

@Benmuiruri, this is just a reminder to please enable the test tests/e2e/default/more-options-menu/offline-user/all-permissions.wdio-spec.js so that we can have all the tests working again.

ralfudx

All AC's in the description look good to me

dianabarsan

Cool stuff! Great work!
I left some comments mostly about readability and reusability.

admin/src/js/controllers/edit-user.js

dianabarsan · 2024-06-06T08:08:14Z

admin/src/js/controllers/edit-user.js

+      const allowedRoles = $scope.permissions.can_have_multiple_places;
+
+      const userHasPermission = userRoles.some(role => allowedRoles.includes(role));


I think we're duplicating logic that we already have in the cht script api. This should already be available in the admin app. https://github.com/medic/cht-core/blob/master/admin/src/js/services/auth.js#L65

Hmm, the value of userCtx.roles is admin. It ends up being a check whether current user (Admin) has the permission

Yea, I think you have to call it like:

return chtScriptApi.v1.hasPermissions(['can_have_multiple_places'], $scope.editUserModel.role, settings.permissions);

admin/src/js/controllers/edit-user.js

shared-libs/user-management/src/users.js

tests/integration/api/controllers/users.spec.js

webapp/src/ts/modules/contacts/contacts.component.ts

Benmuiruri · 2024-06-06T13:30:52Z

Hi @dianabarsan I have made almost all of the requested changes. The one I could not figure out is the suggestion to use _all_docs. Do you mind sharing a suggestion for that one. Thank you 🙏

admin/src/js/services/select2-search.js

This reverts commit d9b1cff.

admin/src/js/services/select2-search.js

dianabarsan

Nice work! Some minor things inline to unblock you. I'll give this a try tomorrow!

dianabarsan · 2024-06-11T17:38:46Z

admin/src/js/controllers/edit-user.js

+      const allowedRoles = $scope.permissions.can_have_multiple_places;
+
+      const userHasPermission = userRoles.some(role => allowedRoles.includes(role));


Yea, I think you have to call it like:

return chtScriptApi.v1.hasPermissions(['can_have_multiple_places'], $scope.editUserModel.role, settings.permissions);

dianabarsan · 2024-06-11T17:41:24Z

admin/src/js/controllers/edit-user.js

+      return DB().allDocs({ keys: placeIds, include_docs: true, limit: docsFetched })
+        .then(result => {
+          const places = result.rows.map(row => row.doc);
+          const contactTypes = new Set(places.map(place => place.contact_type));


I believe that checking the contact_type is not enough here.
A contact can have a type and a contact_type as well.

Have a look at the contact type utils library: https://github.com/medic/cht-core/blob/master/shared-libs/contact-types-utils/src/index.js#L13

I think this function, that checks whether a list of contacts are of the same type can belong there, instead of here.

dianabarsan · 2024-06-11T17:44:41Z

admin/src/js/controllers/edit-user.js

@@ -254,24 +277,57 @@ angular
      return hasPlace && hasContact;
    };

+    const validateFacilityHierarchy = () => {
+      const placeIds = $scope.editUserModel.place;
+      const docsFetched = 10;


It's unclear why we are fetching less docs than we are actually selecting?

Right, I forgot to take it out after some experimentation.

dianabarsan · 2024-06-11T17:45:13Z

admin/src/js/services/select2-search.js

+      };
+
+      const getDocs = function (ids) {
+        const docsFetched = 10;


It's unclear why we are limiting to 10 documents, when we can clearly have a longer selection.

dianabarsan · 2024-06-11T17:45:56Z

admin/src/js/services/select2-search.js

+        const docs = result.rows.map(row => row.doc);
+        const docIds = docs.map(doc => doc._id);


I think you can do this in a single line:

Suggested change

const docs = result.rows.map(row => row.doc);

const docIds = docs.map(doc => doc._id);

const docIds = result.rows.map(row => row.doc._id);

dianabarsan · 2024-06-11T17:48:51Z

admin/src/js/services/select2-search.js

+        return resolution.then(function() { //NoSONAR
+          $timeout(() => { //NoSONAR
+            selectEl.trigger('change');
+          }, 1000);


I think we should figure out why this timeout is needed. I'm not comfortable with keeping this at all.

This seems what's mainly left. I will dedicate tomorrow morning to it.

dianabarsan · 2024-06-11T17:49:29Z

admin/src/js/services/select2-search.js

+        selectEl.select2('data', select2Data);
+        $timeout(() => { //NoSONAR
+          selectEl.trigger('change');
+        }, 1000);


Same. We should not have delays like this, we need to figure out what is happening here.

dianabarsan · 2024-06-11T17:51:37Z

webapp/src/ts/modules/contacts/contacts.component.ts

@@ -257,6 +256,7 @@ export class ContactsComponent implements OnInit, AfterViewInit, OnDestroy {

  private canViewLastVisitedDate() {
    if (this.sessionService.isAdmin()) {
+      // disable UHC for DB admins


Is this added accidentally?

Benmuiruri force-pushed the 9116-update-admin-place-field branch from a50f55f to 94fb460 Compare May 17, 2024 07:07

Benmuiruri linked an issue May 22, 2024 that may be closed by this pull request

Make place field in the user form a multiselect dropdown #9116

Open

latin-panda mentioned this pull request May 22, 2024

feat(#6543): adds api-side support for users with multiple facilities #9126

Merged

5 tasks

Benmuiruri force-pushed the 9116-update-admin-place-field branch from 66c1166 to 7f86ea5 Compare May 28, 2024 08:51

Benmuiruri force-pushed the 6543-ui-changes branch from 114252d to da011a6 Compare May 28, 2024 09:00

Benmuiruri requested a review from ralfudx May 30, 2024 08:23

Benmuiruri marked this pull request as ready for review May 30, 2024 08:23

Benmuiruri mentioned this pull request May 30, 2024

Testing Supervisor Related Features medic/care-teams#122

Open

Benmuiruri force-pushed the 9116-update-admin-place-field branch from 9293711 to c1dd63f Compare May 31, 2024 06:30

Benmuiruri requested a review from dianabarsan June 4, 2024 13:32

ralfudx approved these changes Jun 5, 2024

View reviewed changes

dianabarsan requested changes Jun 6, 2024

View reviewed changes

Benmuiruri force-pushed the 6543-ui-changes branch from da011a6 to 03c645a Compare June 6, 2024 09:02

Benmuiruri mentioned this pull request Jun 6, 2024

chore: Add logic to fill up a report form that generates a task using the settings file #9144

Merged

5 tasks

Benmuiruri force-pushed the 6543-ui-changes branch from 03c645a to d2abe9b Compare June 7, 2024 05:14

Base automatically changed from 6543-ui-changes to master June 7, 2024 12:28

github-advanced-security bot found potential problems Jun 7, 2024

View reviewed changes

admin/src/js/services/select2-search.js Fixed Show fixed Hide fixed

Benmuiruri added 11 commits June 10, 2024 08:05

chore: make it work

6004a81

chore: clean up

462156b

chore: clean up

b756fba

chore: sonar fix

6b82020

chore: update unit test

ef9b7e3

chore: add child places test

135c803

chore: small fix

b55fcd2

chore: finish tests

b187512

chore: add test and refactor

a8d134a

chore: toggle displaying sort option

f13df29

chore: rebase

23cab51

Benmuiruri added 21 commits June 10, 2024 08:14

chore: turn off sonar

ac42a89

chore: turn off sonar

454f944

chore: final try

f921d56

chore: update model

d355d2d

chore: update permissions

ec190a4

chore: rebase

84ff5f9

chore: get tests passing

cb48aec

chore: add multi place tests

e46bf1a

chore: modify url

e6abffd

chore: fix test

5256cd6

chore: fix select bug

a74d0ed

chore: rebase

7be0522

chore: fix test

408633d

Revert "chore: fix test"

3e18ebc

This reverts commit d9b1cff.

chore: feedback

aa12087

chore: feedback

d3d8b7a

chore: adjust help text

5dcbe72

chore: add fr and sw translations

e2617a6

chore: use allDocs

2c3f04d

chore: delete white spaces

e9ddada

chore: update test

43a970b

Benmuiruri force-pushed the 9116-update-admin-place-field branch from 9c72aa4 to 43a970b Compare June 10, 2024 06:18

chore: add select allDocs

b74da3c

github-advanced-security bot found potential problems Jun 10, 2024

View reviewed changes

admin/src/js/services/select2-search.js Dismissed Show dismissed Hide dismissed

Benmuiruri added 4 commits June 10, 2024 12:14

sonar fix

c15ad9f

nepali translations:

43cd038

final sonar

9f33bef

fix typo

e98269e

Benmuiruri requested a review from dianabarsan June 10, 2024 10:23

dianabarsan requested changes Jun 11, 2024

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat(#9116): update user place field in admin to allow setting multiple places #9128

feat(#9116): update user place field in admin to allow setting multiple places #9128

Benmuiruri commented May 16, 2024 •

edited

latin-panda commented May 23, 2024

ralfudx left a comment

dianabarsan left a comment

dianabarsan Jun 6, 2024

Benmuiruri Jun 6, 2024

dianabarsan Jun 11, 2024

Benmuiruri commented Jun 6, 2024

dianabarsan left a comment

dianabarsan Jun 11, 2024

dianabarsan Jun 11, 2024

dianabarsan Jun 11, 2024

Benmuiruri Jun 11, 2024

dianabarsan Jun 11, 2024

dianabarsan Jun 11, 2024

dianabarsan Jun 11, 2024

Benmuiruri Jun 11, 2024

dianabarsan Jun 11, 2024

dianabarsan Jun 11, 2024

		const allowedRoles = $scope.permissions.can_have_multiple_places;

		const userHasPermission = userRoles.some(role => allowedRoles.includes(role));

		const docs = result.rows.map(row => row.doc);
		const docIds = docs.map(doc => doc._id);

	const docs = result.rows.map(row => row.doc);
	const docIds = docs.map(doc => doc._id);
	const docIds = result.rows.map(row => row.doc._id);

feat(#9116): update user place field in admin to allow setting multiple places #9128

Are you sure you want to change the base?

feat(#9116): update user place field in admin to allow setting multiple places #9128

Conversation

Benmuiruri commented May 16, 2024 • edited

Description

Code review checklist

Compose URLs

License

latin-panda commented May 23, 2024

ralfudx left a comment

Choose a reason for hiding this comment

dianabarsan left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Benmuiruri commented Jun 6, 2024

dianabarsan left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Benmuiruri commented May 16, 2024 •

edited