search

LibreHealthIO / lh-ehr-laravel

LibreHealth EHR laravel
27 stars 37 forks source link

GSOC-23: Roles and User Modules #43

Open chaitak-gorai opened 1 year ago

chaitak-gorai commented 1 year ago

Description

This PR is for the Google Summer of Code Project : Migrate EHR to Laravel: Users and Roles Module .

Workflow

*All the commits will be added to this for the final merge to the codebase

chaitak-gorai commented 1 year ago

Added Roles UI with functionalities of creating and fetching roles

Screenshot

image

chaitak-gorai commented 1 year ago

Update: Added Laratrus Seeder for Roles

@muarachmann, I have updated the changes you suggested in the earlier commit. In a later commit, I added the Laratrust Seeder, which will assign 'c,r,u,d' permissions to each module's role.

In config/laratrust_seeder.php I have done this :

Can you please REVIEW these access and suggest to me the changes we need to make??

muarachmann commented 1 year ago

I will review the roles/module in details later this week

chaitak-gorai commented 1 year ago

Update:

@muarachmann, I have updated some changes here I m listing for your review.

Screenshot of update Roles.vue UI image

Can you please REVIEW these changes and suggest to me the changes we need to make??

chaitak-gorai commented 1 year ago

Update:

@muarachmann, I have updated your suggested reviews. Also, I have created the Add Users UI form page with the fields; I have added all necessary form fields. I have a doubt whether the name can be in a single Full Name field or in parts. Please review it and suggest changes if required.

I ll then implement the functions

image

tmccormi commented 1 year ago

We should stick with separate fields.

… But I find it an interesting thought experiment to consider the implications of one name field as better internationally. Search is fast and easy these days, so finding patients would not be an issue.

Sharing data externally would; in the USA, as healthcare businesses are all stuck in old models.

Tony

On Fri, Jul 7, 2023 at 5:56 PM Chaitak Gorai @.***> wrote:

Update:

@muarachmann https://github.com/muarachmann, I have updated your suggested reviews. Also, I have created the Add Users UI form page with the fields; I have added all necessary form fields. I have a doubt whether the name can be in a single Full Name field or in parts. Please review it and suggest changes if required.

I ll then implement the functions

[image: image] https://user-images.githubusercontent.com/77141674/251884447-4283a838-75db-4878-aa4c-63fd8d108006.png

— Reply to this email directly, view it on GitHub https://github.com/LibreHealthIO/lh-ehr-laravel/pull/43#issuecomment-1626399181, or unsubscribe https://github.com/notifications/unsubscribe-auth/AACFZC324C54XWM3WP4KX4LXPCV2BANCNFSM6AAAAAAZA67IAQ . You are receiving this because you are subscribed to this thread.Message ID: @.***>

-- https://thinkstorm.net 503-330-2239

tmccormi commented 1 year ago

The users name as a single field would work , if we ONLY used it for user identification purpose, however we use the provider’s user account on external forms that require separate fields, including title.

On Sat, Jul 8, 2023 at 9:05 AM Tony McCormick @.***> wrote:

We should stick with separate fields.

… But I find it an interesting thought experiment to consider the implications of one name field as better internationally. Search is fast and easy these days, so finding patients would not be an issue.

Sharing data externally would; in the USA, as healthcare businesses are all stuck in old models.

Tony

On Fri, Jul 7, 2023 at 5:56 PM Chaitak Gorai @.***> wrote:

Update:

@muarachmann https://github.com/muarachmann, I have updated your suggested reviews. Also, I have created the Add Users UI form page with the fields; I have added all necessary form fields. I have a doubt whether the name can be in a single Full Name field or in parts. Please review it and suggest changes if required.

I ll then implement the functions

[image: image] https://user-images.githubusercontent.com/77141674/251884447-4283a838-75db-4878-aa4c-63fd8d108006.png

— Reply to this email directly, view it on GitHub https://github.com/LibreHealthIO/lh-ehr-laravel/pull/43#issuecomment-1626399181, or unsubscribe https://github.com/notifications/unsubscribe-auth/AACFZC324C54XWM3WP4KX4LXPCV2BANCNFSM6AAAAAAZA67IAQ . You are receiving this because you are subscribed to this thread.Message ID: @.***>

-- https://thinkstorm.net 503-330-2239

-- https://thinkstorm.net 503-330-2239

chaitak-gorai commented 1 year ago

