Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

GSOC-23: Roles and User Modules #43

Open
wants to merge 43 commits into
base: develop
Choose a base branch
from
Open

GSOC-23: Roles and User Modules #43

wants to merge 43 commits into from

Conversation

chaitak-gorai
Copy link
Contributor

Description

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

Workflow

  • Create Roles Modules with Access Controls with permissions.
    • Identify ACL permissions from legacy codebase.
    • Create permissions
    • Create Role with those permissions
  • Create User Module
  • Integrate Users and Roles Module

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

@chaitak-gorai
Copy link
Contributor Author

chaitak-gorai commented Jun 9, 2023 •
edited
Loading

Added Roles UI with functionalities of creating and fetching roles

  • Created RolesController for the backend logic
  • Created the UI for the Roles Page with Add Role Form and Existing Roles Section in Roles.vue
  • Added the routes in web.php

Screenshot

image

muarachmann
muarachmann reviewed Jun 10, 2023
View reviewed changes
Copy link
Member

@muarachmann muarachmann left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@chaitak-gorai left some reviews

app/Http/Controllers/Dashboard/RolesController.php Outdated Show resolved Hide resolved
resources/app/pages/Roles.vue Outdated Show resolved Hide resolved
routes/web.php Outdated Show resolved Hide resolved
@chaitak-gorai
Copy link
Contributor Author

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 :

  • Super Admin CRUD for all modules
  • Admin is the same as super admin except for no update and deletes in administration
  • lr-user: read for all
  • accounting: fees and reports CRUD
  • clinicians: CRUD for patient-related modules else RU
  • Emergency login: Read for all and CRUD for the patient(keeping in mind the emergency_login will be done by a patient_
  • front office: generally RU
  • physicians: generally CRUD except for the administration
  • supper student: RU

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

muarachmann
muarachmann reviewed Jun 19, 2023
View reviewed changes
app/Http/Controllers/Dashboard/RolesController.php Outdated Show resolved Hide resolved
config/laratrust_seeder.php Outdated Show resolved Hide resolved
@muarachmann
Copy link
Member

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

@muarachmann muarachmann added module:-users Users Module module:-roles Roles Module labels Jun 20, 2023
@chaitak-gorai
Copy link
Contributor Author

Update:

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

  • Help Required Removing old roles completely giving too many redirects. I can't find anywhere the old roles are used. If there is, can you please help me out where these are used?
  • Temporarily, I have solved it by keeping both new and old roles. Commented the truncate function in LaratrustSeeder.php
  • The Existing Roles section is too long so applied scroll there.
  • Also made the dropdown for the permissions scrollable too to fit the large list.

Screenshot of update Roles.vue UI
image

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

muarachmann
muarachmann requested changes Jul 2, 2023
View reviewed changes
app/Http/Controllers/Dashboard/RolesController.php Outdated Show resolved Hide resolved
resources/app/pages/Roles.vue Outdated Show resolved Hide resolved
app/Http/Controllers/Dashboard/RolesController.php Show resolved Hide resolved
@chaitak-gorai
Copy link
Contributor Author

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
Copy link

tmccormi commented Jul 8, 2023 via email

@tmccormi
Copy link

tmccormi commented Jul 8, 2023 via email

@chaitak-gorai
Copy link
Contributor Author

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
Copy link

tmccormi commented Jul 8, 2023 via email

@chaitak-gorai
Copy link
Contributor Author

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

@chaitak-gorai
Copy link
Contributor Author

Update:

  • Used the resourse routes instead to custom routes for users related functions.
    • Remove the custom routes and change the named routes everywhere it is used.
    • Moved the user addition logic from Invitations controller inside store functions of user controller.
    • Removed redundant Setup Account controller. It can be done inside create of User Controller.
  • Added Breadcrumbs in all User Pages Users/Invitations->Full Name->Edit or Users/Invitations->Add
  • Used Inertia link as suggested in review.
  • Rename file names with clean practices
    • AddUsers.vue into Users/Add.vue
    • EditUsers.vue into Users/Edit.vue
    • UsersProfile.vue into Users/Profile.vue

@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
Copy link
Contributor Author

@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
Copy link
Member

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

@chaitak-gorai
Copy link
Contributor Author

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
Copy link
Member

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

@chaitak-gorai
Copy link
Contributor Author

Update

@muarachmann i have added these changes

  • Rebase header-fix branch for the updated menu link.s
  • Added Users Datatable
  • Added links to the pages in Administration menu link for Invitations, Users and ACL for roles.
  • modified the routes for roles and permission-based accessing. As discussed in the call, we don't have to create separate middleware for that. I used the default middleware of Laratrust where we can use both permission and role-based access for the routes.

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
Copy link
Member

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

@chaitak-gorai
Copy link
Contributor Author

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
Copy link
Contributor Author

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.

@chaitak-gorai
Copy link
Contributor Author

Hey @muarachmann hope you are doing fine.
I just want to remind you when it will be merged? Is there any changes i need to commit?
Thank you.

@muarachmann
Copy link
Member

Hey @muarachmann hope you are doing fine. I just want to remind you when it will be merged? Is there any changes i need to commit? Thank you.

Hi @chaitak-gorai I have been really busy lately. Will review this during the weekend and merge

@chaitak-gorai
Copy link
Contributor Author

Hey @muarachmann its ok if you are busy. Just a reminder that if you are available this weekend can we discuss on this.
Also, can we also discuss about the other modules which we can build in upcoming session.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@muarachmann muarachmann muarachmann requested changes

Assignees
No one assigned
Labels
gsoc module:-roles Roles Module module:-users Users Module project Project
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@chaitak-gorai @muarachmann @tmccormi