Moodle
  1. Moodle
  2. MDL-26464

Add entry option is still displayed into settings block even though the maximum entry number is reached

    Details

    • Affected Branches:
      MOODLE_20_STABLE
    • Fixed Branches:
      MOODLE_20_STABLE
    • Rank:
      16182

      Description

      1. Login as a teacher, update a database activity and set the maximum entries to 2.
      2. Login as a student and add 2 entries. The add entry tab is removed but the Settings block still display "Add entry"

        Issue Links

          Activity

          Hide
          Rossiani Wijaya added a comment -

          Create patch to address the issue.
          patch: https://github.com/rwijaya/moodle/compare/master...MDL-26464_m20

          Hi Jerome,
          when you have a chance, could you take a look the patch and let me know if it needs any changes.

          Thanks
          Rosie

          Show
          Rossiani Wijaya added a comment - Create patch to address the issue. patch: https://github.com/rwijaya/moodle/compare/master...MDL-26464_m20 Hi Jerome, when you have a chance, could you take a look the patch and let me know if it needs any changes. Thanks Rosie
          Hide
          Jérôme Mouneyrac added a comment -

          seems good to me +1

          Show
          Jérôme Mouneyrac added a comment - seems good to me +1
          Hide
          Rossiani Wijaya added a comment -

          Additional note: user with mod/data:manageentries capability (eg: teacher) should not be subject to the same limit.

          submitted to pull request: PULL-380

          Show
          Rossiani Wijaya added a comment - Additional note: user with mod/data:manageentries capability (eg: teacher) should not be subject to the same limit. submitted to pull request: PULL-380
          Hide
          Rossiani Wijaya added a comment -

          Petr's rejection comment:
          1/ data_user_can_add_entry() does unnecessary DB access, I suppose we could easily fix that by changing the API there
          2/ when doing ifs always put cheaper condition first - if ("DB access" || has_capability(...)) does extra DB access when you actually have the capability

          There are multiple 2/ in current code base, but I believe we should not be adding more of these.

          Also Eloy pointed out that the data_user_can_add_entry() should be probably doing the max entry tests...

          Show
          Rossiani Wijaya added a comment - Petr's rejection comment: 1/ data_user_can_add_entry() does unnecessary DB access, I suppose we could easily fix that by changing the API there 2/ when doing ifs always put cheaper condition first - if ("DB access" || has_capability(...)) does extra DB access when you actually have the capability There are multiple 2/ in current code base, but I believe we should not be adding more of these. Also Eloy pointed out that the data_user_can_add_entry() should be probably doing the max entry tests...
          Hide
          Rossiani Wijaya added a comment -

          Adding Sam to watcher.

          Sam,
          When you have a chance, could you take a look the latest patch?
          diff url: https://github.com/rwijaya/moodle/compare/master...MDL-26464_m20_v2

          Thanks
          Rosie

          Show
          Rossiani Wijaya added a comment - Adding Sam to watcher. Sam, When you have a chance, could you take a look the latest patch? diff url: https://github.com/rwijaya/moodle/compare/master...MDL-26464_m20_v2 Thanks Rosie
          Hide
          Helen Foster added a comment -

          Please see comments in PULL-380.

          Show
          Helen Foster added a comment - Please see comments in PULL-380.
          Hide
          Sam Hemelryk added a comment -

          Looks good to me Rosie

          Show
          Sam Hemelryk added a comment - Looks good to me Rosie
          Hide
          Rossiani Wijaya added a comment -

          Submitted to pull request: PULL-426

          Show
          Rossiani Wijaya added a comment - Submitted to pull request: PULL-426
          Hide
          Dongsheng Cai added a comment -

          closing, thanks

          Show
          Dongsheng Cai added a comment - closing, thanks
          Hide
          Helen Foster added a comment -

          Reopening so that a 2.0.3 fix version can be set. Also, let's wait until the weekly package is available before closing. Apologies for the extra notification emails.

          Show
          Helen Foster added a comment - Reopening so that a 2.0.3 fix version can be set. Also, let's wait until the weekly package is available before closing. Apologies for the extra notification emails.

            People

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

              Dates

              • Created:
                Updated:
                Resolved: