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

Add non-admin view for scopes #281

Merged
merged 6 commits into from
Feb 15, 2022
Merged

Conversation

ElliottKasoar
Copy link
Contributor

@ElliottKasoar ElliottKasoar commented May 11, 2021

Resolves #273

As suggested in #273, Page_Type=Scope_Help and Page_Type=Admin_Scopes have been combined into a single page, Page_Type=Scopes, with admin controls inaccessible to non-admin users.

Similarly, the Page_Type=Admin_Scope page has been replaced with Page_Type=Scope, which displays the same information, except the admin controls are inaccessible to non-admin users.

Two new controllers, scopes.php and scope.php have also been added, based on the equivalent admin controllers. Their functionality is largely the same, except they pass whether the user is an admin to the views, rather than throwing an exception for non-admin users.

The scopes.php controller also passes lists of reserved and optional scopes, to allow Page_Type=Scopes to display this information in a similar manner to Page_Type=Scope_Help.

The Scope_Help and Admin_Scope views/controllers have been removed. The updated scripts may need their license information etc. updating further?

@gregcorbett gregcorbett requested a review from a team May 14, 2021 14:15
@gregcorbett gregcorbett self-assigned this May 14, 2021
@gregcorbett gregcorbett added this to the 5.9.0 milestone May 14, 2021
Copy link
Contributor

@GRyall GRyall left a comment

Choose a reason for hiding this comment

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

All of those commits need squashing down into self contained commits with commit messages that can be understood without the context of the PR.

GRyall
GRyall previously requested changes May 17, 2021
Copy link
Contributor

@GRyall GRyall left a comment

Choose a reason for hiding this comment

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

Could you also confirm you have searched the codebase for instances of Admin_scope and scopes_help?

htdocs/web_portal/index.php Outdated Show resolved Hide resolved
@ElliottKasoar
Copy link
Contributor Author

All of those commits need squashing down into self contained commits with commit messages that can be understood without the context of the PR.

Is it ok now?

Could you also confirm you have searched the codebase for instances of Admin_scope and scopes_help?

Yes I have. There were still some references in the unused/undeleted scripts, but I can't see any now I've deleted them too.

@GRyall
Copy link
Contributor

GRyall commented May 19, 2021

All of those commits need squashing down into self contained commits with commit messages that can be understood without the context of the PR.

Is it ok now?

Yep!

Copy link
Contributor

@GRyall GRyall left a comment

Choose a reason for hiding this comment

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

I don't see any issues now, but I don't currently have a set-up to test the changes against - so I will leave the final approval to someone who does.

ghost
ghost previously approved these changes Jul 5, 2021
htdocs/web_portal/controllers/scopes.php Outdated Show resolved Hide resolved
htdocs/web_portal/controllers/scopes.php Outdated Show resolved Hide resolved
htdocs/web_portal/views/scopes.php Outdated Show resolved Hide resolved
htdocs/web_portal/views/scopes.php Outdated Show resolved Hide resolved
ghost
ghost previously approved these changes Jul 19, 2021
Copy link

@ghost ghost left a comment

Choose a reason for hiding this comment

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

@ElliottKasoar
Copy link
Contributor Author

(Squashed requested change into last commit)

ghost
ghost previously approved these changes Jul 19, 2021
@ElliottKasoar
Copy link
Contributor Author

Might it be worth hiding some of the information behind a help button, similar to the Downtimes Calendar page?

@gregcorbett
Copy link
Member

Might it be worth hiding some of the information behind a help button, similar to the Downtimes Calendar page?

What information did you have in mind?

@ElliottKasoar
Copy link
Contributor Author

ElliottKasoar commented Aug 2, 2021

What information did you have in mind?

I was thinking potentially some/all of the sections from the Scope_Help page that weren't on the Admin_Scopes page: What are scope tags?, What are Reserved tags?, How are scope tags used in the API? and What does having the EGI scope applied mean?.