The users name as a single field would work , if we ONLY used it for user identification purpose, however we use the provider’s user account on external forms that require separate fields, including title. On Sat, Jul 8, 2023 at 9:05 AM Tony McCormick @.> wrote: We should stick with separate fields. … But I find it an interesting thought experiment to consider the implications of one name field as better internationally. Search is fast and easy these days, so finding patients would not be an issue. Sharing data externally would; in the USA, as healthcare businesses are all stuck in old models. Tony On Fri, Jul 7, 2023 at 5:56 PM Chaitak Gorai @.> wrote: > Update: > > @muarachmann https://github.com/muarachmann, I have updated your > suggested reviews. Also, I have created the Add Users UI form page with > the fields; > I have added all necessary form fields. I have a doubt whether the name > can be in a single Full Name field or in parts. > Please review it and suggest changes if required. > > I ll then implement the functions > > [image: image] > https://user-images.githubusercontent.com/77141674/251884447-4283a838-75db-4878-aa4c-63fd8d108006.png > > — > Reply to this email directly, view it on GitHub > <#43 (comment)>, > or unsubscribe > https://github.com/notifications/unsubscribe-auth/AACFZC324C54XWM3WP4KX4LXPCV2BANCNFSM6AAAAAAZA67IAQ > . > You are receiving this because you are subscribed to this thread.Message > ID: @.***> > -- https://thinkstorm.net 503-330-2239 -- https://thinkstorm.net 503-330-2239

so as of now can we move forward with a single user name field? I think it will be an efficient way to store the name.

tmccormi commented 1 year ago

No, The provider data is used on claim forms that require separate fields. Wish it was otherwise. Mua, what do you think?

Tony

On Sat, Jul 8, 2023 at 9:16 AM Chaitak Gorai @.***> wrote:

The users name as a single field would work , if we ONLY used it for user identification purpose, however we use the provider’s user account on external forms that require separate fields, including title. On Sat, Jul 8, 2023 at 9:05 AM Tony McCormick @. > wrote: We should stick with separate fields. … But I find it an interesting thought experiment to consider the implications of one name field as better internationally. Search is fast and easy these days, so finding patients would not be an issue. Sharing data externally would; in the USA, as healthcare businesses are all stuck in old models. Tony On Fri, Jul 7, 2023 at 5:56 PM Chaitak Gorai @.> wrote: > Update: > > @muarachmann https://github.com/muarachmann https://github.com/muarachmann, I have updated your > suggested reviews. Also, I have created the Add Users UI form page with > the fields; > I have added all necessary form fields. I have a doubt whether the name > can be in a single Full Name field or in parts. > Please review it and suggest changes if required. > > I ll then implement the functions > > [image: image] > https://user-images.githubusercontent.com/77141674/251884447-4283a838-75db-4878-aa4c-63fd8d108006.png

— > Reply to this email directly, view it on GitHub > <#43 (comment) https://github.com/LibreHealthIO/lh-ehr-laravel/pull/43#issuecomment-1626399181>, or unsubscribe > https://github.com/notifications/unsubscribe-auth/AACFZC324C54XWM3WP4KX4LXPCV2BANCNFSM6AAAAAAZA67IAQ . > You are receiving this because you are subscribed to this thread.Message > ID: @.***> > -- https://thinkstorm.net 503-330-2239 -- https://thinkstorm.net 503-330-2239

so as of now can we move forward with a single user name field? I think it will be an efficient way to store the name.

— Reply to this email directly, view it on GitHub https://github.com/LibreHealthIO/lh-ehr-laravel/pull/43#issuecomment-1627391990, or unsubscribe https://github.com/notifications/unsubscribe-auth/AACFZCYVHONB3LULTEXV7ZLXPGBTRANCNFSM6AAAAAAZA67IAQ . You are receiving this because you commented.Message ID: @.***>

-- https://thinkstorm.net 503-330-2239

muarachmann commented 1 year ago

Thanks @tmccormi for your inputs, lets separate it at least to given name & surname. That is the norm I see other wise we can keep the old format.

chaitak-gorai commented 1 year ago

Thanks @tmccormi for your inputs, lets separate it at least to given name & surname. That is the norm I see other wise we can keep the old format.

ok i ll update it to first name and last name

muarachmann commented 1 year ago

@chaitak-gorai we need to equally work on the invitation module. This when a user is created should send an email to them to accept the invite

muarachmann commented 1 year ago

With this we can then focus on the restrictions of the users -roles / permissions

chaitak-gorai commented 1 year ago

@chaitak-gorai we need to equally work on the invitation module. This when a user is created should send an email to them to accept the invite

yes @muarachmann i am going through it. But cant figure out how to properly implement that. Can we have a call today or tomorrow for a short discussion on this ??

muarachmann commented 1 year ago

