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.

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

[GT-183] Add service types page for non-admins #461

Merged

Conversation

Sae126V
Copy link
Contributor

@Sae126V Sae126V commented Jul 13, 2023

  • Admin => Can do CRUD operations in Service Types page.
  • non-Admin => Can ONLY read the data(view) in Service Types page.

"Resolves #126 "

Note:
Sometimes GIT interface will not show the EOL. If the file is NOT having EOL GIT may shows no_newline_at_end explicitly.

@Sae126V Sae126V requested a review from a team as a code owner July 13, 2023 11:41
@Sae126V Sae126V force-pushed the GT-183-Need-non-admin-Service-Types-views branch from fb9a3ce to c1e5ab8 Compare August 31, 2023 15:31
@Sae126V Sae126V marked this pull request as draft September 1, 2023 14:29
@Sae126V Sae126V force-pushed the GT-183-Need-non-admin-Service-Types-views branch from c1e5ab8 to d1e50cb Compare September 11, 2023 14:05
@Sae126V
Copy link
Contributor Author

Sae126V commented Sep 11, 2023

Ready for the review of Functionality. @gregcorbett

@gregcorbett
Copy link
Member

Looks mostly good - a couple of things I spotted were:

  • Page_Type=Service_Types doesn't display for non-registered users, so there will be an bug somewhere.
  • Move "ServiceTypes" to the browse section for admins, and position it between "Scopes" and "Role Action Map" for both admins and non admins.
  • Under GOCDB Admins, have an "Add Service Type" link leading to Page_Type=Admin_Add_Service_Type, and position it after "Add Project"

@Sae126V Sae126V force-pushed the GT-183-Need-non-admin-Service-Types-views branch from d1e50cb to 18d4192 Compare September 12, 2023 07:41
@Sae126V
Copy link
Contributor Author

Sae126V commented Sep 12, 2023

Looks mostly good - a couple of things I spotted were:

  • Page_Type=Service_Types doesn't display for non-registered users, so there will be an bug somewhere.
  • Move "ServiceTypes" to the browse section for admins, and position it between "Scopes" and "Role Action Map" for both admins and non admins.
  • Under GOCDB Admins, have an "Add Service Type" link leading to Page_Type=Admin_Add_Service_Type, and position it after "Add Project"

I have added a single page intentionally to avoid showing the same content in both, Greg. But with your suggestion the problem is not valid anymore. Updated the code to achieve the desired functionality.

@Sae126V Sae126V marked this pull request as ready for review September 12, 2023 07:45
@Sae126V Sae126V marked this pull request as draft September 12, 2023 07:45
@Sae126V Sae126V force-pushed the GT-183-Need-non-admin-Service-Types-views branch from 18d4192 to e1e1fe2 Compare September 12, 2023 07:49
@Sae126V
Copy link
Contributor Author

Sae126V commented Sep 12, 2023

Ready to be viewed for functionality.

@gregcorbett
Copy link
Member

  • Page_Type=Service_Types doesn't display for non-registered users, so there will be an bug somewhere.

ah the same thing happens with Page_Type=Service_Type, there's probably a similar bug.

Move "ServiceTypes" to the browse section for admins, and position it between "Scopes" and "Role Action Map" for both admins and non admins.

Ah sorry, I meant between "Services" and "Scopes".

@Sae126V Sae126V force-pushed the GT-183-Need-non-admin-Service-Types-views branch from e1e1fe2 to 1d43219 Compare September 12, 2023 08:10
@Sae126V
Copy link
Contributor Author

Sae126V commented Sep 12, 2023

  • Page_Type=Service_Types doesn't display for non-registered users, so there will be an bug somewhere.

ah the same thing happens with Page_Type=Service_Type, there's probably a similar bug.

Move "ServiceTypes" to the browse section for admins, and position it between "Scopes" and "Role Action Map" for both admins and non admins.

Ah sorry, I meant between "Services" and "Scopes".

Cool. Updated the flow, Greg. It should now work as expected.

@Sae126V Sae126V force-pushed the GT-183-Need-non-admin-Service-Types-views branch from 1d43219 to 66f0904 Compare September 12, 2023 08:26
@Sae126V Sae126V marked this pull request as ready for review September 12, 2023 09:22
@gregcorbett
Copy link
Member

A previous version I looked at had the "Add Service Type" functionality on Page_Type=Service_Types for admins, please add this back in.

@Sae126V
Copy link
Contributor Author

Sae126V commented Sep 12, 2023

A previous version I looked at had the "Add Service Type" functionality on Page_Type=Service_Types for admins, please add this back in.

The functionality got changed from previous version, Greg.

I have added the below one.
Under GOCDB Admins, have an "Add Service Type" link leading to Page_Type=Admin_Add_Service_Type, and position it after "Add Project"

Since, we are providing an admin option to directly goto the Admin_Add_Service_Type page Just like Add Projects, Add NGI. I have implemented the same for Add service Type.

And "Add Service Type" functionality on Page_Type=Service_Types is no longer needed for normal users or admin. Since admin has the page separately for adding service types. As it only admin rights. They can add service type from admin section. Does that make sense?

@gregcorbett
Copy link
Member

A previous version I looked at had the "Add Service Type" functionality on Page_Type=Service_Types for admins, please add this back in.

The functionality got changed from previous version, Greg.

I have added the below one. Under GOCDB Admins, have an "Add Service Type" link leading to Page_Type=Admin_Add_Service_Type, and position it after "Add Project"

Since, we are providing an admin option to directly goto the Admin_Add_Service_Type page Just like Add Projects, Add NGI. I have implemented the same for Add service Type.

Fair enough, that does give conistency with the Site, NGI etc pages.

Functionality looks good to me 😄

Copy link
Member

@gregcorbett gregcorbett left a comment

Choose a reason for hiding this comment

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

I've left some comments.

Can you move {controllers, views/view_service_type(s).php} out of the relevant admin folders and into new service_type folders?

config/web_portal/menu.xml Outdated Show resolved Hide resolved
htdocs/web_portal/controllers/utils.php Outdated Show resolved Hide resolved
htdocs/web_portal/views/admin/view_service_types.php Outdated Show resolved Hide resolved
htdocs/web_portal/controllers/admin/view_service_type.php Outdated Show resolved Hide resolved
@gregcorbett
Copy link
Member

Looks good to me, cna you squash the 4 commits into the first one please?

@Sae126V Sae126V force-pushed the GT-183-Need-non-admin-Service-Types-views branch from 0de0b6a to df52bdb Compare September 14, 2023 10:10
@Sae126V
Copy link
Contributor Author

Sae126V commented Sep 14, 2023

I squashed into one @gregcorbett . Their is styles PR for this one so can we park it for a while so that I can raise a styles PR ?

@gregcorbett
Copy link
Member

Let's get this merged as it's ready :)

@gregcorbett
Copy link
Member

rebasing on latest dev

@gregcorbett gregcorbett force-pushed the GT-183-Need-non-admin-Service-Types-views branch from df52bdb to d01f15e Compare September 14, 2023 13:13
@gregcorbett gregcorbett merged commit 7bc3b3f into GOCDB:dev Sep 14, 2023
6 of 12 checks passed
Sae126V added a commit to Sae126V/gocdb that referenced this pull request Sep 15, 2023
Sae126V added a commit to Sae126V/gocdb that referenced this pull request Sep 20, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Need non-admin Service Types views
2 participants