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:

      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"

        Gliffy Diagrams

          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: