DiscussionModel, CategoriesController, and You in 2.1b1

hgtonight

I have been playing around with the beta 1 release and looking at the migration path for a community I run. One of the core features I currently use in 2.0.18.x is overriding the category discussion sort order per category. E.g. the news category is sorted by date of discussion, our watercooler category is sorted by date of last comment, and our archive is sorted by discussion ID.

I have been doing this in an event handler called DiscussionModel_BeforeGet_Handler and changing the SortField and SortDirection per my desired configuration. This has worked wonderfully the past year.

This does not work in 2.1b1.

I traced it back to a change in the CategoriesController index function. When setting the discussion data, it uses a new function in the discussion model called GetWhere. In 2.0, this was just Get. Assuming this is an intentional change, I tested reverting it to a Get and it seemed to work the same. On top of this, my sorting plugin works without change.

I don't like modifying core files if possible, any thoughts?

hgtonight

Since this sunk to the second page without any replies, perhaps @Todd or @Lincoln could shed some insight?

hgtonight

Looks like this was fixed on github.

Side question, does anyone have a link to the commit on github that is released here as 2.1b1?

Side side question. is the release/2.1b1 branch considered beta stable?

rbrahmson

I have been doing this in an event handler called DiscussionModel_BeforeGet_Handler and changing the SortField and SortDirection per my desired configuration.

I also need such control over the sorting order.
1. Did you publish that plugin? (if not I think I can write one)
2. I am interested in a more complex sort order - by more than one field (e.g. Field1 ascending, field2 descending). Is this possible through the plugin and the DiscussionModel_BeforeGet_Handler even?

hgtonight

@rbrahmson said:

I have been doing this in an event handler called DiscussionModel_BeforeGet_Handler and changing the SortField and SortDirection per my desired configuration.

I also need such control over the sorting order.
1. Did you publish that plugin? (if not I think I can write one)
2. I am interested in a more complex sort order - by more than one field (e.g. Field1 ascending, field2 descending). Is this possible through the plugin and the DiscussionModel_BeforeGet_Handler even?

1. I think I was talking about my Blander Blog plugin, but that was almost 3 years ago. Here is the relevant excerpt from that:

   public function DiscussionModel_BeforeGet_Handler($Sender) {
     $Wheres = $Sender->EventArguments['Wheres'];
     if (is_array($Wheres) && array_key_exists('d.CategoryID', $Wheres) && in_array($Wheres['d.CategoryID'][0], C('Plugins.BlanderBlog.CategoryIDs'))) {
       $Sender->EventArguments['SortField'] = 'd.DateInserted';
       $Sender->EventArguments['SortDirection'] = 'desc';
     }
   }
   

2. Any SQL you can imagine you can implement. Not via this exact hook probably, but it can be done.

rbrahmson

Ah! Seems like the $wheres can be totally overridden... Will try that. Thanks!

R_J

@rbrahmson said:
Ah! Seems like the $wheres can be totally overridden... Will try that. Thanks!

Your problem was not about filtering the data, so you will not be happy with the $Wheres, you would have to take care for the SortField, and I think it would only accept one field here.

Linc

Rather embarrassing to have a discussion I completely missed on the first go-around resurfaced. Sorry about that.

rbrahmson

I just looked at Vanilla Discussion Models source and not only does it support only one field, but as I understand it, it checks which fields are passed as the sort field. That seems negate the possibility of adding another column for the sort field to serve in the sort order in lieu of multiple fields.
Too bad;-(

But there is a ray of light -- maybe @Linc who just joined in can remove these restrictions.

Linc

Probably should file an issue with some technical explanation if you want me to dive into something that deep in the product. I'm not gonna go from skimming a discussion to making complicated changes to core because you said it would be nice. I'm sure there's a good reason for the restriction if it's there.

rbrahmson

No idea how to file an issue. What is clear is that limiting the sort order to one column as well as to specific columns prevents admins/plugin coders to add columns for customizable sorts.

For example, the DiscussionAlert plugin adds a column to the discussion table. I'd like to sort "alerted" discussions on top while leaving the other sort order intact. Can't do that with the current state of affairs. I think the example is typical to other cases.

R_J

@rbrahmson said:
I just looked at Vanilla Discussion Models source and not only does it support only one field, but as I understand it, it checks which fields are passed as the sort field.

The SortField is checked against $AllowedSortFields, that's true, but there is also the method allowedSortFields() and you can add your own column to the $AllowedSortFields array with it

R_J

The orderBy() method takes a string of fields, so you would be able to pass more than one sort field (comma separated in a string, I guess). But first you have to add that string to the $AllowedSortFields array with the allowedSortFields() method.

The bad news is: this way, you will not be able to sort one column ascending and the other descending.

rbrahmson

Thank you @R_J, this is valuable but as Vanilla is set right now not useful.
@Linc - Please see the last few comments. Still hoping someone would consider this as a deficiency.

Linc

I don't understand what the deficiency is. @R_J just identified how to add another column to what's allowed to be sorted by.

After a quick look at the code, I can see we allow the sorting parameter to be passed in publicly by anyone. So removing the restriction would allow anyone to sort your discussions however they pleased, even by columns that could cause undue strain on your database or present your forum in a nonsensical way. So this seems like an important safety feature, not a deficiency.

rbrahmson

The deficiency is the inability to have different sorting orders to the different fields.
Also, the ability to allow users in my forum to sort by inefficient sorting order is still in my control ( can decide whether to allow such sorting or not through my plugin). My forum, my control. Notwithstanding, I do appreciate the performance warning.

R_J

I agree that there is something less flexible then it might be needed sometimes, but if you want to display something that is totally different than the "original" view, you might have to accept that you have to create your own view for that.

Linc

Am I missing something?

public function base_pickSomeHook_handler() {
    DiscussionModel::allowedSortFields(['whatever', 'you', 'want', 'to', 'sort', 'by']);
}

Done. Where's this deficiency?

R_J

But you cannot do DiscussionModel::allowedSortFields(['whatever asc', 'you desc', 'want asc', 'to desc', 'sort asc', 'by desc']);

rbrahmson

exactly!

Linc

saveToConfig('Vanilla.Discussions.SortDirection', 'asc', false); ?