-
Notifications
You must be signed in to change notification settings - Fork 3
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
Flag to indicate a user is masquerading #20
Comments
I'll convert the patch to a PR for testing. |
@herbdool This works nicely in my testing and simplifies the code quite a bit. It no longer allows masquerading as an anonymous user, which was apparently a thing. |
I need to test the PR. It may remove the need to fix issue #11 although the README file will need to be updated. |
Unfortunately, the PR is not working for me. I initially applied the patch to module version 1.x-1.0.1 on an existing site. The patch applied without difficulty and the need for an update was identified and proceeded to remove the table. At first going the the user profile page a sidebar appeared with the masquerade block for two users. Configuring masquerade with no users identified for the block and a single user in the switch user field showed no users in the block on the admin user page. The module was disabled, uninstalled, flushed caches and re-enabled. From that point on the module does not work and nothing is visible on the admin user page. Similarly, using the backdropcms.org demo to create a fresh site and manually installing the patched version 1.x-1.0.1 of the module does not work with no block appearing on the admin user page. |
@izmeez For clarity, are you saying the issue doesn't work for the purposes I created it, or this PR doesn't work to solve your other (separate) issue in Here's what I did:
If you were checking to see if the PR fixed a separate issue, that's fine, but if it doesn't fix the other issue, let's not derail this issue. If this issue is causing a problem, please clarify how something that works without this PR no longer works after this PR. |
No, I was not expecting it to solve the other issue, only noting when this one is implemented it will have implications for the direction of the other issue. Thank you for identifying the steps you used. I will try those. As mentioned previously I patched the module locally and then used it only to find that even though the patch applied without difficulty it did not work at all for me. Your steps look like a promising way to test it. Thanks. |
Yes, it works. The problem I had was that you have to manually add the masquerade block somewhere. So yes, it works. I will now do more testing. |
Testing works. It looks to me like the module behavior is the same as 1.0.1 the only thing that has changed is it doesn't use a table instead using the session and provides a flag should your theme or maybe css injector wish to use it. Everything else is the same. A masquerade block must be placed in a layout visible to any role required. On local testing on active site the patched module did prompt for an update to remove the table and appears to have worked as required. Maybe a double flush of caches is required to avoid what I encountered. I didn't yet examine the session cookie. |
Testing the module.
The README states:
However, I'm not seeing this flag being set when testing. In investigating on the Drupal side, I found this RTBC patch which does a fairly major overhaul and simplification, including removing
$user->masquerading
and replacing with a simple$_SESSION
variable.Patch by
joelpittet
also attached here.masquerade-remove_masquerade_table_and_rely_on_session-d7-1926074-31.patch
The text was updated successfully, but these errors were encountered: