Uploaded image for project: '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

      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"

        Gliffy Diagrams

          Attachments

            Issue Links

              Activity

              Hide
              rwijaya 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
              rwijaya 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
              jerome Jérôme Mouneyrac added a comment -

              seems good to me +1

              Show
              jerome Jérôme Mouneyrac added a comment - seems good to me +1
              Hide
              rwijaya 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
              rwijaya 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
              rwijaya 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
              rwijaya 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
              rwijaya 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
              rwijaya 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
              tsala Helen Foster added a comment -

              Please see comments in PULL-380.

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

              Looks good to me Rosie

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

              Submitted to pull request: PULL-426

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

              closing, thanks

              Show
              dongsheng Dongsheng Cai added a comment - closing, thanks
              Hide
              tsala 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
              tsala 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:
                    Fix Release Date:
                    5/May/11