@chaitak-gorai we need to equally work on the invitation module. This when a user is created should send an email to them to accept the invite

yes @muarachmann i am going through it. But cant figure out how to properly implement that. Can we have a call today or tomorrow for a short discussion on this ??

Yes @chaitak-gorai let's have a call tomorrow 1PM GMT + 1

chaitak-gorai commented 1 year ago

@chaitak-gorai we need to equally work on the invitation module. This when a user is created should send an email to them to accept the invite

yes @muarachmann i am going through it. But cant figure out how to properly implement that. Can we have a call today or tomorrow for a short discussion on this ??

Yes @chaitak-gorai let's have a call tomorrow 1PM GMT

okk thank you

chaitak-gorai commented 1 year ago

Update: Add User Via Invitation

@muarachmann I have added the functionality of adding a user via Mail invitation. After admin add a user, the user will get a mail in his email and a link to set a password. There is a token which is valid for 7 days. If it gets expired the user cant add a password. Here are the changes i have made

  1. Add users form updated for the name fields
  2. Update the User Model and database schema for form fields that were not there and new fields like token, and token_expiry time for verification.
  3. Create a Mail class for sending mail.
  4. Created the Add Password form where the user will provide a new password.
  5. Added new routes to web.php

Task Left To be done:

image

muarachmann commented 1 year ago

Hi @chaitak-gorai ,

This is no where close to what I asked you to do please consider the architecture where invitations are managed separately see - https://github.com/junaidnasir/larainvite

Also please user mailhog / mailpit for testing emails locally rather than using your gmail.

regards. MR

chaitak-gorai commented 1 year ago

Hi @chaitak-gorai ,

This is no where close to what I asked you to do please consider the architecture where invitations are managed separately see - https://github.com/junaidnasir/larainvite

Also please user mailhog / mailpit for testing emails locally rather than using your gmail.

regards. MR

Thank you for correcting me. I went through the suggested code and I got the idea where I am wrong. Will be updating the changes in a new commit.

chaitak-gorai commented 1 year ago

hey @muarachmann, I have updated the code with a separate invitation class to handle all invitation-related logic. Also, I have added two tables user_invitations which will keep the details about the invitation, and users_invited which will keep the user form data that will be used to create the real user when the invite is accepted. Also, update the login in the Invitation accepting URL. Based on the status of the invitation the URL will be shown.

Please review whether this approach is correct or not and suggest me changes if required. After this, I will be working on the dashboard to show all the invitations with their status.

muarachmann commented 1 year ago

Hi @chaitak-gorai this looks like it, no need for another model UserInvite let's create them as User model and attach it. Also we can rename the table to invitations . Also the invitatation should be facility specific, You can use morphs for this to handle even the invitee

chaitak-gorai commented 1 year ago

@muarachmann yes i have kept the invite in the User Model only in my last approach.

But I thought it will be better if we keep them separate as the user can reject an invitation or it can expire. In that case, the created user will be null and void. So creating the user only after the invitation is accepted and a password is set .

Should i revert this and keep a single table? I will be updating the suggested changes then.

muarachmann commented 1 year ago

Yes please keep a single table. If a user rejects, just mark the user as deleted, inactive

On Wed, Jul 26, 2023 at 1:55 AM Chaitak Gorai @.***> wrote:

@muarachmann https://github.com/muarachmann yes i have kept the invite in the User Model only in my last approach.

But I thought it will be better if we keep them separate as the user can reject an invitation or it can expire. In that case, the created user will be null and void. So creating the user only after the invitation is accepted and a password is set .

Should i revert this and keep a single table? I will be updating the suggested changes then.

— Reply to this email directly, view it on GitHub https://github.com/LibreHealthIO/lh-ehr-laravel/pull/43#issuecomment-1650804296, or unsubscribe https://github.com/notifications/unsubscribe-auth/AD4X4XURLBI5437ZOTGVN7TXSBTGNANCNFSM6AAAAAAZA67IAQ . You are receiving this because you were mentioned.Message ID: @.***>

--

MUA N. LAURENT: Head of Engineering WASPITO https://waspito.com/ @.*** Bonamoussadi Denver, Douala

#HealthWithoutAStep

chaitak-gorai commented 1 year ago

Update :

@muarachmann, I have updated the suggested changes.

[Help Required] I have used Axios to send some requests where JSON response is needed. I can't figure out how to implement it with Inertia. Can you help me with this?

muarachmann commented 1 year ago

Update :

@muarachmann, I have updated the suggested changes.

  • Drop the user_invited table. Only single table for User. and renamed the user_inviations into Invitation.
  • used token instead of code