I would imagine most people would visit the page for the list of scopes itself, similar to most of the other pages, which was partly why my combined page has the table first. However, the explanatory information could end up quite far down the page, particularly as the number of scopes increases, which isn't necessarily as convenient as having it hidden but displayable at the top. I'm not sure though, it was just an idea.

@gregcorbett
Copy link
Member

That sounds like a good improvement to make 😄

@ElliottKasoar
Copy link
Contributor Author

I would have preferred the help button to be styled more consistently with the downtimes calendar help button, but it didn't seem to fit as well, particularly with the Add Scope button, although most people won't see that. I can change it back if it people think it would look better.

@gregcorbett
Copy link
Member

perhaps we could have some kind of Collapsible if it could be style to fit in with the rest of GOCDB?

@ghost
Copy link

ghost commented Aug 12, 2021

The Calendar page 'header' anyway seems inconsistent with other pages in missing a one-line description. Perhaps such a line with a collapsible 'Show more ...', or similar, would work consistently for both cases. The help and question-mark does look out of place IMHO.

@ElliottKasoar
Copy link
Contributor Author

ElliottKasoar commented Aug 12, 2021

perhaps we could have some kind of Collapsible if it could be style to fit in with the rest of GOCDB?

Hmm it feels like a collapsible might be a little intrusive, but perhaps it is still the best solution.

The Calendar page 'header' anyway seems inconsistent with other pages in missing a one-line description. Perhaps such a line with a collapsible 'Show more ...', or similar, would work consistently for both cases. The help and question-mark does look out of place IMHO.

To clarify, are you suggesting a collapsible in the same place as other one-lines, or add a similar one-line as well as a collapsible below (e.g. similar to the Projects and NGIs pages, but a collapsible rather than a "What is a..." link?)

In terms of general consistency, the Scopes page is also lacking its own icon.

@ghost
Copy link

ghost commented Aug 12, 2021

To clarify, are you suggesting a collapsible in the same place as other one-lines, or add a similar one-line as well as a collapsible below (e.g. similar to the Projects and NGIs pages, but a collapsible rather than a "What is a..." link?)

The latter: one liner plus collapsible.

In terms of general consistency, the Scopes page is also lacking own icon.

A nice hackathon/first change issue for somebody then. 😃

@ElliottKasoar
Copy link
Contributor Author

ElliottKasoar commented Aug 24, 2021

As discussed with @ineilson and @gregcorbett, toggling to show/hide the information is now done via a link, the text of which changes to provide the correct context.

Overall the page is also now more consistent with others (e.g. NGIs), apart from the lack of an image.

ghost
ghost previously approved these changes Sep 6, 2021
The scope/scopes views and controllers have been moved out of their respective admin directories, as they are forming the basis for the non-admin scripts. Rather than throwing an error if the user is not an admin, the controllers instead pass a flag to the views, hiding admin controls, although the controls would not work for a non-admin user anyway. References to Admin_Scopes pages have been updated to Scopes, and the page has been added to the menu for non-admin users. The Scope_Help link has also been renamed to more clearly differentiate it, as previously the Admin_Scopes and Scope_Help pages were both labelled "Scopes".
Add information from Scope Help page to Scopes, including help information, scope descriptions and whether scopes are reserved.
Deleted Scope Help view, controller and menu link, as all information is now in Scopes page. References to the page have been updated to instead reference Scopes.
Replaced sizeof() with count(), and corrected use of numberOfScopes in if statements.
Similar to the downtimes calendar view, this hides the scope help information by default, but allows it to be viewed by clicking a link. This means the list of scopes is displayed first, but it is not necessary to scroll past the entire table to see the help information. The link text also changes on clicking.
@gregcorbett
Copy link
Member

rebasing on the latest dev

@gregcorbett gregcorbett merged commit 291308f into GOCDB:dev Feb 15, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Need non-admin view of an individual scope
4 participants