Details

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

      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

        Activity

        Hide
        Petr Škoda 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
        Petr Škoda 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
        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
        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
        Petr Škoda added a comment -

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

        Show
        Petr Škoda added a comment - ooops, I must have used some other bug number see cvs
        Hide
        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
        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
        Petr Škoda 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
        Petr Škoda 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
        Penny Leach added a comment -

        fair enough.

        Show
        Penny Leach added a comment - fair enough.
        Hide
        Petr Škoda added a comment -

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

        Show
        Petr Škoda added a comment - I suppose in this case could be separate groups at the start of each case/if
        Hide
        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
        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
        Penny Leach added a comment -

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

        Show
        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: