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

Menambahkan kode untuk menghandle akun dengan > 1 role/kode_org #11

Open
wants to merge 3 commits into
base: master
Choose a base branch
from

Conversation

imiro
Copy link

@imiro imiro commented Aug 9, 2019

Solusi sederhana untuk issue #10
Mungkin perlu dipikirkan adanya kemungkinan bahwa bukan hanya mahasiswa yang dapat memiliki >1 role?

@albadrun
Copy link

albadrun commented Aug 23, 2019

oke, di-review dulu ya, sorry agak lama, hehe
@indravb6 boleh tolong bantu ga?

Copy link

@fatanugraha fatanugraha left a comment

Choose a reason for hiding this comment

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

might want to wait for other mantainer's input

SSO/SSO.php Outdated
$user->study_program = $data['study_program'];
$user->educational_program = $data['educational_program'];
} else {
$user->faculty = array();

Choose a reason for hiding this comment

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

We might want to keep this property's type consistent to make sure existing apps that use this library did not break.

One that i can suggest is keep faculty, study_program, and educational_program to contain the first role, and put the others to another property.

Choose a reason for hiding this comment

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

agree with this

When more than 1  `kd_org` is present for a single user, 
additional faculty, study_program, and educational_program are 
stored in properties named after their plural forms, as arrays.

This is done to ensure type consistency of existing properties.
@imiro
Copy link
Author

imiro commented Jul 6, 2020

Hi,

Sorry for letting this off for months. Thank you for the feedback, added commit 13667ee to address this issue.

Add Sub Specialist Program FK (ristekoss#9)
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.

3 participants