Uploaded image for project: 'Moodle'
  1. Moodle
  2. MDL-4781

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

    Details

    • Type: Bug
    • Status: Closed
    • Priority: 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

      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.

        Gliffy Diagrams

          Attachments

            Activity

            Hide
            vyshane 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
            vyshane 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
            tsala 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
            tsala 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
            tsala 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
            tsala 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
            skodak Petr Skoda added a comment -

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

            thanks!

            Show
            skodak Petr Skoda added a comment - I think I fixed this recently, please reopen if you can replicate this... thanks!
            Hide
            tsala 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
            tsala 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
            skodak Petr Skoda 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
            skodak Petr Skoda 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
            skodak Petr Skoda added a comment -

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

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

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

            Show
            tsala Helen Foster added a comment - Just noting another discussion about this problem: http://moodle.org/mod/forum/discuss.php?d=134924
            Hide
            chuckcts 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
            chuckcts 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
            skodak Petr Skoda added a comment -

            reassigning to HQ

            Show
            skodak Petr Skoda added a comment - reassigning to HQ
            Hide
            tsala 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
            tsala 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
            pjfish06 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
            pjfish06 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
            cmurad Chris Murad added a comment -

            I am encountering this Bug on 2.1 as well.

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

            Thanks Chris, also in 2.2.

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

            We are experiencing this on Moodle 2.2 as well.

            Show
            ddyet David Dyet added a comment - We are experiencing this on Moodle 2.2 as well.
            Hide
            cfulton 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
            cfulton 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
            abgreeve 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
            abgreeve 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.. 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.. 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
            stronk7 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
            stronk7 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
            stronk7 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
            stronk7 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
            cfulton Charles Fulton added a comment -

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

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

            Submitting back to integration after being accidentally closed.

            Show
            abgreeve Adrian Greeve added a comment - Submitting back to integration after being accidentally closed.
            Hide
            nebgor 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
            nebgor 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
            samhemelryk 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
            samhemelryk 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 CiBoT added a comment -

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

            Show
            cibot CiBoT added a comment - Moving this reopened issue out from current integration. Please, re-submit it for integration once ready.
            Hide
            cfulton 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
            cfulton 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
            cfulton Charles Fulton added a comment -

            Rebased against 2.4/2.5.

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

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

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

            Thanks Charles, this has been integrated now

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

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

            Show
            rajeshtaneja Rajesh Taneja added a comment - Thanks for fixing this Charles. Post is shortened in site news for admin/teacher/student.
            Hide
            stronk7 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
            stronk7 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:
                  Fix Release Date:
                  14/Jan/13