Moodle
  1. Moodle
  2. MDL-4781

Site news - forum length setting has no effect for certain users

    Details

    • Type: Bug Bug
    • Status: Closed
    • Priority: Major Major
    • Resolution: Fixed
    • Affects Version/s: 1.5.3, 1.9, 2.0.1, 2.2
    • Fix Version/s: 2.2.7, 2.3.4, 2.4.1
    • Component/s: Forum
    • Labels:
    • Environment:
      All
    • Database:
      MySQL
    • Testing Instructions:
      Hide

      1. As an administrator, create a very long post on the site news forum.
      2. Login as a different role (student, teacher).
      3. Verify that the post is shortened.

      Show
      1. As an administrator, create a very long post on the site news forum. 2. Login as a different role (student, teacher). 3. Verify that the post is shortened.
    • Affected Branches:
      MOODLE_15_STABLE, MOODLE_19_STABLE, MOODLE_20_STABLE, MOODLE_22_STABLE
    • Fixed Branches:
      MOODLE_22_STABLE, MOODLE_23_STABLE, MOODLE_24_STABLE
    • Pull from Repository:
    • Pull Master Branch:
      MDL-4781-master
    • Rank:
      485

      Description

      Please see http://moodle.org/mod/forum/discuss.php?d=39697 and http://moodle.org/mod/forum/discuss.php?d=36918

      This makes it very messy to use news as the option for the front page.

      No difference if viewed in i.e. or firefox.

        Activity

        Hide
        Vy-Shane Sin Fat added a comment -

        I could not reproduce this with the latest 1.5, nor with the latest 1.6. This is an old bug. Maybe it's already been fixed?

        Show
        Vy-Shane Sin Fat added a comment - I could not reproduce this with the latest 1.5, nor with the latest 1.6. This is an old bug. Maybe it's already been fixed?
        Hide
        Helen Foster added a comment -

        Reopening this issue, as it remains a problem in Moodle 1.9.

        See also http://moodle.org/mod/forum/discuss.php?d=94543

        Show
        Helen Foster added a comment - Reopening this issue, as it remains a problem in Moodle 1.9. See also http://moodle.org/mod/forum/discuss.php?d=94543
        Hide
        Helen Foster added a comment -

        Just tested with latest 1.9+, and found that, with defaultfrontpageroleid set to student, an admin sees the reduced length site news item, whereas a student sees the full length item.

        Show
        Helen Foster added a comment - Just tested with latest 1.9+, and found that, with defaultfrontpageroleid set to student, an admin sees the reduced length site news item, whereas a student sees the full length item.
        Hide
        Petr Škoda added a comment -

        I think I fixed this recently, please reopen if you can replicate this...

        thanks!

        Show
        Petr Škoda added a comment - I think I fixed this recently, please reopen if you can replicate this... thanks!
        Hide
        Helen Foster added a comment -

        Reopening, as the problem remains in the latest 1.9+. Also, if the front page is set to display news items, then non-logged in users also see the full-length site news item.

        Show
        Helen Foster added a comment - Reopening, as the problem remains in the latest 1.9+. Also, if the front page is set to display news items, then non-logged in users also see the full-length site news item.
        Hide
        Petr Škoda added a comment -

        I have checked the code, the problem is that forum_shorten_post($message) uses a very simplified algorithm that does not work in many cases - it is not unicode aware at all.

        Solution would be to rewrite it completely and use tidyhtml or htmlpurifier, in anycase this will require a lot of work

        Show
        Petr Škoda added a comment - I have checked the code, the problem is that forum_shorten_post($message) uses a very simplified algorithm that does not work in many cases - it is not unicode aware at all. Solution would be to rewrite it completely and use tidyhtml or htmlpurifier, in anycase this will require a lot of work
        Hide
        Petr Škoda added a comment -

        hmm, maybe we could do some shortening using javascrip instead - like a small +/- icon at the bottom of longer posts...

        Show
        Petr Škoda added a comment - hmm, maybe we could do some shortening using javascrip instead - like a small +/- icon at the bottom of longer posts...
        Hide
        Helen Foster added a comment -

        Just noting another discussion about this problem:
        http://moodle.org/mod/forum/discuss.php?d=134924

        Show
        Helen Foster added a comment - Just noting another discussion about this problem: http://moodle.org/mod/forum/discuss.php?d=134924
        Hide
        Dušan Ristić added a comment -

        Using Moodle 1.9.9+ build 20100908

        Just hit this problem today, users that aren't registered/loged in are shown full text instead of short version with link.

        Made a fix and still not seeing any side effects. In moodle/mod/forum/lib.php line 4939 should look like:

        $link = forum_user_can_post($forum, $discussion, $USER, $cm, $course, $modcontext);

        I've just replaced it with:

        $link = forum_user_can_see_post($forum, $discussion, $post, $USER, $cm);

        It doesn't seems to be afecting other controls (edit/delete).

        Show
        Dušan Ristić added a comment - Using Moodle 1.9.9+ build 20100908 Just hit this problem today, users that aren't registered/loged in are shown full text instead of short version with link. Made a fix and still not seeing any side effects. In moodle/mod/forum/lib.php line 4939 should look like: $link = forum_user_can_post($forum, $discussion, $USER, $cm, $course, $modcontext); I've just replaced it with: $link = forum_user_can_see_post($forum, $discussion, $post, $USER, $cm); It doesn't seems to be afecting other controls (edit/delete).
        Hide
        Petr Škoda added a comment -

        reassigning to HQ

        Show
        Petr Škoda added a comment - reassigning to HQ
        Hide
        Helen Foster added a comment -

        Just checked and found 2.0.1 still affected by this issue. Only admins see shorted news forum posts. Non-logged in users, students and guests all see the full-length post.

        Show
        Helen Foster added a comment - Just checked and found 2.0.1 still affected by this issue. Only admins see shorted news forum posts. Non-logged in users, students and guests all see the full-length post.
        Hide
        Susan Mangan added a comment -

        I guess our posts haven't been incredibly long until now as I JUST noticed this bug!! ... after years of using Moodle. crazy. I log in as Admin to post messages on the front page and it always shortens so I never doubted it!!! Anyway - fyi another potential workaround that may or may not work for you is to post your full entry as a Site Event and hyperlink to it from the News Forum where you can post a shorter blurb there.

        Show
        Susan Mangan added a comment - I guess our posts haven't been incredibly long until now as I JUST noticed this bug!! ... after years of using Moodle. crazy. I log in as Admin to post messages on the front page and it always shortens so I never doubted it!!! Anyway - fyi another potential workaround that may or may not work for you is to post your full entry as a Site Event and hyperlink to it from the News Forum where you can post a shorter blurb there.
        Hide
        Chris Murad added a comment -

        I am encountering this Bug on 2.1 as well.

        Show
        Chris Murad added a comment - I am encountering this Bug on 2.1 as well.
        Hide
        Helen Foster added a comment -

        Thanks Chris, also in 2.2.

        Show
        Helen Foster added a comment - Thanks Chris, also in 2.2.
        Hide
        David Dyet added a comment -

        We are experiencing this on Moodle 2.2 as well.

        Show
        David Dyet added a comment - We are experiencing this on Moodle 2.2 as well.
        Hide
        Charles Fulton added a comment -

        Duan's proposed patch for 1.9.9 works with only minor tweaks. Is there a use case for forum_print_latest_discussions() not setting $link to true in all cases? It will anyway if there's even one reply, regardless of the user's ability to reply.

        Show
        Charles Fulton added a comment - Duan's proposed patch for 1.9.9 works with only minor tweaks. Is there a use case for forum_print_latest_discussions() not setting $link to true in all cases? It will anyway if there's even one reply, regardless of the user's ability to reply.
        Hide
        Adrian Greeve added a comment -

        Hi Charles,

        I think that the alteration is ok. It definitely fixes the problem with posts on the front page not being shortened when long. I was concerned about the problem that Petr mentioned about not being unicode aware and with a bit of testing I noticed that it doesn't handle some languages all that well. I think that might be out of the scope of this issue, so I'm happy for it to move to integration.

        Show
        Adrian Greeve added a comment - Hi Charles, I think that the alteration is ok. It definitely fixes the problem with posts on the front page not being shortened when long. I was concerned about the problem that Petr mentioned about not being unicode aware and with a bit of testing I noticed that it doesn't handle some languages all that well. I think that might be out of the scope of this issue, so I'm happy for it to move to integration.
        Hide
        Per Hessellund Laursen added a comment -

        In Moodle 2.2. we had the problem with no effect for certain users. After opening the forum for logged in users reply, the forum respect the length setting for all logged in users

        Show
        Per Hessellund Laursen added a comment - In Moodle 2.2. we had the problem with no effect for certain users. After opening the forum for logged in users reply, the forum respect the length setting for all logged in users
        Hide
        Eloy Lafuente (stronk7) added a comment -

        U P S T R E A M I Z E D !

        Many thanks, this is now available in all the repos (git & cvs).

        Closing, ciao

        Show
        Eloy Lafuente (stronk7) added a comment - U P S T R E A M I Z E D ! Many thanks, this is now available in all the repos (git & cvs). Closing, ciao
        Hide
        Eloy Lafuente (stronk7) added a comment -

        Doh,

        somehow this issue was closed incorrectly when processing all the integrated issues this week. (sort of most voted and current in integration filters mix). Apologies for the confusion, reseting to previous status!

        Ciao, Eloy

        Show
        Eloy Lafuente (stronk7) added a comment - Doh, somehow this issue was closed incorrectly when processing all the integrated issues this week. (sort of most voted and current in integration filters mix). Apologies for the confusion, reseting to previous status! Ciao, Eloy
        Hide
        Charles Fulton added a comment -

        Hi Adrian, could you push this to integration? Thanks, Charles.

        Show
        Charles Fulton added a comment - Hi Adrian, could you push this to integration? Thanks, Charles.
        Hide
        Adrian Greeve added a comment -

        Submitting back to integration after being accidentally closed.

        Show
        Adrian Greeve added a comment - Submitting back to integration after being accidentally closed.
        Hide
        Aparup Banerjee added a comment -

        The main moodle.git repository has just been updated with latest weekly modifications. You may wish to rebase your PULL branches to simplify history and avoid any possible merge conflicts. This would also make integrator's life easier next week.

        TIA and ciao

        Show
        Aparup Banerjee added a comment - The main moodle.git repository has just been updated with latest weekly modifications. You may wish to rebase your PULL branches to simplify history and avoid any possible merge conflicts. This would also make integrator's life easier next week. TIA and ciao
        Hide
        Sam Hemelryk added a comment -

        Hi Charles,

        I'm sending this back sorry.
        The reason being an incorrect function call.
        The use of $discussion as the third arg to forum_user_can_see_post. Looking at the phpdocs for forum_user_can_see_post the third arg should be the post that we are checking the users ability to view.

        In general if we see this sort of thing popping up we send it back even if we are 100% sure that at the time of submission it is working correctly. Off the top of my head there are two good reasons for this:

        1. The function expects it to be a post. Should the function evolve and interaction with the param change there is a very high chance of regressions and importantly not regressions with the function but with the calling code.
        2. The function trusts it is a post object. PHP passes all objects by reference, as part of the functions operation it may in fact modify the object [by reference]. That object may end up pretty messed up and may cause untold trouble if passed to further API functions.

        In general as well if you have to trick a function or pass fake vars then its best to take a close look at what you're trying to achieve and see what can be done.
        In this case I think perhaps its an easy solution, there is a function forum_user_can_see_discussion (directly above forum_user_can_see_post in forum/lib.php) that I imagine is doing what you want?

        Many thanks, and sorry for the fuss.
        Sam

        Show
        Sam Hemelryk added a comment - Hi Charles, I'm sending this back sorry. The reason being an incorrect function call. The use of $discussion as the third arg to forum_user_can_see_post. Looking at the phpdocs for forum_user_can_see_post the third arg should be the post that we are checking the users ability to view. In general if we see this sort of thing popping up we send it back even if we are 100% sure that at the time of submission it is working correctly. Off the top of my head there are two good reasons for this: The function expects it to be a post. Should the function evolve and interaction with the param change there is a very high chance of regressions and importantly not regressions with the function but with the calling code. The function trusts it is a post object. PHP passes all objects by reference, as part of the functions operation it may in fact modify the object [by reference] . That object may end up pretty messed up and may cause untold trouble if passed to further API functions. In general as well if you have to trick a function or pass fake vars then its best to take a close look at what you're trying to achieve and see what can be done. In this case I think perhaps its an easy solution, there is a function forum_user_can_see_discussion (directly above forum_user_can_see_post in forum/lib.php) that I imagine is doing what you want? Many thanks, and sorry for the fuss. Sam
        Hide
        CiBoT added a comment -

        Moving this reopened issue out from current integration. Please, re-submit it for integration once ready.

        Show
        CiBoT added a comment - Moving this reopened issue out from current integration. Please, re-submit it for integration once ready.
        Hide
        Charles Fulton added a comment -

        Thanks Sam for that detailed feedback--you're absolutely right and I feel silly for not even looking for that function. Rebased and updated.

        Show
        Charles Fulton added a comment - Thanks Sam for that detailed feedback--you're absolutely right and I feel silly for not even looking for that function. Rebased and updated.
        Hide
        Charles Fulton added a comment -

        Rebased against 2.4/2.5.

        Show
        Charles Fulton added a comment - Rebased against 2.4/2.5.
        Hide
        Adrian Greeve added a comment -

        Everything looks fine here. I'll push it back to integration.

        Show
        Adrian Greeve added a comment - Everything looks fine here. I'll push it back to integration.
        Hide
        Sam Hemelryk added a comment -

        Thanks Charles, this has been integrated now

        Show
        Sam Hemelryk added a comment - Thanks Charles, this has been integrated now
        Hide
        Rajesh Taneja added a comment -

        Thanks for fixing this Charles.
        Post is shortened in site news for admin/teacher/student.

        Show
        Rajesh Taneja added a comment - Thanks for fixing this Charles. Post is shortened in site news for admin/teacher/student.
        Hide
        Eloy Lafuente (stronk7) added a comment -

        And your fantastic code has met core, hope they become good friends for a long period.

        Closing, thanks!

        Show
        Eloy Lafuente (stronk7) added a comment - And your fantastic code has met core, hope they become good friends for a long period. Closing, thanks!

          People

          • Votes:
            23 Vote for this issue
            Watchers:
            19 Start watching this issue

            Dates

            • Created:
              Updated:
              Resolved: