PKP Bugzilla – Bug 5321
store "actingAsUserGroupId" in session table
Last modified: 2010-11-02 14:33:45 PDT
We are moving to Git Issues for bug tracking in future releases. During transition, content will be in both tools. If you'd like to file a new bug, please create an issue.
We need to start storing the userGroupId in the sessions table. For starters, it needs to be added to a monograph when it is first created.
It will likely be used in other places as well. For now, I think it needs to be set by changing the drop down in the "Roles" toolbox item.
Matt: can you have a look at adding this? I'll be committing the change to the Monograph and MonographDAO class, but wont get a chance to the other changes.
Florian: adding you to the CC because this will likely fall in your jurisdiction
I just pushed a preliminary version of the "acting as user group" change to omp and pkp-lib:
A few minor things are missing (translations etc., see list below) because I thought you should better look over the chosen solution to see whether it has any problems.
Of course it was not a 5-minute change. There was a bug in the block.tpl which cost me some time (the iterate tag had from=$... rather than from="..." which wasn't really easy to debug. ;-) ) After that I spent most of the time figuring the right approach:
- Your initial implementation always redirected to the user home page. IIRC the user group should be changeable without leaving the page.
- I first implemented a normal post directly and had the "user group" stuff in the PageRouter. That would mean that users could loose form data, though. And it's also much slower than a background AJAX request.
- I then came up with the "headless AJAX component" approach that I liked most, so far. It's fast and looks like a usable solution to me. We might have to implement a client-side event though that client-side methods can subscribe to when they depend on the "acting-as-user-group". We may have to refresh some grids for example so that their interface changes accordingly.
Do you see any problems with the approach I came up with? If not then I'll implement the remaining todos which are (AFAICS):
- set the currently selected user group in the form.
- translate localization strings
- block interface until AJAX request returns (show a throbber)
- implement client-side refresh event
It would be great if I could test that approach with a real page then. I could take one of the workflow stage pages (if ready) and see whether it works there implementing the full validation for one of them...
- use the notification framework to give visual feedback
- persist session data (use dedicated column in the session table)
Will be exemplified for the submission process.
Implemented todo "persist session data", see http://github.com/pkp/pkp-lib/commit/1a8cb1c64f1dc5dd2e8b8c56508eecc6a636a359
A small bug fix: http://github.com/pkp/pkp-lib/commit/7dbeb740b6d59bf8d70cf0933ec157da8839e78f
first use of this in real code.
Correction to how the session variable is fetched in a few places.
Another todo: have a look at the Role policy handler and see how we can replace the 'manager' path mentioned in there.
minor change to sidebar plugin to show selected role in sidebar.
Juan, I reopen this bug for myself because I've got a few todos in here which have not been implemented yet, see the comment history.
I didn't mean to close it. Just wanted to add the comment. Must have selected the wrong action below.
select correct group in role selection plug-in: http://github.com/pkp/omp/commit/466867870ffb1f8da0c27d184bcb0ece817993b7
Only display groups that are available in the current context: http://github.com/pkp/omp/commit/e0b5b7c96b6a95315bdb0bc98e95861538199bb3
I've set the block plugin to reload the page after a role is switched: http://github.com/pkp/omp/commit/26a3d585f97bbf3f973593ecf799168c70d9507c
Matt, reloading the page is not a solution as this will destroy form data (or any other unsaved client side state) which we cannot tolerate. We already have thought about a solution for this problem (see the comments in this bug report). I've just been waiting for a use case to exemplify this on.
Can you please revert your change and tell me which page you have trouble with so that I can use that one as a test case.
Sorry. This is my bad. I told Matt to go ahead with that. The problem is that we sometimes load a page with a grid we don't have access to an so we have to change the actingas and reload the page.
Question: Why is it possible that a user sees functionality that isn't available in his/her group? That seems to contradict specifications. IIRC the permissions spec says that changing the user group should disable functionality the user doesn't have access to (including links to pages or grids the user then cannot access anymore of course). I'm so sure about this because I found this user-group stuff rather strange and asked twice about that point. This means that the case you're describing shouldn't exist (if I understand you correctly).
Disabling and enabling functionality (including grids) would be done via the client side change event we talked about.
Reversed commit: http://github.com/pkp/omp/commit/163152c7edac2e81fcf90dece807154ba08aafac
I see now why reloading the page will not always be sufficient here, but in some cases (e.g. when the user is in the dashboard), changing roles should refresh content. This will need more discussion.
I agree. Refreshing is an important requirement for this change. My original idea was that we implement a custom JS event that signals a change of the role on the client side. Any client side component (including the page as a whole) that needs to change can subscribe to that event and either refresh itself locally or from the server so that the user doesn't loose work. This enables all kinds of refresh and gives us the best from both worlds (local + server refresh).
Would that be a solution that solves the problems you have in mind?
If you tell me what page you're working on (URL) I could use that as a test case for the client event implementation. I think it's not difficult to do. I just was waiting for some real use case to test that.
This bug probably needs a new name. Florian: with your new experience with custom JS events, you still think this will be possible?
We are ready to write the pages to test this out. I think the grids that are currently loading at /editor /author, for example, should be loading at /submissions (or arguably at /submission/list) . the grid that loads should change depending on the actingAsUserGroupId.
also, may need to change this bug name.
Yes, I think this can be implemented as intended. I know how to work around these bugs now. Nothing insurmountable there. ;-) This is one of the bugs high up on my list now.
I'll open up a new bug report for the submission list change itself and will only depend on this report so that we don't mix up the different topics.
I've just committed the change set that introduces a client-side custom event which client-side components can subscribe to to update the page when the user group (or role) changes. Please have a look at /templates/submission/index.tpl for an example of how this works in practice, see http://github.com/pkp/omp/commit/0df3d3cd04d85bb211460617507d33cc4832f8fe