[Help Required] I have used Axios to send some requests where JSON response is needed. I can't figure out how to implement it with Inertia. Can you help me with this?

Why do you need a json response in those routes ? That is the first question you should be asking yourself. You don't need a separate route for the inivitation status, if it has expired show it in the view that it has expired.

With regards to the invitations table.

Emails are unique for these kind of stuffs and we can have the facility that invited the user. What I suggest is using Morph relationship to know the invitee, invitor and facility in the invitations table and also that will help isolate the for_user used in the users table which has no need. In the end you will have a powerful module that will manage users invitations accross the app. If you are worried about roles and permissions, You are free to store it in the invitations table column say invite_params as a json object where when accepted you can assign the user these roles. So a user can be created with basic user access role.

Let me know if you need a call for this Regards, MR

chaitak-gorai commented 1 year ago

Update :

@muarachmann, I have updated the suggested changes.

  • Drop the user_invited table. Only single table for User. and renamed the user_inviations into Invitation.
  • used token instead of code

[Help Required] I have used Axios to send some requests where JSON response is needed. I can't figure out how to implement it with Inertia. Can you help me with this?

Why do you need a json response in those routes ? That is the first question you should be asking yourself. You don't need a separate route for the inivitation status, if it has expired show it in the view that it has expired.

With regards to the invitations table.

Emails are unique for these kind of stuffs and we can have the facility that invited the user. What I suggest is using Morph relationship to know the invitee, invitor and facility in the invitations table and also that will help isolate the for_user used in the users table which has no need. In the end you will have a powerful module that will manage users invitations accross the app. If you are worried about roles and permissions, You are free to store it in the invitations table column say invite_params as a json object where when accepted you can assign the user these roles. So a user can be created with basic user access role.

Let me know if you need a call for this Regards, MR

Yes, it will be better to discuss this on a call. Are you free tomorrow ??

muarachmann commented 1 year ago

Tomorrow 1 pm WAT is fine with me.

On Thu, 27 Jul 2023 at 12:08 Chaitak Gorai @.***> wrote:

Update :

@muarachmann https://github.com/muarachmann, I have updated the suggested changes.

  • Drop the user_invited table. Only single table for User. and renamed the user_inviations into Invitation.
  • used token instead of code

[Help Required] I have used Axios to send some requests where JSON response is needed. I can't figure out how to implement it with Inertia. Can you help me with this?

Why do you need a json response in those routes ? That is the first question you should be asking yourself. You don't need a separate route for the inivitation status, if it has expired show it in the view that it has expired.

With regards to the invitations table.

Emails are unique for these kind of stuffs and we can have the facility that invited the user. What I suggest is using Morph relationship to know the invitee, invitor and facility in the invitations table and also that will help isolate the for_user used in the users table which has no need. In the end you will have a powerful module that will manage users invitations accross the app. If you are worried about roles and permissions, You are free to store it in the invitations table column say invite_params as a json object where when accepted you can assign the user these roles. So a user can be created with basic user access role.

Let me know if you need a call for this Regards, MR

Yes, it will be better to discuss this on a call. Are you free tomorrow ??

— Reply to this email directly, view it on GitHub https://github.com/LibreHealthIO/lh-ehr-laravel/pull/43#issuecomment-1653399957, or unsubscribe https://github.com/notifications/unsubscribe-auth/AD4X4XWTW2UVNSDWEAL46N3XSJD2DANCNFSM6AAAAAAZA67IAQ . You are receiving this because you were mentioned.Message ID: @.***>

chaitak-gorai commented 1 year ago

Tomorrow 1 pm WAT is fine with me. … On Thu, 27 Jul 2023 at 12:08 Chaitak Gorai @.> wrote: Update : @muarachmann https://github.com/muarachmann, I have updated the suggested changes. - Drop the user_invited table. Only single table for User. and renamed the user_inviations into Invitation. - used token instead of code [Help Required] I have used Axios to send some requests where JSON response is needed. I can't figure out how to implement it with Inertia. Can you help me with this? Why do you need a json response in those routes ? That is the first question you should be asking yourself. You don't need a separate route for the inivitation status, if it has expired show it in the view that it has expired. With regards to the invitations table. Emails are unique for these kind of stuffs and we can have the facility that invited the user. What I suggest is using Morph relationship to know the invitee, invitor and facility in the invitations table and also that will help isolate the for_user used in the users table which has no need. In the end you will have a powerful module that will manage users invitations accross the app. If you are worried about roles and permissions, You are free to store it in the invitations table column say invite_params as a json object where when accepted you can assign the user these roles. So a user can be created with basic user access role. Let me know if you need a call for this Regards, MR Yes, it will be better to discuss this on a call. Are you free tomorrow ?? — Reply to this email directly, view it on GitHub <#43 (comment)>, or unsubscribe https://github.com/notifications/unsubscribe-auth/AD4X4XWTW2UVNSDWEAL46N3XSJD2DANCNFSM6AAAAAAZA67IAQ . You are receiving this because you were mentioned.Message ID: @.> Hey @muarachmann I got some urgent family commitments today. Can it be rescheduled for tomorrow at 1 PM?

