Moodle
  1. Moodle
  2. MDL-31959

Some notifications are marked as headings in user profile page

    Details

    • Type: Bug Bug
    • Status: Closed
    • Priority: Minor Minor
    • Resolution: Fixed
    • Affects Version/s: 2.2.4, 2.3.1, 2.4
    • Fix Version/s: 2.2.5, 2.3.2
    • Component/s: General
    • Labels:
    • Testing Instructions:
      Hide

      Login as student and access this url: http://yoursite.for.moodle/user/profile.php?id=## (replace ## with other user id).
      make sure the 'The details of this user are not available to you' message is not in heading tag.

      Modified the user table. set deleted for one of the user. then use that user id for the url above.
      make sure the 'This user account has been deleted' message is not in heading tag.

      Show
      Login as student and access this url: http://yoursite.for.moodle/user/profile.php?id=## (replace ## with other user id). make sure the 'The details of this user are not available to you' message is not in heading tag. Modified the user table. set deleted for one of the user. then use that user id for the url above. make sure the 'This user account has been deleted' message is not in heading tag.
    • Difficulty:
      Easy
    • Affected Branches:
      MOODLE_22_STABLE, MOODLE_23_STABLE, MOODLE_24_STABLE
    • Fixed Branches:
      MOODLE_22_STABLE, MOODLE_23_STABLE
    • Pull from Repository:
    • Pull Master Branch:
    • Rank:
      38621

      Description

      When you for example try to view a user profile that you don't have access the notification below is outputted as heading.
      "The details of this user are not available to you"

      It should probably be a notification. This holds also for message: "This user account has been deleted"

        Activity

        Show
        Juho Viitasalo added a comment - Patch: https://github.com/jiv-e/moodle/commit/c4d92ee07590cd2fe05a7a74645417f80201b447
        Hide
        Michael de Raadt added a comment -

        Thanks for spotting that. Having these notices as notifications will also help with consistency in theming.

        Show
        Michael de Raadt added a comment - Thanks for spotting that. Having these notices as notifications will also help with consistency in theming.
        Hide
        Rossiani Wijaya added a comment -

        Thanks Juho for proving patch for this. It looks good.

        I created patch for 2.2 and 2.3 branches.

        Sending for peer-review.

        Show
        Rossiani Wijaya added a comment - Thanks Juho for proving patch for this. It looks good. I created patch for 2.2 and 2.3 branches. Sending for peer-review.
        Hide
        Rossiani Wijaya added a comment -

        It seems that the master branch is not up to date. I applied the patch to the latest version of master.

        Show
        Rossiani Wijaya added a comment - It seems that the master branch is not up to date. I applied the patch to the latest version of master.
        Hide
        Ankit Agarwal added a comment -

        Hi Rosie,
        This looks good.
        Thanks

        Show
        Ankit Agarwal added a comment - Hi Rosie, This looks good. Thanks
        Hide
        Rossiani Wijaya added a comment -

        Thanks Ankit for reviewing.

        Sending for integration review.

        Show
        Rossiani Wijaya added a comment - Thanks Ankit for reviewing. Sending for integration review.
        Hide
        Eloy Lafuente (stronk7) 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
        Eloy Lafuente (stronk7) 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
        Aparup Banerjee added a comment -

        thanks, that's integrated into 22, 23 and master for testing now.

        Show
        Aparup Banerjee added a comment - thanks, that's integrated into 22, 23 and master for testing now.
        Hide
        Aparup Banerjee added a comment -

        grr, just noting (i only just spotted after integrating) that the commit message here didn't include the MDL-xxx but the merge message above it does. Please lets pay some attention to commit and adding the MDL

        Show
        Aparup Banerjee added a comment - grr, just noting (i only just spotted after integrating) that the commit message here didn't include the MDL-xxx but the merge message above it does. Please lets pay some attention to commit and adding the MDL
        Hide
        Adrian Greeve added a comment -

        Tested on 2.2, 2.3 and master.
        The notification is no longer a header.
        No other problems encountered.
        Test passed.

        Show
        Adrian Greeve added a comment - Tested on 2.2, 2.3 and master. The notification is no longer a header. No other problems encountered. Test passed.
        Hide
        Eloy Lafuente (stronk7) added a comment -

        I'm so proud...of you, many thanks!

        http://youtu.be/n64CdfDRnZY

        Closing as fixed, ciao

        Show
        Eloy Lafuente (stronk7) added a comment - I'm so proud...of you, many thanks! http://youtu.be/n64CdfDRnZY Closing as fixed, ciao

          People

          • Votes:
            0 Vote for this issue
            Watchers:
            1 Start watching this issue

            Dates

            • Created:
              Updated:
              Resolved: