Sandbox Home: http://drupal.org/sandbox/htumanyan/1867634

This Drupal 7 module compliments more popular and better known Login Toboggan and Login/Logout Destinations. It performs similar functions, but is specifically targeted at sites, running Spaces and Organic Groups.

The typical use of Spaces and Organic Groups is to segregate sections of the site and associate users with these sections. It is not unusual to use Spaces and Organic Groups to create an appearance of multiple, independently running sites on top of single Drupal deployment. In doing so, site owners often encounter a problem, related to user's login/logout experience: the standard Drupal implementation takes users to user's profile page upon login and takes to the home page of the overall site upon logout. This module provides a feature, which, if enabled within given space and organic group, alters the flow as follows:

a. The user visited a page within the organic group and he/she is not logged in, while the page requires login. After successful login the user is taken to the home page for the space/group.

b. The user is logged in and clicks the logout link on a page within the organic group - after the logout she or he will be taken to the home page for that organic group.

The net effect of these two flows is that the user is kept within the organic group throughout his/her experience and never taken to other organic groups or overall site home, unless there are links, specifically pointing there.

It is in the plans to add a configuration option that would allow enabling or disabling the login and logout parts of the functionality separately. At present it is all or nothing.

Reviews of other projects:

Comments

klausi’s picture

klausi
he/him||they/them
Primary language German
Location 🇦🇹 Vienna
commented
Status: Needs review » Needs work

Welcome,

please get a review bonus first. Then try to fix issues raised by automated review tools: http://ventral.org/pareview/httpgitdrupalorgsandboxhtumanyan1867634git

htumanyan’s picture

htumanyan commented

I've addressed all issues, detected by the automated review tool and re-run it to ensure no new issues were added. The report comes out clean now.

It does state an issue with that the master branch still exists, however, git branch -D master reports that the "master" branch is not found.

htumanyan’s picture

htumanyan commented
Status: Needs work » Needs review
fr3shw3b’s picture

fr3shw3b commented
Status: Needs review » Needs work

Hello htumanyan,

Great Idea for a module,

It works for me as expected.

Automated review:

http://ventral.org/pareview/httpgitdrupalorgsandboxhtumanyan1867634git

Change master branch to version specific branch, if you're not sure how to do this then go here...
http://drupal.org/empty-git-master

Manual review:

- Firstly does the name of the module need to have three underscores between login and logout?

It's not a big issue but it looked a bit odd to me, if there is a reason please tell me.

- Secondly within the .info file for good practice it's probably worth changing the order so it will start with name and description.

http://drupal.org/node/542202

- Thirdly you need a hook_uninstall to remove the custom variable when the module is uninstalled. To do this create a spaces_og_login___logout.install file and in that add the hook_uninstall function and in that use variable_del() to remove your custom variable on uninstall.

htumanyan’s picture

htumanyan commented

fr3shw3b,
Thank you for review and feedback!

>Change master branch to version specific branch, if you're not sure how to do this then go here...
Fixed

>Firstly does the name of the module need to have three underscores between login and logout?
I did not pick that name :) Drupal.org generated it from the module name, I've entered. For "Spaces OG Login/Logout" it generated the name with three dashes. I'm not sure if I have any control over that, do you know if there is a way to change the module name?

>Secondly within the .info file for good practice it's probably worth changing the order so it will start with name and description.
Done and checked in. Thanks for the suggestion.

>Thirdly you need a hook_uninstall to remove the custom variable when the module is uninstalled. To do this create a >spaces_og_login___logout.install file and in that add the hook_uninstall function and in that use variable_del() to remove >your custom variable on uninstall.
I did, I started writing the uninstall file and it occurred to me.... that variables, created with this module are stored in the organic group node rather than in the settings table. That has a few consequences:
a. In order to read/write or delete the variable, the code needs to be executed in the context of the space, otherwise, variable_get, variable_delete etc. won't have any effect. It does not seem to be possible to manipulate space specific variables from the uninstall hook.

b. Variable removal upon uninstall, even if it was possible, would, essentially, modify content nodes. I'm not sure if that would be intuitive or expected... It is also not clear which nodes would need to be stripped of variables, introduced by this module. Stripping from all nodes is one possibility, but again, it does not seem to be possible (I couldn't find any good way) without direct access to spaces_overrides table, which seems to be a questionable idea.

Do you have any thoughts on these points? Tips or suggestions? I'd gladly take any.

Thank you very much,
Hovhannes

htumanyan’s picture

htumanyan commented
Status: Needs work » Needs review
fr3shw3b’s picture

fr3shw3b commented
Status: Needs review » Reviewed & tested by the community

Hello htumanyan,

I did not pick that name :) Drupal.org generated it from the module name, I've entered. For "Spaces OG Login/Logout" it generated the name with three dashes. I'm not sure if I have any control over that, do you know if there is a way to change the module name?

Yes there is a way to change the module name just rename the folder and the files to whatever you think best fits such as spaces_og_login_logout.

a. In order to read/write or delete the variable, the code needs to be executed in the context of the space, otherwise, variable_get, variable_delete etc. won't have any effect. It does not seem to be possible to manipulate space specific variables from the uninstall hook.

b. Variable removal upon uninstall, even if it was possible, would, essentially, modify content nodes. I'm not sure if that would be intuitive or expected... It is also not clear which nodes would need to be stripped of variables, introduced by this module. Stripping from all nodes is one possibility, but again, it does not seem to be possible (I couldn't find any good way) without direct access to spaces_overrides table, which seems to be a questionable idea.

So spaces removes these variables on uninstall? If that's the case then they aren't module specific variables alone so it doesn't seem such a problem, somebody might have better suggestions but I can't think of any.

You might want to do review bonus to get greater insight from others but as far as I am aware of it all seems to be working right and I can't find any major problems, so it's RTBC from my point of view.

jthorson’s picture

jthorson commented

Comments:

Filename
I'll echo the previous comment, to rename the files and remove the triple underscore.
.module file
Line 4: Typo in 'override'
Line 33: The title value of "Settings" here resulted in two "Settings" menu items in my menu, conflicting with spaces_dashboard. I'd recommend a more specific title.

No showstoppers, so ...

Thanks for your contribution, htumanyan!

I updated your account to let you promote this to a full project and also create new projects as either a sandbox or a "full" project.

Here are some recommended readings to help with excellent maintainership:

You can find lots more contributors chatting on IRC in #drupal-contribute. So, come hang out and get involved!

Thanks, also, for your patience with the review process. Anyone is welcome to participate in the review process. Please consider reviewing other projects that are pending review. I encourage you to learn more about that process and join the group of reviewers.

Thanks to the dedicated reviewer(s) as well.

klausi’s picture

klausi
he/him||they/them
Primary language German
Location 🇦🇹 Vienna
commented
Status: Reviewed & tested by the community » Fixed

Status: Fixed » Closed (fixed)

Automatically closed -- issue fixed for 2 weeks with no activity.

Anonymous’s picture

(not verified) commented
Issue summary: View changes

Added a link to review