Support multiple roles on a contact type #86

paulpascal · 2024-03-11T01:24:27Z

Fix #69

change user_role to string[]
have a config function to generate userRoleProperty anytime we may need it (default property_name, type,... and only passing the user_role as parameter)
implement changes in Place and PlaceFactory to process the new property
add ValidatorRole to handle check selected roles against user_role (behaving as allowed roles when having more than one item)
update in UserPayload (to only use role and no more type)
update concerned templates
update doc

kennsippell

Nice work, buddy! Really really nice start on this work.

src/public/place/create_form.html

src/public/place/bulk_create_form.html

src/public/place/create_form.html

test/services/upload-manager.spec.ts

kennsippell · 2024-03-11T06:26:17Z

README.md

@@ -44,6 +44,7 @@ Property | Type | Description
 `contact_types.replacement_property` | Property | Defines how this `contact_type` is described when being replaced. The `property_name` is always `replacement`. See [ConfigProperty](#ConfigProperty).
 `contact_types.place_properties` | Array<ConfigProperty> | Defines the attributes which are collected and set on the user's created place. See [ConfigProperty](#ConfigProperty).
 `contact_types.contact_properties` | Array<ConfigProperty> | Defines the attributes which are collected and set on the user's primary contact doc. See [ConfigProperty](#ConfigProperty).
+`contact_types.user_roles_property` | ConfigProperty | Defines the property used for specifying user role(s) when creating users of this contact type. Add flexibility to choose role(s) when there are multiple roles available at this level of your hierarchy. See [ConfigProperty](#ConfigProperty).


I think it is important that we somehow capture the "allowed roles" at each level. Currently, this feature supports free-text input but what if somebody uses the wrong case? What if they use a CHW role for a supervisor user (this can actually result in quite serious bugs)? What if there are trailing spaces on the role, which is super common when working with CSVs?

Ideally, I think the best user experience for this feature would be a set of checkboxes like in the CHT today (see screenshot). You don't need to implement that UI here in this PR, we can start with free-text; but if we capture the "allowed roles" and validate them, I think that's a good start.

There are probably a few ways to approach this. But you'll want to validate this propery like the other property types. You'll probably end up with something like argument: ["role1", "role2"].

This conversation may be useful or consideration.

Tried to use it, but since select_multiple only accept parameter to be object for the moment (not simple array).

Got a simple role validator that can do the job, maybe the time to complete select_multiple feature.

src/config/index.ts

src/services/user-payload.ts

…upport-multiple-level-roles

paulpascal · 2024-03-12T18:12:10Z

Hi @kennsippell, thank you for your thorough review - I have worked on them.
Whenever you have the time, could you please take another look? I appreciate your feedback.
Thanks!

kennsippell

Great progress @paulpascal !

src/lib/validation.ts

src/liquid/place/bulk_create_form.html

src/config/index.ts

test/services/place.spec.ts

test/mocks.ts

src/liquid/place/bulk_create_form.html

test/lib/validation.spec.ts

src/liquid/place/list.html

…upport-multiple-level-roles

…evel-roles

paulpascal · 2024-03-15T17:02:40Z

Hello @kennsippell, had some work after our discussion, and made changes accordingly.
Would you please review it back ?

Many thanks

kennsippell

Nice. Feels very close!

kennsippell · 2024-03-18T10:00:15Z

src/lib/validator-role.ts

+import { ContactProperty } from '../config';
+import { IValidator } from './validation';
+
+export default class ValidatorRole implements IValidator {


can this be repalced with @inromualdo 's select_multiple? or are you planning to keep this?

i assumed you're going to delete this; so i didn't review this file. let me know if this is incorrect

Yes @kennsippell , of course. Was waiting for his work to be validated to rebase.
But I can rebase right now it is okay with you.

ideally i think we would set Ro's work to be the base branch for this work. then we would merge Ro's PR first.

given the circumstances, i'm fine with pushing this change forward as-is and we can ensure we delete it in Ro's PR

src/liquid/place/bulk_create_form.html

src/services/place-factory.ts

test/mocks.ts

test/services/upload-manager.spec.ts

README.md

…evel-roles

paulpascal · 2024-03-18T16:41:09Z

Nice. Feels very close!

Thanks @kennsippell , feedback addressed

kennsippell

Very well done, sir

src/config/index.ts

kennsippell · 2024-03-19T09:46:32Z

src/lib/validator-role.ts

+import { ContactProperty } from '../config';
+import { IValidator } from './validation';
+
+export default class ValidatorRole implements IValidator {


ideally i think we would set Ro's work to be the base branch for this work. then we would merge Ro's PR first.

given the circumstances, i'm fine with pushing this change forward as-is and we can ensure we delete it in Ro's PR

kennsippell · 2024-03-19T09:51:24Z

src/liquid/place/bulk_create_form.html

+            <tr>
+              <td>{{ userRoleProperty.friendly_name }}</td>
+              <td>{{ userRoleProperty.required }}</td>
+              <td>Role of the user. Specify multiple roles with <i>space</i>. Allowed roles are <i>{{ contactType.user_role | join: ', ' }}


Suggested change

<td>Role of the user. Specify multiple roles with space. Allowed roles are {{ contactType.user_role | join: ', ' }}

<td>Role of the user. Separate multiple roles with a space. Allowed roles are {{ contactType.user_role | join: ', ' }}

Can you have a space in a valid CouchDB role? If so, we should change to ,.

src/services/place.ts

test/services/place.spec.ts

paulpascal added 3 commits March 11, 2024 01:10

feat(#69): add support for multiple user roles at a level

8f98ce2

test(#69): add unit test

6ccf846

doc(#69): update doc

752cb4e

paulpascal requested a review from kennsippell March 11, 2024 01:24

kennsippell requested changes Mar 11, 2024

View reviewed changes

Merge branch 'main' of github.com:medic/cht-user-management into 69-s…

0129dc0

…upport-multiple-level-roles

kennsippell mentioned this pull request Mar 12, 2024

Gender form input should be a dropdown instead of freetext field #55

Closed

paulpascal added 3 commits March 12, 2024 18:00

fix(#69): code review feedback

afa23b9

Merge branch 'main' of github.com:medic/cht-user-management into 69-s…

25498c7

…upport-multiple-level-roles

fix(#69): update doc

bb142ee

kennsippell self-requested a review March 12, 2024 18:33

kennsippell requested changes Mar 14, 2024

View reviewed changes

paulpascal added 5 commits March 14, 2024 09:23

Merge branch 'main' of github.com:medic/cht-user-management into 69-s…

1114bcb

…upport-multiple-level-roles

fix(#69): address review feedback

9a6be77

docs(#69): update readme

ac8cabb

fix(#69): delegate validation to role validator

0e51ad1

Merge remote-tracking branch 'origin/main' into 69-support-multiple-l…

8b5f13e

…evel-roles

paulpascal requested a review from kennsippell March 15, 2024 16:40

kennsippell requested changes Mar 18, 2024

View reviewed changes

kennsippell reviewed Mar 18, 2024

View reviewed changes

README.md Outdated Show resolved Hide resolved

paulpascal added 2 commits March 18, 2024 16:00

fix(#69): address kenn's feedback

0e3c200

Merge remote-tracking branch 'origin/main' into 69-support-multiple-l…

72350db

…evel-roles

paulpascal requested a review from kennsippell March 18, 2024 16:13

kennsippell approved these changes Mar 19, 2024

View reviewed changes

paulpascal added 2 commits March 19, 2024 11:58

fix(#69): address final feedback

86d51c9

1.1.6

2670290

paulpascal merged commit b110678 into main Mar 19, 2024
1 check passed

paulpascal deleted the 69-support-multiple-level-roles branch March 27, 2024 06:25

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Support multiple roles on a contact type #86

Support multiple roles on a contact type #86

paulpascal commented Mar 11, 2024 •

edited

Loading

kennsippell left a comment

kennsippell Mar 11, 2024

kennsippell Mar 12, 2024 •

edited

Loading

paulpascal Mar 15, 2024

paulpascal commented Mar 12, 2024

kennsippell left a comment

paulpascal commented Mar 15, 2024

kennsippell left a comment

kennsippell Mar 18, 2024 •

edited

Loading

paulpascal Mar 18, 2024

kennsippell Mar 19, 2024

paulpascal commented Mar 18, 2024

kennsippell left a comment

kennsippell Mar 19, 2024

kennsippell Mar 19, 2024

	<td>Role of the user. Specify multiple roles with <i>space</i>. Allowed roles are <i>{{ contactType.user_role \| join: ', ' }}
	<td>Role of the user. Separate multiple roles with a <i>space</i>. Allowed roles are <i>{{ contactType.user_role \| join: ', ' }}

Support multiple roles on a contact type #86

Support multiple roles on a contact type #86

Conversation

paulpascal commented Mar 11, 2024 • edited Loading

kennsippell left a comment

Choose a reason for hiding this comment

kennsippell Mar 11, 2024

Choose a reason for hiding this comment

kennsippell Mar 12, 2024 • edited Loading

Choose a reason for hiding this comment

paulpascal Mar 15, 2024

Choose a reason for hiding this comment

paulpascal commented Mar 12, 2024

kennsippell left a comment

Choose a reason for hiding this comment

paulpascal commented Mar 15, 2024

kennsippell left a comment

Choose a reason for hiding this comment

kennsippell Mar 18, 2024 • edited Loading

Choose a reason for hiding this comment

paulpascal Mar 18, 2024

Choose a reason for hiding this comment

kennsippell Mar 19, 2024

Choose a reason for hiding this comment

paulpascal commented Mar 18, 2024

kennsippell left a comment

Choose a reason for hiding this comment

kennsippell Mar 19, 2024

Choose a reason for hiding this comment

kennsippell Mar 19, 2024

Choose a reason for hiding this comment

paulpascal commented Mar 11, 2024 •

edited

Loading

kennsippell Mar 12, 2024 •

edited

Loading

kennsippell Mar 18, 2024 •

edited

Loading