there is no way to do that with the BeforeGet event which gets passed $SortField and $SortOrder. Later in the DiscussionModel, you'll find ->orderBy($SortField, $SortDirection).
While orderBy() accepts an array for the sort field , the sort order could only be 'asc' and is set to 'desc' if any other value is passed.
So to overcome what might be seen as a deficiency, I think there would be three possibilities:
a) Changing the DiscussionModels getWhere() method so that it contains some is_array($SortField) code
b) Adding code to the SQLDrivers orderBy() method to handle $SortField being an array.
But I cannot think of some elegant code for either "solution" and @rbrahmson I do not think that some edge case like that would justify inserting more complicated code to handle that.
c) Add $this->EventArguments['Sql'] =& $Sql; right before the fireEvent('BeforeGet').
That way plugin authors would have much more freedom, but I guess there is a reason why it hasn't been done this way. @rbrahmson at least you could try and make a pull request for that change.
@Linc - My forum users asked me to find a way to put starred and alerted discussions (discussions marked by a custom field added by one of our plugins) on top while keeping the rest of the list sorted as usual. That's basically a combination of three sort fields. When I was set to write a plugin for that I did a little research and stumbled upon these limitations. Having to write a custom view looks like an overkill but unfortunately it seems like the only option here.
In addition to this real life example I can easily see users or plugin developers benefiting from the added flexibility.
@R_J - I opened a github account but haven't used it yet. No clue how to open a "pull" request but from the lingo here I guess it means "request for change".
git is a very mighty tool and because I do not use it regularly, I always have to struggle. But changing one file only could easily be done in the web front end of GitHub. Just go here and click on the pen in the upper right corner. You can make the change around line 297 and have to provide a title and an explanation of your change. That should be enough.
Don't forget to sign the contributors agreement
I signed the agreement, I think I did what you asked for (web interface), it seemed to make the change in a cloned repository under my userid and when I look at the Pull Requests tab (in the Vanilla repository) and filter by my userid I do not see the change. Maybe I don't understand Github but how will the Vanilla team know of this if it doesn't show up under that tab?
Finally got it done. Must have missed something along the way. Thanks for holding my hands.
I guess you wanted to get me over the fear of using Github because you deserve the credit for this pull request as the change is your code contribution, not mine. You are a gentleman and a teacher!
@rbrahmson said:
I am already derided and mocked by some in this forum
@rbrahmson said: @Linc - My forum users asked me to find a way to put starred and alerted discussions (discussions marked by a custom field added by one of our plugins) on top while keeping the rest of the list sorted as usual.
You could create a starred module and alerted module that spits out a discussion list of these. Target the content asset. Then you add those modules to the discussions controller. You could use CSS to make the list look seamless.
The only "difficulty" would be removing any duplicates from the list. This could be done via JS. It would slightly mess up your discussion list length, but I don't think many would notice this or even care about it.
@rbrahmson said:
I am already derided and mocked by some in this forum so thanks for the coup de grâce.
If that was a poke at me, the source of my frustration is that you didn't ask how to do a more complex discussion sort or lay out the scope of your challenge. You came in here and pinged me to a fix a "bug" that didn't exist, only told me half the story, and acted like we were out to somehow restrict your use of the software. If I'm taking time out of my work day to come over here and dole out advice, it'd be nice if you didn't approach me that way.
The filter is both a security and performance measure. There's plenty of ways to accomplish what you want. @R_J's code is a good first step, so is @hgtonight's idea.
Sorry Linc, it's not you -- "by some in this forum" talks about the forum, not this discussion, so it's definitely not you. Besides, you were professional and respectful and I thank you and everyone who participated in this dialog, so how could have I have aimed this at you? Sometimes humor is dangerous in forums, so that comment with the coup de grace ending was a dangerous response to R_J funny "If this is an idiotic security hole, they'll blame you. That's why I sent you to the front. "
As it is, this my first "play" at github and I deserve the lesson of jumping a pull request. Still not sure how I undo (push?) a pull ;-).
Choosing any sort order can make the forum run slower if the field is not properly indexed. I remember the restriction was there for reason. If you want to sort by a different field apply an index to it if one doesn't already exist.
Thanks, @Linc. I'm already following this discussion full of interest I've done a full text search through all the plugins I've downloaded and those are around 100. I've found only one plugin using that and the author is me So I guess the danger of breaking plugins is 1:100
Generally I'd say that backwards compatibility for edge cases shouldn't hinder new feature development. If the old feature is of less importance it might be canceled completely.
@x00: I totally agree, but I think a framework should be flexible and that includes not making it impossible to write code that's not optimal.
Thanks @linc. I think the change is easy enough that while it is not backward compatible it is an easy plugin code change (easy for me to say...).
As for performance hit risks that were mentioned earlier, I think that in principle it falls on the shoulders of admins and plugin developers. There are endless ways in which a plugin could cause performance hits. Admins too can degrade performance by choosing bad plugin customization options. In my opinion there should not be additional built-in restrictions aside from warnings (plugin description/readme/config screens).
This has evolved even more. I think where we're at now is: we're going to overhaul this entire system and remove all the old behavior. We're removing the 'sort' endpoint, which also absolves us from needing the whitelist since a user can't arbitrarily call a different sort column then. Discussion sorting will be handled by a new module, which will be 100% pluggable.
Comments
@Linc Changing one sort order is no problem at all, but you cannot have two different sort orders.
I'll give an example but I must admit I cannot think of a combination that someone would be interested in...
While you would be able to write
$sql ->orderBy('d.CountViews', 'desc') ->orderBy('d.Name', 'asc')there is no way to do that with the BeforeGet event which gets passed $SortField and $SortOrder. Later in the DiscussionModel, you'll find
->orderBy($SortField, $SortDirection).While orderBy() accepts an array for the sort field , the sort order could only be 'asc' and is set to 'desc' if any other value is passed.
So to overcome what might be seen as a deficiency, I think there would be three possibilities:
a) Changing the DiscussionModels getWhere() method so that it contains some
is_array($SortField)codeb) Adding code to the SQLDrivers orderBy() method to handle $SortField being an array.
But I cannot think of some elegant code for either "solution" and @rbrahmson I do not think that some edge case like that would justify inserting more complicated code to handle that.
c) Add
$this->EventArguments['Sql'] =& $Sql;right before the fireEvent('BeforeGet').That way plugin authors would have much more freedom, but I guess there is a reason why it hasn't been done this way. @rbrahmson at least you could try and make a pull request for that change.
@Linc - My forum users asked me to find a way to put starred and alerted discussions (discussions marked by a custom field added by one of our plugins) on top while keeping the rest of the list sorted as usual. That's basically a combination of three sort fields. When I was set to write a plugin for that I did a little research and stumbled upon these limitations. Having to write a custom view looks like an overkill but unfortunately it seems like the only option here.
In addition to this real life example I can easily see users or plugin developers benefiting from the added flexibility.
@R_J - I opened a github account but haven't used it yet. No clue how to open a "pull" request but from the lingo here I guess it means "request for change".
git is a very mighty tool and because I do not use it regularly, I always have to struggle. But changing one file only could easily be done in the web front end of GitHub. Just go here and click on the pen in the upper right corner. You can make the change around line 297 and have to provide a title and an explanation of your change. That should be enough.
Don't forget to sign the contributors agreement
I signed the agreement, I think I did what you asked for (web interface), it seemed to make the change in a cloned repository under my userid and when I look at the Pull Requests tab (in the Vanilla repository) and filter by my userid I do not see the change. Maybe I don't understand Github but how will the Vanilla team know of this if it doesn't show up under that tab?
Look at https://github.com/your_name_here/vanilla : you'll see a green button "New pull request". Press ist and follow the instructions.
Finally got it done. Must have missed something along the way. Thanks for holding my hands.
I guess you wanted to get me over the fear of using Github because you deserve the credit for this pull request as the change is your code contribution, not mine. You are a gentleman and a teacher!
If this is an idiotic security hole, they'll blame you. That's why I sent you to the front.
Well, sure, that is another way to teach me;-) I am already derided and mocked by some in this forum so thanks for the coup de grâce
You could create a starred module and alerted module that spits out a discussion list of these. Target the content asset. Then you add those modules to the discussions controller. You could use CSS to make the list look seamless.
The only "difficulty" would be removing any duplicates from the list. This could be done via JS. It would slightly mess up your discussion list length, but I don't think many would notice this or even care about it.
Search first
Check out the Documentation! We are always looking for new content and pull requests.
Click on insightful, awesome, and funny reactions to thank community volunteers for their valuable posts.
There is already an answer on GitHub and the answer is promising. It lets me assume that you would be able to do
public function discussionModel_beforeGet_handler($sender, $args) { Gdn::sql()->orderBy('columnA', 'asc'); Gdn::sql()->orderBy('columnB', 'desc'); }If that was a poke at me, the source of my frustration is that you didn't ask how to do a more complex discussion sort or lay out the scope of your challenge. You came in here and pinged me to a fix a "bug" that didn't exist, only told me half the story, and acted like we were out to somehow restrict your use of the software. If I'm taking time out of my work day to come over here and dole out advice, it'd be nice if you didn't approach me that way.
The filter is both a security and performance measure. There's plenty of ways to accomplish what you want. @R_J's code is a good first step, so is @hgtonight's idea.
Sorry Linc, it's not you -- "by some in this forum" talks about the forum, not this discussion, so it's definitely not you. Besides, you were professional and respectful and I thank you and everyone who participated in this dialog, so how could have I have aimed this at you? Sometimes humor is dangerous in forums, so that comment with the coup de grace ending was a dangerous response to R_J funny "If this is an idiotic security hole, they'll blame you. That's why I sent you to the front. "
As it is, this my first "play" at github and I deserve the lesson of jumping a pull request. Still not sure how I undo (push?) a pull ;-).
No worries. If you're all done with a pull request, just close it.
closed it.
Thank you all for helping - most valuable!
@R_J @rbrahmson Want to draw your attention to this potential change: https://github.com/vanilla/vanilla/pull/3523
On one hand, it will make this much easier. On the other, it's gonna break anything using it in 2.3.
Choosing any sort order can make the forum run slower if the field is not properly indexed. I remember the restriction was there for reason. If you want to sort by a different field apply an index to it if one doesn't already exist.
grep is your friend.
Thanks, @Linc. I'm already following this discussion full of interest
I've done a full text search through all the plugins I've downloaded and those are around 100. I've found only one plugin using that and the author is me 
So I guess the danger of breaking plugins is 1:100
Generally I'd say that backwards compatibility for edge cases shouldn't hinder new feature development. If the old feature is of less importance it might be canceled completely.
@x00: I totally agree, but I think a framework should be flexible and that includes not making it impossible to write code that's not optimal.
Thanks @linc. I think the change is easy enough that while it is not backward compatible it is an easy plugin code change (easy for me to say...).
As for performance hit risks that were mentioned earlier, I think that in principle it falls on the shoulders of admins and plugin developers. There are endless ways in which a plugin could cause performance hits. Admins too can degrade performance by choosing bad plugin customization options. In my opinion there should not be additional built-in restrictions aside from warnings (plugin description/readme/config screens).
This has evolved even more. I think where we're at now is: we're going to overhaul this entire system and remove all the old behavior. We're removing the 'sort' endpoint, which also absolves us from needing the whitelist since a user can't arbitrarily call a different sort column then. Discussion sorting will be handled by a new module, which will be 100% pluggable.
@Linc - that's great. Now if you could do the same to filtering (by any field) then my FilterDiscussion plugin would become so much simpler