Details

    • Type: Sub-task Sub-task
    • Status: Closed
    • Priority: Minor Minor
    • Resolution: Fixed
    • Affects Version/s: 2.3
    • Fix Version/s: 2.3
    • Component/s: Book
    • Labels:
    • Testing Instructions:
      Hide
      1. Set book | numbering config to numbers from admin settings
      2. Create a book resource
      3. Add at-least 3 chapters and one sub chapter(to the second chapter)
      4. hide the second chapter and the sub-chapter
      5. Make sure the numbering in the navigation block and in the content are are as follows
      • x for the chapter
      • x.x for the sub chapter
      1. Enable the second chapter and hide the sub-chapter
      2. Make sure the numbering in the navigation block and in the content are are as follows
      • 2 for the chapter
      • 2.x for the sub chapter
      Show
      Set book | numbering config to numbers from admin settings Create a book resource Add at-least 3 chapters and one sub chapter(to the second chapter) hide the second chapter and the sub-chapter Make sure the numbering in the navigation block and in the content are are as follows x for the chapter x.x for the sub chapter Enable the second chapter and hide the sub-chapter Make sure the numbering in the navigation block and in the content are are as follows 2 for the chapter 2.x for the sub chapter
    • Affected Branches:
      MOODLE_23_STABLE
    • Fixed Branches:
      MOODLE_23_STABLE
    • Pull Master Branch:
      MDL-33123-master
    • Rank:
      40389

      Description

      If you create a chapter and then a sub-chapter and mark the sub-chapter as hidden, its numbered is inconsistent.

      In the side navigation it appears as x.x whereas in the content area it is numbered as 1.x (assuming 1 is the number assigned to the parent chapter).

      It should be made consistent.

      Replication steps:-

      1. Create a book resource
      2. Add a chapter
      3. Add a sub-chapter
      4. Mark sub-chapter as hidden
      5. Click on the sub-chapter name from navigation

      Expected:-
      The sub-chapter is numbered consistently both in navigation and content area.

      Actual:-
      The sub-chapter is numbered as x.x in navigation and 1.x in the content area.

        Activity

        Hide
        Michael de Raadt added a comment -

        Hi, Ankit.

        Could you please add some replication steps for this?

        Show
        Michael de Raadt added a comment - Hi, Ankit. Could you please add some replication steps for this?
        Hide
        Ankit Agarwal added a comment - - edited

        Sending this for peer-review
        Thanks

        Show
        Ankit Agarwal added a comment - - edited Sending this for peer-review Thanks
        Hide
        Rossiani Wijaya added a comment -

        Hi Ankit,

        The patch looks good.

        However, I think it would be better to display the hidden sub-chapters as 1.x and when both chapter and sub-chapter are hidden it displays as x.x.

        You might want to take a look book_get_toc() and ask other's opinion.

        Show
        Rossiani Wijaya added a comment - Hi Ankit, The patch looks good. However, I think it would be better to display the hidden sub-chapters as 1.x and when both chapter and sub-chapter are hidden it displays as x.x. You might want to take a look book_get_toc() and ask other's opinion.
        Hide
        Ankit Agarwal added a comment -

        Thanks for the review Rosie,
        I will try to get petr's opinion on this.

        book_get_toc() uses "x.x $title" for this at the moment. I have used the same in the content area in the current patch.
        Thanks

        Show
        Ankit Agarwal added a comment - Thanks for the review Rosie, I will try to get petr's opinion on this. book_get_toc() uses "x.x $title" for this at the moment. I have used the same in the content area in the current patch. Thanks
        Hide
        Ankit Agarwal added a comment -

        I have changed code to display 1.X for hidden sub-chapters.
        Requesting another review.

        Show
        Ankit Agarwal added a comment - I have changed code to display 1.X for hidden sub-chapters. Requesting another review.
        Hide
        Rossiani Wijaya added a comment -

        Hi Ankit,

        Just a minor issue to fix the spacing for the if statement.

        Other than that, it looks great.

        Show
        Rossiani Wijaya added a comment - Hi Ankit, Just a minor issue to fix the spacing for the if statement. Other than that, it looks great.
        Hide
        Ankit Agarwal added a comment -

        Thanks for the review Rosie.
        updated the patch sending for integration.
        Thanks

        Show
        Ankit Agarwal added a comment - Thanks for the review Rosie. updated the patch sending for integration. Thanks
        Hide
        Eloy Lafuente (stronk7) added a comment -

        Integrated, thanks!

        Show
        Eloy Lafuente (stronk7) added a comment - Integrated, thanks!
        Hide
        Tim Barker added a comment -

        Tested as per testing instructions and passed.

        Show
        Tim Barker added a comment - Tested as per testing instructions and passed.
        Hide
        Eloy Lafuente (stronk7) added a comment -

        Big thanks for the effort. This is now part of Moodle upstream. Let's wait for regressions, yay! LOL

        Ciao

        Show
        Eloy Lafuente (stronk7) added a comment - Big thanks for the effort. This is now part of Moodle upstream. Let's wait for regressions, yay! LOL Ciao

          People

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

            Dates

            • Created:
              Updated:
              Resolved: