Hi,

I'm testing this module that seems to perfectly go for my needs. Thank you so much to the maintainer, the interface is well thinked.

There is just one things that I need and the module don't seems to provide. It's the possibility for user to apply to several roles at the same time.

With a little option in the administration page, you could check if you want to allow this possibility or not...

Another things, if this feature is planned, I think it would be better to provide radio or checkboxes instead of a list to choose roles.. (radio if multiple role disabled and checkboxes if enables).

What do you think about that ? Will do you plan it ? Do I have to hack the module core and submit patch ? If patch submitted, will you apply it for future version ?

In advance, thanks

zmove

CommentFileSizeAuthor
#2 apply_for_role.multi_.module.patch4.85 KBSoren Jones
#1 apply_for_role.patch23.45 KBzmove
#1 apply_for_role.zip10.34 KBzmove

Comments

zmove’s picture

zmove commented
Status: Active » Needs review
StatusFileSize
new apply_for_role.zip10.34 KB
new apply_for_role.patch23.45 KB

I made it, so I submit the patch.

I add a checkbox in admin menu to check if you want to let users to apply for several roles at the same time. If checked, it provide checkboxes under the apply for role tab in the user page instead of a list.

NB : I have difficulties to create a patch with winmerge, I posted it but I think it's not good, so I posted the entire modified module in a zip file.

Let's try and give me feedback.

Soren Jones’s picture

Soren Jones commented
StatusFileSize
new apply_for_role.multi_.module.patch4.85 KB

Zmove,

First, this sounds cool. And I'm sure your implementation is fine. But I can't test it until sometime Tuesday (in Japan/Monday in France). Maybe someone else has time now?

By the way, to create a sexier/usable patch in Windows you can use Cygwin. There's a great tutorial for setting it up at #32875.

After following the instructions at #32875.

1) Start Cygwin

2) Navigate to your apply_for_roles folder. (cd c:/[path to apply_for_role folder])

3) You have two copies of apply_for_role.module in your apply_for_role folder, right? For example, apply_for_role.module and apply_for_role.multi.module, where the first *.module is from the latest 5.x-1.x-dev and the second *.multi.module is your "patched" module.

4) Create a patch using the diff command. For example,

diff -up apply_for_role.module apply_for_role.multi.module > apply_for_role.multi.module.patch

(You can find more information on the Creating patches pages of the handbook.)

I usually create patches in Linux using cvs diff, but I followed the above instructions to create the attached patch and it seems all right.

Soren Jones’s picture

Soren Jones commented

I updated the instructions because I realized it's a bad idea to rename the original module. If a module is going to be renamed, it should be the patched module.

zmove’s picture

zmove commented

Hi,

No problem for testing it tuesday. I really pay attention to let admin choose the behavior they want by providing an option for multi application.

In addition, as the module was pretty well done, I didn't have to change a lot of code as you can see in the patch I just provided checkboxes instead o a list and made a loop to insert several apply if several checkboxes are checked. with this method, role apply moderator receive an application from visitor for each role they apply for, and so, can choose to accept one role, but not the other.

I corrected too a missing t() function when there are no role available and make a better custom message when a user apply for a role.

Soren Jones’s picture

Soren Jones commented

Hah. You're right. That wasn't a translatable string. Thank you! And I like the idea behind your more descriptive application notice/error strings. I wonder if that could be more compact in the multiple version. One message for all of the roles applied for rather than a seperate message for each.

Anyway...the way this stuff is usually done, at least as I understand it, is that changes are made in the most recent version (i.e., 6) and then backported.

So...what I would do on Tuesday is port your contribution to 6, test it (not that it needs it) and then (ehem) backport it to 5 (well, not really, but you get the idea).

However! If you have a copy of 6 running somewhere, you could port your addition to 6 and then make a sexy patch against the 6 -dev and post it here. What do you think? Just a thought.

zmove’s picture

zmove commented

Hi,

I just tested the 6 to see the change, but I didn't read how to make a module compatible with 6. I will try to read some documentation about it, I don't have a lot of time for that at the moment.

But I think I will work on another functionnality I would see in your module, it's to put the role selection form in a theme function. to be able to add some description, pictures etc... to the form, because a brut list of list or checkboxes could be not very easy to use for a common user.

zmove’s picture

zmove commented

Hi,

Just another little patch. I don't have installed Cygwin, I promise I will do it today. But it's just a very small patch that can easily be added with hands :

After the apply_for_role_apply_form_submit() function just add :

	function theme_apply_for_role_apply_form($form) {
		$output = drupal_render($form);
		return $output;
	}

It will allow designer to theme the apply for role form by redefining the theme function in their template.php file.

Very easy to implement and very usefull.

Soren Jones’s picture

Soren Jones commented
Status: Needs review » Needs work

Changing status. There's no patch attached yet that's ready to test.

Soren Jones’s picture

Soren Jones commented

This has been implemented in the 5 and 6 -devs.

Thanks to zmove for the code that got the ball rolling.

Soren Jones’s picture

Soren Jones commented
Assigned: Unassigned » Soren Jones
Status: Needs work » Fixed

Fixed in -dev. The other feature requests and bug reports by zmove in this thread (missing t(), more descriptive notice on role application, and a theme function) have also been implemented.

Anonymous’s picture

Anonymous (not verified) commented
Status: Fixed » Closed (fixed)

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

fehin’s picture

fehin commented

Hi Zmove, you said "It will allow designer to theme the apply for role form by redefining the theme function in their template.php file."
Would you please show me what you added to your template.php? Thanks.