Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

capability management #1

Open
ak4t0sh opened this issue May 15, 2019 · 0 comments
Open

capability management #1

ak4t0sh opened this issue May 15, 2019 · 0 comments

Comments

@ak4t0sh
Copy link

ak4t0sh commented May 15, 2019

Implementing function such as is_allowed_to_display_students() add complexity to your code.
It should be replace by

$hassiteconfig = has_capability('moodle/site:config', $context);
foreach ($mycourses as $course) {
    $coursecontext = context_course::instance($course->id);
    if ($hassiteconfig || has_capability('moodle/course:update', $coursecontext)) {
    ...
    }
}

which do the same thing but is more performance efficient (check for moodle/site:config is done only once) and more straightforward to understand for reviewers :)
It also remove the global $context usage which is against Moodle coding guidelines. Global should by use only for moodle core global variables.

Optional (non blocking for approval) :
I think that instead of using moodle/course:update you should implements your own capability. This way you could use this plugin on role which do not have the permission to edit the course (such as tutor for example)

cellule-tice pushed a commit that referenced this issue May 16, 2019
An optiobnal improvment is also done : link to user profile in display
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

1 participant