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

Local settings #2

Open
Kemmotar83 opened this issue Oct 24, 2016 · 9 comments
Open

Local settings #2

Kemmotar83 opened this issue Oct 24, 2016 · 9 comments

Comments

@Kemmotar83
Copy link

Hi Alexander,

we use this plugin in our university and I really appreciate what it does.

Now we need a new functionality, the possibility to have a local customization of roles listed in a specific block instance (i.e. local customization of global course contacts used by default).

If you want I can develop this new feature.

@abias
Copy link
Member

abias commented Oct 24, 2016

Hi Giorgio,

great to hear that you like our plugin.

Currently, the block has no settings (neither for admins nor for the block instances) because it pull every setting from other sources - I think you know that if you have read https://github.com/moodleuulm/moodle-block_people#user-content-settings.

However, I think your proposal is useful and feasible. You would need:

  • to add a capability to the block which controls if a user is allowed to override the roles to be displayed on a certain block instance.
  • to add a settings.php file which contains
    • a setting to set the default roles to be shown in the block (with defaults pulled from $CFG->coursecontact)
    • a setting to set the available roles to be shown in the block (with defaults pulled from $CFG->coursecontact)
  • to add a edit_form.php file which contains
    • a setting to override the default roles and to set an individual role set from the set of available roles (which is only available for users who have the newly created capability)
  • to update the logic in block_people.php to use the configured roles instead of $CFG->coursecontact
  • to update the instructions in the README.md file
  • to update version.php

Unfortunately, we don't have the need for this feature here and can't spend resources on implementing this feature. But as it can be implemented with full backwards-compatibility, I would be happy to integrate it and maintain it in the long run if you send me a pull request.

Thanks,
Alex

@Kemmotar83
Copy link
Author

Ok Alexander,
as I said, it's not a problem if you can't manage to implement the feature, I'll do it myself and then I'll pull the changes.

I thought to introduce only local settings but it's ok also to have global plugin settings, so let me understand better what you wrote.

Is the second global plugin setting (available roles) intended to restrict the list of roles available for the local override? What's if the available roles have different roles selected than global default roles and one of these is selected as a default one, what will happen when I have to override these roles locally?

Thanks,
Giorgio

@abias
Copy link
Member

abias commented Oct 24, 2016

Hi Giorgio,

Is the second global plugin setting (available roles) intended to restrict the list of roles available for the local override?

Exactly. We have more roles here in the system than we would want our teachers to show in the people block. And the setting would prevent mistakes - imagine a teacher added the "student" role to the people block in 500+ student course would not make much sense.

What's if the available roles have different roles selected than global default roles and one of these is selected as a default one, what will happen when I have to override these roles locally?

I am sorry, but I don't fully understand what you are asking. Could you give an example?

Thanks,
Alex

@Kemmotar83
Copy link
Author

Ok, I'll give you an example.

Global plugin settings:

  • default roles to display: Manager, Teacher and Non-Editing Teacher
  • roles available for local override: Teacher and Non-Editing Teacher

Local instance settings:

  • roles to display: Teacher

Now what are we displaying? Note that locally we can't unselect role Manager because it is unavailable.
There are two solutions:

  • mixing global and local settings, so we'll display roles Manager and Teacher,
  • consider only local settings, so we'll display only role Teacher
    Personally I think that the first one is the right solution but I wanna see what's your opinion.

Other than that, I would also add some flags to specific express the intention to override default settings (also useful to reset to default) and to set if we want to allow local overrides.

In conclusion, let's write down the global and local settings.

Global:

  • override course contacts (configcheckbox - default: no)
  • default roles to display (configmultiselect) - available only if the previous one is selected
  • allow local overrides (configcheckbox - default: no)
  • available roles for local override (configmultiselect) - available only if the previous one is selected

Local:

  • override default settings (configcheckbox - default: no)
  • default roles to display (configmultiselect) - available only if the previous one is selected

Thanks,
Giorgio

p.s. Sorry, I'm writing too much! :P

@abias
Copy link
Member

abias commented Jul 17, 2018

Hi Giorgio,

I am really sorry that I didn't touch this issue and your corresponding pull request anymore for 1.5 years now.

In the meantime, the plugin was updated to Moodle 3.5 and I have added myself the possibility to define the roles to show in the block.

If you are still interested to see per-instance setting in this block, I would be grateful if you could update your pul request and I will do my best to merge it in reasonable time.

Thanks for your patience,
Alex

@Kemmotar83
Copy link
Author

Hi Alexander,

I was giving up on this feature and, in the last days, I started to work on a fork for my university, since we are working on the 3.5 update for this summer.

That said, for me it's a pleasure to contribute so I'll try to create a patch for your last version. It should not be a problem.

Thanks,
Giorgio

@abias
Copy link
Member

abias commented Jul 18, 2018

Hi Giorgio,

I can only repeat that I am sorry for the huge delay. University business has kept me from finishing this issue.

As the plugin is already ready for 3.5, I hope that we can finish it now and prevent that you must fork the plugin which would be a pity.

If there are any other things which you would have changed in your fork, don't hesitate to create new issues for those things.

Thanks,
Alex

@Kemmotar83
Copy link
Author

Don't worry. I understand you may not have had time to work on it.

In our fork (only in development) I've just replicated the features listed before.
So, since you have already added the roles setting, I would add the local config feature.

  • Global settings
    • override course contacts (configcheckbox - default: no) - I would let users to choose if use the Course Contacts instead of the global config
    • allow local overrides (configcheckbox - default: no)
    • available roles for local override (admin_setting_pickroles) - available only if the previous one is selected
  • Local settings:
    • override default settings (configcheckbox - default: no) - available only if global config allow local overrides is enabled
    • default roles to display (configmultiselect or configmulticheckbox) - available only if the previous one is selected

Is that ok for you?

Giorgio

@abias
Copy link
Member

abias commented Jul 24, 2018

Hi Giorgio,

thanks for your pull request in #5. I have added some comments there and would propose to continue the discussion in the pull request.

Thanks,
Alex

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

2 participants