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

Added optional user profile fields #44

Closed

Conversation

DanielSunami
Copy link

No description provided.

@dmitriim
Copy link
Member

dmitriim commented May 8, 2020

Hi @DanielSunami!
Thank you for working on this patch. As the ocde of the plugin has been changed, you may want to rebase your changes to fix all conflicts.

@@ -614,6 +614,14 @@ public function test_get_request_login_url_user_parameters_based_on_plugin_confi
'lastname' => new external_value(PARAM_NOTAGS, 'The family name of the user', VALUE_OPTIONAL),
'email' => new external_value(PARAM_RAW_TRIMMED, 'A valid and unique email address', VALUE_OPTIONAL),
'username' => new external_value(PARAM_USERNAME, 'A valid and unique username', VALUE_OPTIONAL),

'department' => new external_value(PARAM_TEXT, 'Department of the user', VALUE_OPTIONAL),
Copy link
Member

@dmitriim dmitriim May 8, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Those fields are still hardcoded. Can you please modify the code to be able to provide a list of any fields. An example of that in core could be found here https://github.com/moodle/moodle/blob/master/user/externallib.php#L105

So we would have similar here e.g. "extrafields" multiple structure with "name" and "value". There should be a validation before processing and If we receive an unknown field we should throw an exception.

Standard user fields should be a good start. In a future we can improve it to process custom userfields as well.

Also we would like to see unit tests with any of the code changes.

@dmitriim
Copy link
Member

dmitriim commented Sep 4, 2022

This one has been sitting here not touched for a long time. I'll close it off to clean up a list of PRs. However, opened #85 as I like the idea.
Please come back and resubmit when you have time to work on this feature.

@dmitriim dmitriim closed this Sep 4, 2022
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

Successfully merging this pull request may close these issues.

2 participants