Please upgrade here. These earlier versions are no longer being updated and have security issues.

DiscussionModel, CategoriesController, and You in 2.1b1

hgtonighthgtonight ∞ · New Moderator
edited April 2013 in Vanilla 2.0 - 2.8

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?

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.

UnderDog
«13

Comments

  • hgtonighthgtonight ∞ · New Moderator

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

    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.

  • hgtonighthgtonight ∞ · New Moderator

    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?

    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.

    UnderDog
  • rbrahmsonrbrahmson "You may say I'm a dreamer / But I'm not the only one" NY ✭✭✭

    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?

  • hgtonighthgtonight ∞ · New Moderator

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

    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.

  • rbrahmsonrbrahmson "You may say I'm a dreamer / But I'm not the only one" NY ✭✭✭

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

    hgtonight
  • R_JR_J Cheerleader & Troubleshooter Munich Moderator

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


  • LincLinc Senior Bard Detroit Admin

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

  • rbrahmsonrbrahmson "You may say I'm a dreamer / But I'm not the only one" NY ✭✭✭
    edited January 2016

    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.

  • LincLinc Senior Bard Detroit Admin
    edited January 2016

    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.

  • rbrahmsonrbrahmson "You may say I'm a dreamer / But I'm not the only one" NY ✭✭✭

    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_JR_J Cheerleader & Troubleshooter Munich Moderator

    @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


    Shadowdare
  • R_JR_J Cheerleader & Troubleshooter Munich Moderator

    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.


    Shadowdare
  • rbrahmsonrbrahmson "You may say I'm a dreamer / But I'm not the only one" NY ✭✭✭

    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.

  • LincLinc Senior Bard Detroit Admin
    edited January 2016

    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.

  • rbrahmsonrbrahmson "You may say I'm a dreamer / But I'm not the only one" NY ✭✭✭

    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_JR_J Cheerleader & Troubleshooter Munich Moderator

    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.


  • LincLinc Senior Bard Detroit Admin
    edited January 2016

    Am I missing something?

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

    Done. Where's this deficiency?

  • R_JR_J Cheerleader & Troubleshooter Munich Moderator

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


  • rbrahmsonrbrahmson "You may say I'm a dreamer / But I'm not the only one" NY ✭✭✭

    exactly!

  • LincLinc Senior Bard Detroit Admin

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

Sign In or Register to comment.