chaitak-gorai commented 1 year ago

Update

muarachmann commented 1 year ago

@chaitak-gorai cool we are getting there... left some reviews

chaitak-gorai commented 1 year ago

@muarachmann I have updated the suggested changes

chaitak-gorai commented 1 year ago

Invitations List UI

Hey @muarachmann, I have created the dashboard UI to show the list of Invitations sent to the Users.

here is the Screenshot. Please give it a review and suggest me changes if required. image

chaitak-gorai commented 1 year ago

Update: Use of Datatable, Pagination, Filtering

@muarachmann, I have updated the reviewed changes.

Please review and suggest to me if further updates are required.

muarachmann commented 1 year ago

@chaitak-gorai good job... good progress so far. Now ensure everything in invitations is under the users folder (frontend). Also ensure you extend the layouts from the dashboard layout and the accept/reject from the auth layout. With that just a few polishing we can work on the headers and access by roles now

chaitak-gorai commented 1 year ago

Thank You @muarachmann for the reviews. I have a great learning curve so far in this project.

chaitak-gorai commented 1 year ago

Update

This is how it looks: image

muarachmann commented 1 year ago

Update

  • Create a User Profile Page with the current details from the Add-User form. As more fields are added we can update it.
  • Linked this in the invitations list for each User.

This is how it looks: image

Nice @chaitak-gorai. Please add buttons to edit profile and roles. Also add breadcrumbs like the other screens so it looks better.

chaitak-gorai commented 1 year ago

Hey @muarachmann i have a query about the value provided in the active link of Breadcrumb component as like in dashboard page it is set to activeLink: i18n.t('general.dashboard'), how it is set like we just give a name of the page or it is dynamically calculated? I have a confusion here. Similarly for the users page will we just pass "userProfile" in this function?

chaitak-gorai commented 1 year ago

Update: Edit User Functionality

image

muarachmann commented 1 year ago

Hey @muarachmann i have a query about the value provided in the active link of Breadcrumb component as like in dashboard page it is set to activeLink: i18n.t('general.dashboard'), how it is set like we just give a name of the page or it is dynamically calculated? I have a confusion here. Similarly for the users page will we just pass "userProfile" in this function?

Good question, look at the patient profile component in the codebase.

chaitak-gorai commented 1 year ago

Thank you for the reviews. I will update them ASAP.

chaitak-gorai commented 1 year ago

Update:

@muarachmann these are the bunch of changes I have updated based on your reviews. Please give it a review and suggest how to proceed further.

chaitak-gorai commented 1 year ago

@muarachmann i have added a multiselect input type in EhrInput and replaced the old select dropdown for permission with this component in Roles.vue page. Please review this and suggest me changes if required.

muarachmann commented 1 year ago

Hi @chaitak-gorai please use this - MultiSelect component in the project

chaitak-gorai commented 1 year ago

Hi @chaitak-gorai please use this - MultiSelect component in the project

I looked for the component in the project but couldn't able to find it. Can you please tell me where it is located. Do you mean by the vue-multiselect package?

muarachmann commented 1 year ago

@chaitak-gorai do not forget to list the users in the datatable as well. Regards

chaitak-gorai commented 1 year ago

Update

@muarachmann i have added these changes

I haven't hide the menu link in frontend. It will be better if we show them a Acess Denied Page instead . Any thought?

muarachmann commented 1 year ago

Hi @chaitak-gorai will be reviewing this this weekend for a merge. Anything you have to polish?

chaitak-gorai commented 1 year ago

Hi @chaitak-gorai will be reviewing this this weekend for a merge. Anything you have to polish?

Hey @muarachmann, thank you for the update. I'll look into it and do the necessary polishing by the weekend. There are some validations left i guess. I'll update the changes if any. Regards, Chaitak Gorai

chaitak-gorai commented 1 year ago

Hey @muarachmann, i have added the validation for the Add-User form which was left. As of now, i have done with polishing the PR. You can suggest me any further polishing if required. Thank you.