I fixed several issues in the module and now it should work. Feedback is welcome.
My patch includes http://drupal.org/node/1037428#comment-4286166

CommentFileSizeAuthor
#9 user_restrictions_d7fixes-1214080.patch12.55 KBpfaocle
#4 user_restrictions.patch12.73 KBmarco
user_restrictions.patch12.63 KBmarco

Comments

BenK’s picture

BenK commented

Subscribing

garypdb’s picture

garypdb commented

Subscribing.

I had to erase the second line on each patched file if I wanted it to work

zeemp’s picture

zeemp commented

The patch is working great, but after I install it, the login at the site is only working for user #1. Any clue?

Update: This is happening in the module without the patch too.

marco’s picture

marco commented
StatusFileSize
new user_restrictions.patch12.73 KB

I should have fixed the bug reported by zeemp, and another bug which was related.
The attached patch shouldn't have the problem reported by garypdb.
Thank you for the feedback.

Sparhawk104’s picture

Sparhawk104 commented

I'd love to see this module in working order, but I'm fairly sure I can't use patches. I just hope these fixes will work their way into the module proper soon! Keep up the great work! :)

attiks’s picture

attiks commented

sub and thanks

attiks’s picture

attiks commented
Status: Active » Reviewed & tested by the community
pfaocle’s picture

pfaocle
He/Him
Primary language English
commented
Status: Reviewed & tested by the community » Needs work

Patch doesn't apply.

pfaocle’s picture

pfaocle
He/Him
Primary language English
commented
Category: task » bug
Status: Needs work » Needs review
StatusFileSize
new user_restrictions_d7fixes-1214080.patch12.55 KB

However, applying the patch manually seems to bring the module up to scratch. We have it running on a development site at present.

Patch attached adds one more fix to user_restrictions_ui.module - a correction to the permission name.

snufkin’s picture

snufkin commented
Status: Needs review » Needs work

Sorry to be a pain, but could we break up this patch to resolve individual issues? I can see several unrelated changes and it would be much more helpful to have separate commit for separate bugs - that way we can roll back or identify issues with each change separately as opposed to debugging a single larger change.

fizk’s picture

fizk commented
Status: Needs work » Reviewed & tested by the community

snufkin, since the patch works and fixes critical issues that can break the entire site (#1223924: Users can't login after the installation of User restrictions module), and that this issue is 6 months old, I'll go ahead and commit it in separate commits so that we can identify issues with each change.

fizk’s picture

fizk commented
Assigned: marco » fizk
Anonymous’s picture

Anonymous (not verified) commented
Version: 7.x-1.0-beta7 » 7.x-1.x-dev
Status: Reviewed & tested by the community » Needs work

The code I am developing is different from the one contained in the beta 7 release. Some of the code still needs to be committed. it is better to way to create any patches until I have committed all the code to the development snapshot, which is the release against which the patch are created.

snufkin’s picture

snufkin commented

fizk: I understand that you are eager to fix the module, but in general its a much better practice to discuss and submit the patches for public review in individual changs. I understand that the patch fixes critical issues _also_, but on top of that it contains changes not related to this. I hope kiam releases his updated dev code soon, and then we can close this issue and answer relevant bugs in their own issues.

fizk’s picture

fizk commented

kiam, thanks for the update, but why are you developing the code in secret? You should be committing developmental work to git on a regular basis so everyone knows what's gong on.

fizk’s picture

fizk commented

snufkin, you keep mentioning that there's non-related changes in the patch without going into detail. Exactly which changes are non-related?

snufkin’s picture

snufkin commented

for instance the string rewrites. based on that - should i really waste hours to figure out what else changes what as well? there is no actual bugreport in this issue. this patch fixes at least two issues that you marked duplicate elsewhere. which change fixes which? I wouldnt know, but the author of the patch would - but instead of splitting up the patch there is one patch for all of those reported issues, and some other style fixes he found along the way.

I am not saying these fixes are irrelevant. they are necessary, but this is the process how maintainers can also see what change fixes what issue. note - i am not a maintainer of this project, i have commit access because of historical reasons. but i maintain a few projects on d.o and i know what helps the maintainers the most. and merging a lot of fixes into one patch does not.

fizk’s picture

fizk commented

snufkin, generally I agree with you - I would always prefer separate bugs fixed in their own individual patches, but I've gone over this patch so many times that it looks very simple and straight forward.

I'll give this another week to settle, if nothing changes, I'll commit it. After that we can look into implementing some interesting feature requests into this module, like #1270152: Add rules support for User Restrictions.

Cheers,
Yonas

Anonymous’s picture

Anonymous (not verified) commented

I agree with snufkin: Merging many issue reports in one that needs a patch is messy, and not self documenting. It's quite easier to leave issues separated, and create a patch for each issue. When I create a new release, the drush extension that creates the release log will automatically link to the related issue for each commit message. Imagine how useful can be a link to an issue report whose summary is just:

I fixed several issues in the module and now it should work. Feedback is welcome.
My patch includes http://drupal.org/node/1037428#comment-4286166

I am not writing code in secret; the fact is that I have been away from home from some months, and when I returned I had to fix some issues with my computer, retrieve the code I developed, and see if it were update, check the code in the repositories, and see if it were updated. I also had other personal issues I had to solve.

My goal is to make the module as more generic as possible, and allow third-party to add rules to the ones already implemented from the module, including the possibility of using temporary rules that don't have any effect after X days/months.

bachomp’s picture

bachomp commented

How is the commit coming along fizk?

fizk’s picture

fizk commented
Status: Needs work » Reviewed & tested by the community

bachomp, I guess it's been over a month now with no activity in git or this thread, so I'll commit this patch as soon as possible. Thanks for reminding me ;-)

fizk’s picture

fizk commented

bachomp, can you confirm that the patch works for you as well?

bachomp’s picture

bachomp commented

will do, report back tomorrow.

adammalone’s picture

adammalone
Primary language English
Location San Francisco
commented

It's been over a year since this broken module was put on drupal.org. There is a patch available that apparently fixes it, why has this patch not been committed yet?

It should also probably be noted on the module page until then that this module is broken without the patch.

fizk’s picture

fizk commented
Status: Reviewed & tested by the community » Closed (fixed)

Sorry for the delay. The patch has been committed!

fizk’s picture

fizk commented

The fixes are included in 7.x-1.0-beta8.

  • Commit f931d6a on 7.x-1.x, 8.x-1.x by fizk: 
    #1214080 marco, leafish_paul, fizk: Fix several issues to make this...