Too many forums can make a site unusable. We have a site that has over 3,000 forums and the initial forum page load takes forever. Once you have drilled-down to sub-forums, things speed up somewhat. This patch gives an option to limit the number of forums displayed on the main screen so that only one level of forums are displayed (sub-forums are not displayed until you drill-down into their parent).
The patch consists of two pieces, one modification to the forum.admin.inc that adds a check-mark option to enable this bug-fix. The other is the modification to forum.module which, when the check-mark is selected, returns only the top-level forums for the level one is looking at.
Comment | File | Size | Author |
---|---|---|---|
#21 | forum-783230-21.patch | 1.98 KB | alexweber |
#10 | forum-783230-2.patch | 2.01 KB | alexweber |
#9 | forum-783230.patch | 3.88 KB | alexweber |
drupaldiff-D6.patch | 1.86 KB | tyoungls | |
Comments
Comment #1
dddave CreditAttribution: dddave commentedNot sure if this is a bug or a feature request...
Comment #2
tyoungls CreditAttribution: tyoungls commentedFor us, it is a bug. Our site becomes slow to the point of unusability without this. We programmed the "fix" to appear as a feature, because it changes the look and feel in a way that not everyone wants. Not everyone will want this "bug fix" the way it is implemented because of the change in look/feel, so you can selectively enable/disable it. Those of us with massive numbers of forums and sub-forums will highly appreciate the increase in speed of not generating one page that lists all the forums at once. It is much easier to navigate this way than by simply putting a paging system on the top-level.
So, for us it is not a bug that makes the site go "boom" or a security glitch that allows hackers in. But it is a bug that turns our fluid site into a frozen mass, literally taking minutes to load some pages. Maybe taking minutes to load is considered a "feature" by some, but for us, it is a bug. Therefore, the fix for us is a "bug fix." I can completely understand if you all do not see it as such. Very few people have a site with as many forums as we have, and so it is probably a very rarely encountered "bug". For most other people, this will simply be a change in appearance and navigation, instead of a matter of life and death. For them, it is a feature.
I can see people perceiving this either way. Almost a year ago I posted this as a "new feature" and the response was, "new features are only being added to the next version." They said to upgrade to 7 and re-submit the request. We were not at a spot where we wanted to upgrade to the beta version, so we have just kept applying the security releases, re-applying this fix of ours. We would love it if it were a "bug" that could be applied to the version 6, or even considered a feature, but still applied to version 6.
Comment #3
dddave CreditAttribution: dddave commentedI said that completely free from any judgment and I find it great that you provide your fix back to the community. I am in no position to review your code but I really hope someone with the necessary skills takes notice.
Comment #4
tyoungls CreditAttribution: tyoungls commentedThanks. It is nice that someone at least noticed it. :)
Comment #5
Jody LynnI don't believe core forum module was designed to handle 3,000 forums, so making it more robust is a feature request not a bug, and features need to be added first to 8.x now.
Comment #6
pumpkinkid CreditAttribution: pumpkinkid commentedI too would like to see this feature... If as Jody Lynn says, Drupal Forums was not meant to handle so many forums, then it needs to be able to do so...
I haven't yet looked at the code in the patch... but could this be made into a module?
I myself am new to creating modules, however, wouldn't mind taking a crack at it seeing as I too want this feature...
Comment #7
pumpkinkid CreditAttribution: pumpkinkid commentedAs I said, not very well versed in creating modules... however, wouldn't an implementation of the forum_get_forums() hook allow the same type of modification at the module level?
Comment #8
alexweber CreditAttribution: alexweber commentedDecided to take this one on, the patch seems pretty solid just need to tweak it for D7 compatibility
Comment #9
alexweber CreditAttribution: alexweber commentedpatch attached!
it's pretty much the same basic idea from the original patch but instead of an on/off checkbox to limit depth, I added a select where you can specify the depth, from 0 (disabled) to 10.
Comment #10
alexweber CreditAttribution: alexweber commenteddespite passing the test the patch had an error where i incorrectly incremented the depth counter, updated patch attached.
Comment #11
DevElCuy CreditAttribution: DevElCuy commentedI love this feature!
Comment #12
DevElCuy CreditAttribution: DevElCuy commentedPatches at #9 and #10 need to be merged.
Comment #13
alexweber CreditAttribution: alexweber commented@develCuy, patch #10 is already a merged version of both! It looks smaller because patch #9 included some unecessary commits because I first made the setting a checkbox and then later, in another commit, made it into a select box.
Patch #10 goes straight to the select approach! :)
Should be good to go!
Comment #14
DevElCuy CreditAttribution: DevElCuy commentedGreat job! I confirm that the patch works, now it needs the unit test.
Comment #15
alexweber CreditAttribution: alexweber commentedFair enough, test comming up! :)
Comment #16
alexweber CreditAttribution: alexweber commentedWriting the test is blocked by #1374152: Allow forum_forum_load() to force static cache reset.
Otherwise the patch seems to be solid, but we need the test... right?
Comment #17
DevElCuy CreditAttribution: DevElCuy commentedAdding tag "dlatino" for reference of the Drupal Latino community.
Comment #18
edutrul CreditAttribution: edutrul commentedNo need to make a test. Good to go!
Comment #19
oriol_e9gIstead of:
You can use:
It's shorter an easier to maintain.
Plus: There is a bug in the sequence, see 8, 8 --> 8, 9.
Comment #20
oriol_e9gComment #21
alexweber CreditAttribution: alexweber commented@oriol_e9g attached patch with the range() implementation, don't understand what you mean by the sequence thing.
Comment #22
oriol_e9g@alexweber I said that in #10:
there were two eights and none nine! look carefully!
...but this is solved in #21 using the range();
Comment #23
alexweber CreditAttribution: alexweber commented@oriol_e9g, awesome!
Comment #24
xuruo CreditAttribution: xuruo commentedcan this patch used in version 7.x?
Comment #39
quietone CreditAttribution: quietone at PreviousNext commentedForum is approved for removal. See #1898812: [policy] Deprecate forum module for removal in Drupal 11
This is now Postponed. The status is set according to two policies. The Remove a core extension and move it to a contributed project and the Extensions approved for removal policies.
It will be moved to the contributed extension once the Drupal 11 branch is open.