Closed (fixed)
Project:
Bad Behavior
Version:
7.x-1.x-dev
Component:
Code
Priority:
Normal
Category:
Feature request
Assigned:
Unassigned
Reporter:
Created:
23 Mar 2011 at 00:44 UTC
Updated:
16 Jan 2012 at 15:58 UTC
Jump to comment: Most recent file
Comments
Comment #1
jeffwidman commentedsubscribe
Comment #2
dreadlocks1221 commentedsubscribe
Comment #3
gregarios commentedIf someone wants to submit a patch for the 6.x-2.x-dev release that brings it into 7.x compliance I will commit it to a new 7.x-2.x-dev branch release. I have no Drupal 7 sites nor time to develop a 7.x release yet without help. I have no idea what in the 6.x-2.x-dev release needs to change to make it a 7.x release.
Comment #4
Romaq commentedsubscribe
Comment #5
darioshanghai commented+1
Comment #6
flightrisk commentedNo one can help? I wish I knew D7 and PHP enough to port this
Comment #7
gregarios commentedI actually am part way through trying to get this ported, but I got stuck on some errors like this one and can't muster enough PHP or Drupal smarts to figure out the new method to get 'er done:
...happens on this code in the badbehavior.admin.inc file...
I can't seem to get the new database querying down.
Comment #8
espirates commentedsubscribe+
Comment #9
Mamouri commentedsubscribe
Comment #10
niteflyer32 commentedRunning 7 here and really need a solution to the scraper and spam bots. Have Mollem module already. Thanks for any help.
Comment #11
toxiclung commentedsubscribe
Comment #12
hansfn commented@gregarios: I'm probably capable of solving your remaining issues, but doesn't it make sense that you create a 7.x-1.x branch and commit your stuff there so I can work on it from there?
Comment #13
gregarios commented@hansfn:
I hate to say it but I lost all my work on the 7.x branch I was developing when my hard drive crashed and burned. Any 7.x branch will have to be started from scratch from the new 6.x-2.x-dev version found here:
https://drupal.org/node/1147668
I don't want to post any new 7.x-2.x-dev release until I have a working copy on my own installation of Drupal 7 I can test. If you'd like to start with the 6.x version and send me a zipped (or .tar.gz) version of a working 7.x I'll test it and begin a new 7.x release with it. (giving you full credit BTW)
(Also, I have no time to work on it myself right now, or for the next 6mo at least. The most I can do is keep up with new releases to match any new BB Script releases that come out)
Comment #14
hansfn commentedOK, I have completed the port to Drupal 7 - see attached patch. I have tested it thoroughly.
I would very much like it if you made sure that I was credited for the commit (of the patch). The patch is Git aware - read more on Adding a commit author if that doesn't mean anything to you.
Comment #15
gregarios commentedThanks hansfn. I'm away the next few days, but should be able to get this up next week. I will read up on the 'commit author' commands.
Comment #16
smithdalec commentedWell this is ironic. I've been working on converting BB to Drupal 7 for the last week or so, and also just finished this morning.
I have a fully working sandbox module here: http://drupal.org/sandbox/smithdalec/1354660
We could combine hansfn's code with my code, if that helps. I used the Database Abstraction Layer as opposed to query strings. Other than that, our changes are pretty much the same!
If you would like to add me to the maintainers' list of this project, I will be more than happy to keep contributing :D
Comment #17
gregarios commentedI think it would be far better if you (smithdalec) and hansfn get together and combine your forces to come up with an improved 7.x release, since my releases were so "newbie" in my opinion anyway. Since I have very little time (if any) to contribute to this lately, it would be very beneficial for one of you to contact Dave Reid to gain maintainer access for this purpose as well, since my grasp of Git is pretty weak conpared to both of you, too. I don't have full module access except to release new versions anyway.
You would have my blessing and encouragement to improve my 6.x version as well as release a new 7.x version of this module. I don't think there is any need to come up with an entirely new module like you have worked on, smithdalec, when you could just take over this one. (-;
Let me know if this sounds acceptable to both of you.
Comment #18
hansfn commented@smithdalec:
To be honest, I find it mostly annoying ;-) I wasted an evening writing the port when you were already 90% done. Anyway, we can do as you suggest - complete the work in your sandbox (since you have added even more nice comment blocks than I have done and removed unused functions) before turning the sandbox into the 7.x-2.x branch here. Since I won't get credit for actually commit here on Drupal, you'll have to add me to the README.txt
I have some comments:
1) bb2_db_escape should do some escaping - you probably understand why. (I notice you have fixed this now - last night.)
2) The comment to bb2_db_date seems excessive - the function doesn't take any timestamp input.
3) Your version doesn't work with Drush - apply my fixes.
4) You have wrong path in badbehavior_menu - apply my fix. (Add it to the .info file too.)
5) You misunderstood badbehavior_permission - see my version.
6) I think you have misunderstood the use of "files" in the .info file. I suggest removing those lines. (The paths are wrong anyway.) And why have you added "php = 5.3". That is stricter than Drupal 7 ...
7) define('BB2_CWD', dirname(__FILE__) . '/../../libraries/bad-behavior'); doesn't work if someone install this module in a subfolder of sites/all/modules. Maybe better to use the constant DRUPAL_ROOT
8) Remove badbehavior_footer and apply my fix for badbehavior_page_build.
OK, I'll do another review of your code when you have looked through the above comments. (Just for the record, we have done many of the same changes and many parts of your code is better than mine - don't take the comments above as critique.)
Do you want maintainer access for the module or this a one time effort? If you want access, I suggest contacting Dave as suggested above.
Comment #19
smithdalec commentedThanks for the code review, Hans. It was actually very helpful.
I've gone ahead and pushed my latest changes to my sandbox, and requested maintainer access from Dave Reid.
As far as the individual issues to my code go:
1. Already done
2. Done - removed the @return description
3. What part of Drush is my version not working with?
4. Done - used your code
5. Done - used your code
6. Done - removed lines
7. Done - changed back to how it was in 6.x-2.x
8. Done - used your code
I added you to the README.txt list of contributors, and I'll be sure to also give you credit in the 7.x-1/2.x fork commit message.
Thanks again for the help, and let me know if theres anything else you see wrong with the code.
Comment #20
hansfn commentedRegarding Drush: BadBehavior blocked Drush - "Header 'Referer' present but blank". This stopped Drush from for example disabling BadBehavior.
There are two more things I find strange - regard badbehavior_boot and badbehavior_init:
Added: OK, I guess the reason for the different uid checks in badbehavior_boot and badbehavior_init is to avoid running Bad Behavior twice. If that is the case, isn't it better to remove the uid check and just call badbehavior_start_badbehavior inside badbehavior_boot?
Comment #21
smithdalec commentedI really don't think we need hook_init. It runs after hook_boot (we want BB to load as early as possible), and hook_init only runs on non-cached pages, while hook_boot runs on all pages. So if a non-cached page is requested, the BB scripts run twice, which is rather pointless.
Comment #22
mntash commentedBump! Would love to have a D7 version of Bad Behavior... so surprised there isn't more interest in this module!
Comment #23
smithdalec commentedI've been running the version that Hansfn and I created on Drupal 7 for a month now without problems.
I requested maintainer access from Dave Reid a while back, but haven't heard from him.
In any case, I've attached the patch for the BB 6.x-216 to D7 port.
@ mntash: You can take the patch and apply it over the current version of BB, if you'd like.
@ Bad Behavior maintainers: I would love to become a fellow maintainer, if possible! It would be nice to see this patch get applied to an official 7.x release ;-)
Comment #24
gregarios commentedHow do I apply that patch? I tried
patch < BadBehavior_6.x-216_to_7.x-1.0.patchwithin the badbehavior directory but it had errors all over...Comment #25
smithdalec commentedIn the Bad Behavior directory...
See what changes will be made when the patch is applied:
git apply --stat BadBehavior_6.x-216_to_7.x-1.0.patchTest the patch before applying:
git apply --check BadBehavior_6.x-216_to_7.x-1.0.patchApply the patch:
git apply BadBehavior_6.x-216_to_7.x-1.0.patchComment #26
gregarios commentedComment #27
gregarios commentedComment #28
gregarios commentedSee the attachment. I may need you to walk me through this... Are you proficient enough to walk me through applying the patch and getting the 7.x dev released?
Comment #29
smithdalec commentedWow, that's strange. Is your BB the dev version? I patched the 6.x-216 version on the main page, and it works fine for me.
Also, I've never released an official module version on Drupal.org before, but I'm certainly willing to research and learn :D
Comment #30
gregarios commentedOh... my mistake it was the dev version... I'll try it again...
Comment #31
gregarios commentedPlease check the attached archive of a complete 7.x version and make sure it is still working if you would. If it is, I may be able to release it as a new 7.x-2.x-dev version.
Comment #32
smithdalec commentedThat 7.x-2.x package works under my Drupal installation.
I just ran diff comparing that .zip to my version which I used to patch it, and everything between the two packages looks to be the same (other than some trailing whitespace I've gotten rid of since Friday), so we can be assured that the patch worked.
It looks like it's ready for the dev release.
Comment #33
gregarios commentedA new release is forthcoming today. Check here: http://drupal.org/node/1398882
@smithdalec: Also, would you please do me a big favor? Since you guys know a lot more about module design than I, can you please grab the 6.x-2.216 version, make any changes that would improve it, ZIP up the whole package and send it to me (attach it to a new issue if you like). I know it probably has some inefficiencies or Drupal "no-no's" that could be improved. In turn, I'll continue to make releases to the 7.x branch using your patches in the future. (Ok I probably would anyway, but please!?) lol
Comment #34
smithdalec commentedSure, I'll take a look at it.
It should be noted that I'm a new Drupal module developer, though, and I know D7 better than D6 :D
Comment #35
gregarios commentedComment #36
smithdalec commentedGood to see the new 7.x-2.x dev release on the main page!
However, don't forget to credit Hansfn's and my contributions for the port in your commit ;-)
Comment #37
gregarios commentedCrud I knew I forgot something! Sorry 'bout that. I'll edit it now...
Comment #38
gregarios commented