Details

    • Type: Sub-task
    • Status: Closed
    • Priority: Minor
    • Resolution: Fixed
    • Affects Version/s: 2.0
    • Fix Version/s: 2.0
    • Component/s: Portfolio
    • Labels:
      None
    • Affected Branches:
      MOODLE_20_STABLE
    • Fixed Branches:
      MOODLE_20_STABLE

      Description

      The files /portfolio/*.php do not have require_login()
      Those scripts are using $USER and $SESSION, adding require logins and adding forgotten kill switch into portfolio/file.php

      Please review and test

        Gliffy Diagrams

          Attachments

            Activity

            Hide
            skodak Petr Skoda added a comment -

            silly question, why is add.php using optional_param() all over the place?

            It would be also nice if each script contained a short description describing what it does, especially when is uses something from session created by other scripts.

            Show
            skodak Petr Skoda added a comment - silly question, why is add.php using optional_param() all over the place? It would be also nice if each script contained a short description describing what it does, especially when is uses something from session created by other scripts.
            Hide
            mjollnir Penny Leach added a comment -

            Hey Petr, good catch. I'll add docs too.

            What do you want me to review? There's no patch on this bug and nothing in the version control tab?

            I don't understand your comment about optional_param - that isn't deprecated, what's the problem?

            Show
            mjollnir Penny Leach added a comment - Hey Petr, good catch. I'll add docs too. What do you want me to review? There's no patch on this bug and nothing in the version control tab? I don't understand your comment about optional_param - that isn't deprecated, what's the problem?
            Hide
            skodak Petr Skoda added a comment -

            ooops, I must have used some other bug number
            see cvs

            Show
            skodak Petr Skoda added a comment - ooops, I must have used some other bug number see cvs
            Hide
            mjollnir Penny Leach added a comment -

            looks fine.I'll look at the docs as time allows.

            Petr -still waiting on hearing back on the problem with optional_param.

            Show
            mjollnir Penny Leach added a comment - looks fine.I'll look at the docs as time allows. Petr -still waiting on hearing back on the problem with optional_param.
            Hide
            skodak Petr Skoda added a comment -

            from http://docs.moodle.org/en/Development:Coding:

            8. Wherever possible group all your required_param(), optional_param() and other variables initialisation at the beginning of each file to make them easy to find.

            Show
            skodak Petr Skoda added a comment - from http://docs.moodle.org/en/Development:Coding: 8. Wherever possible group all your required_param(), optional_param() and other variables initialisation at the beginning of each file to make them easy to find.
            Hide
            mjollnir Penny Leach added a comment -

            fair enough.

            Show
            mjollnir Penny Leach added a comment - fair enough.
            Hide
            skodak Petr Skoda added a comment -

            I suppose in this case could be separate groups at the start of each case/if

            Show
            skodak Petr Skoda added a comment - I suppose in this case could be separate groups at the start of each case/if
            Hide
            mjollnir Penny Leach added a comment -

            I think I was probably trying to avoid unnecessary variable assignment, but it really doesn't matter. I will just put it all at the top.

            Show
            mjollnir Penny Leach added a comment - I think I was probably trying to avoid unnecessary variable assignment, but it really doesn't matter. I will just put it all at the top.
            Hide
            mjollnir Penny Leach added a comment -

            added docs and moved all optional_param and required_param to the top.

            Show
            mjollnir Penny Leach added a comment - added docs and moved all optional_param and required_param to the top.

              People

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

                Dates

                • Created:
                  Updated:
                  Resolved:
                  Fix Release Date:
                  24/Nov/10