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.

Support from Acquia helps fund testing for Drupal Acquia logo

Comments

montesq’s picture

Status: Active » Postponed (maintainer needs more info)

When 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?

sammyman’s picture

I 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

montesq’s picture

Status: Postponed (maintainer needs more info) » Active

Ok it's clear and I confirm it's the same on my side with dev version

droplet’s picture

Priority: Normal » Major

bug confirmed. it have not add orderby into sql query.

droplet’s picture

Title: Forums will only organize by oldest first » Forums headings sorting broken
Status: Active » Needs review
FileSize
3.7 KB
droplet’s picture

Title: Forums headings sorting broken » Forum list header sorting broken
sammyman’s picture

Looks like this works for the sorting, but now all of my forums say that my posts were from "Anonymous". Weird.

droplet’s picture

FileSize
3.73 KB

needs review again, thanks

Star’s picture

Topic and Replies DO sort correclty but Last Reply is stuck is ascending order.

droplet’s picture

FileSize
3.72 KB

thanks 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.

droplet’s picture

Assigned: Unassigned » droplet
Status: Needs review » Needs work

postponed myself. I needs a benchmarks here.

droplet’s picture

Status: Needs work » Needs review
FileSize
2.95 KB

also fixed default $sortby issue

sammyman’s picture

Thanks for getting this fixed. Any idea why I get "By Anonymous" on every post even though it shows on the post who really posted?

catch’s picture

Patch 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.

catch’s picture

Status: Needs review » Needs work
Issue tags: +Performance

Also:

->orderBy('n.sticky', 'DESC')

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.

droplet’s picture

Status: Needs work » Needs review
FileSize
3.3 KB

Add a NID PRIMARY key to forum_index will be cool

id select_type table type possible_keys key key_len ref rows extra
1 SIMPLE ncs range PRIMARY,last_comment_uid PRIMARY 4 25 Using where; Using temporary; Using filesort
1 SIMPLE u2 eq_ref PRIMARY PRIMARY 4 drupal7x.ncs.last_comment_uid 1 Using where
1 SIMPLE n eq_ref PRIMARY,uid PRIMARY 4 drupal7x.ncs.nid 1
1 SIMPLE u eq_ref PRIMARY PRIMARY 4 drupal7x.n.uid 1 Using where
1 SIMPLE f eq_ref PRIMARY PRIMARY 4 drupal7x.n.nid 1 Using where

(*15193 rows in forum_index table.)

catch’s picture

Could 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.

droplet’s picture

FileSize
3.31 KB

EXPLAIN after PATHED (includes alter forum_index)


->orderBy('f.sticky', 'DESC')

id select_type table type possible_keys key key_len ref rows extra
1 SIMPLE ncs range PRIMARY,last_comment_uid PRIMARY 4 25 Using where; Using temporary; Using filesort
1 SIMPLE u2 eq_ref PRIMARY PRIMARY 4 drupal7x.ncs.last_comment_uid 1 Using where
1 SIMPLE n eq_ref PRIMARY,uid PRIMARY 4 drupal7x.ncs.nid 1
1 SIMPLE u eq_ref PRIMARY PRIMARY 4 drupal7x.n.uid 1 Using where
1 SIMPLE f eq_ref PRIMARY PRIMARY 4 drupal7x.n.nid 1 Using where


->orderBy('n.sticky', 'DESC')

id select_type table type possible_keys key key_len ref rows extra
1 SIMPLE f range PRIMARY PRIMARY 4 25 Using where; Using temporary; Using filesort
1 SIMPLE u2 index PRIMARY name 182 2 Using index; Using join buffer
1 SIMPLE ncs eq_ref PRIMARY,last_comment_uid PRIMARY 4 drupal7x.f.nid 1 Using where
1 SIMPLE u index PRIMARY name 182 2 Using index; Using join buffer
1 SIMPLE n eq_ref PRIMARY,uid PRIMARY 4 drupal7x.ncs.nid 1 Using where
mbarnson’s picture

Patch 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/

mikl’s picture

Subcribing…

mikl’s picture

Status: Needs review » Reviewed & tested by the community

I've tested this on http://drupaldanmark.dk/forum and here it works admirably.

troky’s picture

Status: Reviewed & tested by the community » Needs work

Setting forum_index.nid as primary key gives "Integrity constraint violation... duplicate key" when topic is moved to another forum leaving shadow copy.

droplet’s picture

Status: Needs work » Needs review
FileSize
3.41 KB

old patch tester, please remove old index yourself, thanks.

ALTER TABLE `forum_index` DROP PRIMARY KEY 
troky’s picture

Status: Needs review » Reviewed & tested by the community

Looks ok now.

mikl’s picture

Status: Reviewed & tested by the community » Needs work

I 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

Berdir’s picture

+++ modules/forum/forum.module
@@ -895,7 +894,30 @@ function forum_get_topics($tid, $sortby, $forum_per_page) {
+      ->addTag('node_access')

This is not necessary because node_access was already checked above. Would make it unecessary slow.

Powered by Dreditor.

droplet’s picture

@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.

rfay’s picture

Status: Needs work » Needs review

#23: forum.patch queued for re-testing.

rfay’s picture

Status: Needs review » Needs work

Sorry - mistake to change state.

Berdir’s picture

Sure, 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.

droplet’s picture

@Berdir,
thanks!!

I'm waiting for #25 feedback and will update the patch again :)

droplet’s picture

Status: Needs work » Needs review
FileSize
3.69 KB

new patch:
- remove not necessary addTag('node_access')
- also apply forum_index changes to new forum install

Aonoa’s picture

@droplet: I have applied your patch from #32 and so far so good. =)

Thank you!

Best regards,
Ao

Berdir’s picture

Version: 7.x-dev » 8.x-dev
Issue tags: +Needs backport to D7

Moving to the 8.x queue and tagging for possible backport.

mikl’s picture

Version: 8.x-dev » 7.x-dev
Status: Needs review » Reviewed & tested by the community

I 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?

rfay’s picture

Version: 7.x-dev » 8.x-dev

We 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

Dries’s picture

Status: Reviewed & tested by the community » Needs work

I tried applying the patch in #32 to my 8.x branch but it did not apply.

deimos:drupal-head dries$ git apply -p1 ../f.p 
error: patch failed: includes/entity.inc:408
error: includes/entity.inc: patch does not apply

I think it is because some of the documentation might be updated. Either way, this needs a re-roll.

droplet’s picture

FileSize
3.37 KB
3.67 KB

forum_d7, reroll, same as #32
forum_reroll, reroll & remove hook_update_7xxx

droplet’s picture

Status: Needs work » Needs review
drupalmonkey’s picture

droplet,

your d7 patch in #38 seems to be working perfectly for me.

omnyx’s picture

subscribing

catch’s picture

Status: Needs review » Reviewed & tested by the community

#38 passes tests, back to rtbc.

Dries’s picture

Status: Reviewed & tested by the community » Fixed

Looks good now. Committed to 7.x and 8.x.

andypost’s picture

Why this query does not use "node_access" tag?

Berdir’s picture

Because that was already done in the query above, it just queries a fixed set of nid's that was already checked for access.

andypost’s picture

Status: Fixed » Needs review
FileSize
503 bytes
Berdir’s picture

Status: Needs review » Fixed

Status: Fixed » Closed (fixed)

Automatically closed -- issue fixed for 2 weeks with no activity.

Valeratal’s picture

Wheare we have update for forum-module for drupal 7 ?

catch’s picture

When the next point release comes out.