Closed (fixed)
Project:
Filter by node type
Version:
6.x-1.x-dev
Component:
Code
Priority:
Normal
Category:
Feature request
Assigned:
Unassigned
Reporter:
Created:
26 Jun 2007 at 14:29 UTC
Updated:
12 Jun 2009 at 20:20 UTC
Jump to comment: Most recent file
Comments
Comment #1
Crell commentedComments are not currently supported, just node bodies. It's probably possible to extend it to do comments, but I haven't tried doing so.
Comment #2
physiotek commentedi would like to have this possibility also.
thanks,
pht3k
Comment #3
momper commentedyes - that would be nice ...
greetings momper
Comment #4
tjerah commentedThe code below should work but needs some refinements.
Hope it helps..
Comment #5
Crell commented@tjerah: Can you roll a proper patch? Thanks: http://drupal.org/diffandpatch
Comment #6
dejbar commented@tjerah : Correct me if I'm wrong your but your solution only alters the form so a user can't select a non-allowed filter. This is probably all a lot of sites need however a malicious user could still manually enter another filter option. In most case this level of paranoia probably isn't warranted. But if you were trying to restrict users from PHP filter access on comments or you had a really bad case of persistant link spammers then maybe it could be a problem.
Anyhow I've hacked by own shorter version of this. I'm not using 'Filter by node type' at the moment so this doesn't depend on it.
Thanks pointing me in the right direction.
Comment #7
Crell commentedNope. Drupal's Form API, by default, will check to ensure that a submitted value for a select box or radio button is one of the values listed in the form. That was one of its original reasons for existing. If the form, as it exists post form_alter, has only 2 possible radio buttons, then Drupal will reject any other value before it ever gets back to our submit handler. Form API is cool like that. :-)
Comment #8
dejbar commentedThat is indeed cool. Thanks for letting me know.
Comment #9
alippai commentedsubscribing
Comment #10
Crell commentedNew features only accepted against the D6 version. And this still doesn't have a patch.
Comment #11
Richard Blackborder commentedHere's a patch for a version of the module that handles comments per content type identically to the content types themselves. I've used some of the code from the module and the code from tjerah's post, and modified both slightly.
Comment #12
Richard Blackborder commentedActually, that patch doesn't apply the permissions settings. Here's an updated patch.
Comment #13
Crell commentedHaven't looked at the code yet, but it doesn't make sense, IMO, to restrict comments to the same format as the node. That makes sense only for forums, but not for anything else. It should either be all comments period, or comments-per-nodetype. The latter is probably more flexible.
Comment #14
Richard Blackborder commentedThere's a bit of misunderstanding here. Reading my post again, I can see why, but the patch gives comments-per-nodetype.
Here's what the form layout looks like (list items for checkboxes):
Allowed input formats:
Specify which input formats will be allowed on this node type's body field. Note that a user must still have access to the appropriate input format in order to be able to use it.
Allowed input formats (comments):
Specify which input formats will be allowed on this node type's comments. Note that a user must still have access to the appropriate input format in order to be able to use it.
Comment #15
Crell commentedAh! OK, good, that's what we should do. :-) I'll see if I can review this soon, but, um, no promises. (Anyone else who wants to jump in here and review the patch, feel free.)
Comment #16
batbug2 commentedapplied patch from #12, see no changes at all in the node type settings form
well scratch that. this was due to http://drupal.org/node/326373,
new options do show up, settings work fine. approved.
Comment #17
batbug2 commentedchanging the title
Comment #18
batbug2 commentedwell, thinking about that - you should apply the fix from http://drupal.org/node/326373 to you patch also. It's just one letter :)
Comment #19
batbug2 commentedOh, forgot, one more thing
this line
gives php error
Comment #20
Richard Blackborder commentedHere's the updated version.
Comment #21
skizzo commentedwill this feature be backported to D5?
Comment #22
Richard Blackborder commentedPersonally I only learned D6 coding, so it won't be backported by me, sorry.
Comment #23
Crell commentedThe D5 version is in support mode now, so I won't be adding new features to it. I don't have enough spare bandwidth to work on new features for both versions.
I'll try to test this patch as soon as I get the chance. Others are welcome and encouraged to do so in the meantime. :-)
Comment #24
gddI gave this patch a run-through today, and it works well although I have some comments on the code and implementation
- This patch also fixes a typo bug (the role 'administer filters' is mis-spelled in filterbynodetype_form_node_type) which has already been fixed and committed in #326373: Non-uid 1 cannot set configuration
- I think it makes more sense to put the comment input filter settings under 'Comment Settings' rather than in 'Submission Form Settings' with the other choices.
- There is a lot of repeated code here and I'm thinking there should be a way to break it out into a function call for the sake of maintainability.
Otherwise this is great and well-needed.
Comment #25
Richard Blackborder commentedThank you for reviewing the patch heyrocker.
Previous reviewers asked for that change to be included in this patch. I remade the patch to do so. I do not know which way to go with this now.
Comment #26
Crell commentedThe dev release of this module already has that bugfix, so the patch here won't apply if you try to fix it again. Don't fix what's already been fixed. :-)
Comment #27
Richard Blackborder commentedOkay, I have attempted to address these issues:
I agree, but I don't think it is possible. The comment module uses hook_form_alter, just like filterbynodetype does. comment.module thus overwrites the comment section with a blank fieldset element, erasing any fields which filterbynodetype may have added to it. Going by this, there's nothing I can do about this.
I've done that in this new version of the patch. I didn't move everything inside the if blocks, but trying to move all the code out would require a lot more if statements and probably create more maintenance worries.
Comment #28
Crell commentedWe can always push the module weight up to be comment.module +5 or something. That can be done in the installer and in an update hook.
Comment #29
Richard Blackborder commentedI've applied that technique, but I also had to move some of the code around, so this patch is now bigger than either module file. I've also patched the install file, which allowed me to add variable clean up to filterbynodetype_uninstall() as well.
Comment #30
summit commentedSubscribing, greetings, Martijn
Comment #31
TripleEmcoder commentedHello,
I made a patch to support comments before finding this discussion. It does solve some the most important issues you mention here, however, so I submitted it anyway. It's against 6.x-1.2, so a made a separate issue. I needed it in production, so I didn't even think about working with 6.x-dev - sorry about that!
Anyway, please check out #351313: Comment support, maybe it could be useful.
Marcin
Comment #32
Richard Blackborder commentedThrough my own use I did find a bug with this patch and I fixed it.
Comment #33
jrglasgow commentedthe patches in #32 are working for me.
I have re-rolled them though into one file.
Comment #34
jrglasgow commentedI found some typos in a comment or two, those have been fixed in this patch
Comment #35
Richard Blackborder commentedThanks jrglasgow.
I've been using the old version for about 3 months, and this new version for about a week now with no problems.
With that plus jrglasgow's review, would it be appropriate to status change this to "reviewed & tested by the community"?
Maybe even... get it into the module?
Comment #36
jrglasgow commentedI feel that it is in fine working order. I think it should be considered tested
Comment #37
gddThis patch has been committed, thanks all for your help and patience.
I'm going to do some slight code cleanup and roll a new release later today.
Comment #38
Crell commented