Early Bird Registration for DrupalCon Portland 2024 is open! Register by 23:59 UTC on 18 March 2024, to get $100 off your ticket.
I have tried to change the organization of forum threads to newest first, but for some reason it is stuck in the oldest posts first organization.
Comment | File | Size | Author |
---|---|---|---|
#46 | 1008410-forum.patch | 503 bytes | andypost |
#38 | forum_d7.patch | 3.67 KB | droplet |
#38 | forum_reroll.patch | 3.37 KB | droplet |
#32 | f.patch | 3.69 KB | droplet |
#23 | forum.patch | 3.41 KB | droplet |
Comments
Comment #1
montesq CreditAttribution: montesq commentedWhen I am in a forum, the table has 3 columns that are "sortable" : topic, replies and last reply. So I don't know how to sort by creation "date". Could you specify how to try it?
Comment #2
sammyman CreditAttribution: sammyman commentedI can't sort for some reason by clicking on last reply or topic. You can see my problem here:
http://www.dealdaddy.com/forum/hot-deals
Comment #3
montesq CreditAttribution: montesq commentedOk it's clear and I confirm it's the same on my side with dev version
Comment #4
droplet CreditAttribution: droplet commentedbug confirmed. it have not add orderby into sql query.
Comment #5
droplet CreditAttribution: droplet commentedComment #6
droplet CreditAttribution: droplet commentedComment #7
sammyman CreditAttribution: sammyman commentedLooks like this works for the sorting, but now all of my forums say that my posts were from "Anonymous". Weird.
Comment #8
droplet CreditAttribution: droplet commentedneeds review again, thanks
Comment #9
Star CreditAttribution: Star commentedTopic and Replies DO sort correclty but Last Reply is stuck is ascending order.
Comment #10
droplet CreditAttribution: droplet commentedthanks for all testing the patch, here is a new one.
- 3 header sorter: WORKED
- stick post: WORKED
when create a new content, drupal (nodes, content types)/ forum added same timestamps to create & last reply, so Last Reply also following that and have not avoid no comment topics. (now it's all same as D6)
we may need to create an new issues for timestamps problem.
Comment #11
droplet CreditAttribution: droplet commentedpostponed myself. I needs a benchmarks here.
Comment #12
droplet CreditAttribution: droplet commentedalso fixed default $sortby issue
Comment #13
sammyman CreditAttribution: sammyman commentedThanks for getting this fixed. Any idea why I get "By Anonymous" on every post even though it shows on the post who really posted?
Comment #14
catchPatch looks good so far. Can't we skip the join on {forum} and use {forum_index} for that part of the query instead?
Also this absolutely needs EXPLAIN posting for the query (with some generated or real forum data) before it's committed.
Comment #15
catchAlso:
this should be changed to f.sticky.
If we do this, then along with the other changes here this query may finally use an index, which would be amazing.
Comment #16
droplet CreditAttribution: droplet commentedAdd a NID PRIMARY key to forum_index will be cool
(*15193 rows in forum_index table.)
Comment #17
catchCould you post the EXPLAIN for the new query too? This looks like the old one.
Also nitpicking:
+ $query->fields('n', array('title','nid','type','sticky','created','uid'));
there should be a space before each comma.Comment #18
droplet CreditAttribution: droplet commentedEXPLAIN after PATHED (includes alter forum_index)
->orderBy('f.sticky', 'DESC')
->orderBy('n.sticky', 'DESC')
Comment #19
mbarnson CreditAttribution: mbarnson commentedPatch fixes issue on 7.0 release. Not that my opinion means much, but I'd love to see this accepted for 7.1!
Now can sort by columns (dates, replies, etc.), and the forums obey the default sort order, too.
Demo forum with patch working at http://utahheli.org/
Comment #20
mikl CreditAttribution: mikl commentedSubcribing…
Comment #21
mikl CreditAttribution: mikl commentedI've tested this on http://drupaldanmark.dk/forum and here it works admirably.
Comment #22
troky CreditAttribution: troky commentedSetting forum_index.nid as primary key gives "Integrity constraint violation... duplicate key" when topic is moved to another forum leaving shadow copy.
Comment #23
droplet CreditAttribution: droplet commentedold patch tester, please remove old index yourself, thanks.
Comment #24
troky CreditAttribution: troky commentedLooks ok now.
Comment #25
mikl CreditAttribution: mikl commentedI have tried out the latest patch, but I still see problems here – this list is frozen as of weeks ago, so new posts do not appear on top of the list: http://drupaldanmark.dk/forums/ordet-frit
Comment #26
BerdirThis is not necessary because node_access was already checked above. Would make it unecessary slow.
Powered by Dreditor.
Comment #27
droplet CreditAttribution: droplet commented@mikl,
can you explain your issue more. I can't see any problem, thanks.
@Berdir,
already defined ?? can you point out where is it, thanks.
Comment #28
rfay#23: forum.patch queued for re-testing.
Comment #29
rfaySorry - mistake to change state.
Comment #30
BerdirSure, see http://api.drupal.org/api/drupal/modules--forum--forum.module/function/f.... In the first query, we are adding the node_access tag, so the node permissions are checked.
In the query you are updating, we only look for a already defined set of nid's for which we know that the user has permissions.
Comment #31
droplet CreditAttribution: droplet commented@Berdir,
thanks!!
I'm waiting for #25 feedback and will update the patch again :)
Comment #32
droplet CreditAttribution: droplet commentednew patch:
- remove not necessary addTag('node_access')
- also apply forum_index changes to new forum install
Comment #33
Aonoa CreditAttribution: Aonoa commented@droplet: I have applied your patch from #32 and so far so good. =)
Thank you!
Best regards,
Ao
Comment #34
BerdirMoving to the 8.x queue and tagging for possible backport.
Comment #35
mikl CreditAttribution: mikl commentedI have tried the patch from #32 on my Drupal site and a test installation, and it works perfectly. Could we just get it merged and worry about D8 later?
Comment #36
rfayWe all wish this was the case, but the simple fact is it will never go into D7 until it goes into D8.
Best thing you can do is a quick test on D8 and push it to RTBC on 8
Comment #37
Dries CreditAttribution: Dries commentedI tried applying the patch in #32 to my 8.x branch but it did not apply.
I think it is because some of the documentation might be updated. Either way, this needs a re-roll.
Comment #38
droplet CreditAttribution: droplet commentedforum_d7, reroll, same as #32
forum_reroll, reroll & remove hook_update_7xxx
Comment #39
droplet CreditAttribution: droplet commentedComment #40
drupalmonkey CreditAttribution: drupalmonkey commenteddroplet,
your d7 patch in #38 seems to be working perfectly for me.
Comment #41
omnyx CreditAttribution: omnyx commentedsubscribing
Comment #42
catch#38 passes tests, back to rtbc.
Comment #43
Dries CreditAttribution: Dries commentedLooks good now. Committed to 7.x and 8.x.
Comment #44
andypostWhy this query does not use "node_access" tag?
Comment #45
BerdirBecause that was already done in the query above, it just queries a fixed set of nid's that was already checked for access.
Comment #46
andypostComment #47
BerdirNo, this is not necessary, see http://api.drupal.org/api/drupal/modules--forum--forum.module/function/f....
Comment #50
Valeratal CreditAttribution: Valeratal commentedWheare we have update for forum-module for drupal 7 ?
Comment #51
catchWhen the next point release comes out.