Bug 5321 - store "actingAsUserGroupId" in session table
store "actingAsUserGroupId" in session table
Status: RESOLVED FIXED
Product: OMP
Classification: Unclassified
Component: General
1.0b
All All
: P3 normal
Assigned To: jerico
Depends on: 5807
Blocks:
  Show dependency treegraph
 
Reported: 2010-04-16 18:20 PDT by Juan Pablo Alperin
Modified: 2010-11-02 14:33 PDT (History)
2 users (show)

See Also:
Version Reported In:
Also Affects:


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description Juan Pablo Alperin 2010-04-16 18:20:47 PDT
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.
Comment 1 Juan Pablo Alperin 2010-05-09 18:36:43 PDT
Florian: adding you to the CC because this will likely fall in your jurisdiction
Comment 2 jerico 2010-05-18 09:30:17 PDT
Hi Juan,

I just pushed a preliminary version of the "acting as user group" change to omp and pkp-lib:
http://github.com/pkp/pkp-lib/commit/29c38c3d27742bf88df9fa5069e746bafa2c77ee
http://github.com/pkp/omp/commit/aaeca49d3907b4ab0bd089b797f9f8f4a22c3f15

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...

Kind regards,
Florian
Comment 3 jerico 2010-05-19 15:47:15 PDT
Other todos:
- 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.
Comment 4 jerico 2010-05-20 07:47:52 PDT
Implemented todo "persist session data", see http://github.com/pkp/pkp-lib/commit/1a8cb1c64f1dc5dd2e8b8c56508eecc6a636a359
Comment 6 Juan Pablo Alperin 2010-05-21 10:53:26 PDT
first use of this in real code.

http://github.com/pkp/omp/commit/75627be07d23dbac6e087e30f991039e5c717102
Comment 7 Juan Pablo Alperin 2010-07-02 15:08:36 PDT
Correction to how the session variable is fetched in a few places.

http://github.com/pkp/omp/commit/81a049fd035fea81e4ac88a3106247247b9d4f2a
Comment 8 jerico 2010-07-07 10:38:15 PDT
Another todo: have a look at the Role policy handler and see how we can replace the 'manager' path mentioned in there.
Comment 9 Juan Pablo Alperin 2010-07-09 10:24:52 PDT
minor change to sidebar plugin to show selected role in sidebar.

http://github.com/pkp/omp/commit/97abe6d09b7559e0d8c3281da9a59ff48c97d231
Comment 10 jerico 2010-07-09 10:29:25 PDT
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.
Comment 11 Juan Pablo Alperin 2010-07-09 13:12:51 PDT
I didn't mean to close it.  Just wanted to add the comment.  Must have selected the wrong action below.
Comment 12 jerico 2010-07-10 00:26:12 PDT
select correct group in role selection plug-in: http://github.com/pkp/omp/commit/466867870ffb1f8da0c27d184bcb0ece817993b7
Comment 13 jerico 2010-07-12 12:43:11 PDT
Only display groups that are available in the current context: http://github.com/pkp/omp/commit/e0b5b7c96b6a95315bdb0bc98e95861538199bb3
Comment 14 Matthew Crider 2010-07-23 16:55:50 PDT
I've set the block plugin to reload the page after a role is switched: http://github.com/pkp/omp/commit/26a3d585f97bbf3f973593ecf799168c70d9507c
Comment 15 jerico 2010-07-23 17:10:09 PDT
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.
Comment 16 Juan Pablo Alperin 2010-07-23 17:32:10 PDT
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.
Comment 17 jerico 2010-07-23 17:59:17 PDT
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.
Comment 18 Matthew Crider 2010-07-26 10:42:06 PDT
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.
Comment 19 jerico 2010-07-26 12:22:00 PDT
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.
Comment 20 Juan Pablo Alperin 2010-08-24 18:48:08 PDT
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.
Comment 21 jerico 2010-08-24 19:02:07 PDT
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.
Comment 22 jerico 2010-08-24 20:21:51 PDT
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.
Comment 23 jerico 2010-09-01 07:48:37 PDT
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