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

Make BSN visible to HR Officer users. #40

Merged
merged 3 commits into from
Mar 21, 2016
Merged

Make BSN visible to HR Officer users. #40

merged 3 commits into from
Mar 21, 2016

Conversation

astirpe
Copy link
Member

@astirpe astirpe commented Mar 3, 2016

For security reasons this should be only visible to HR related roles. Otherwise this will be in violation to the security framework of WBP regarding the protection of persons info.

@pankk
Copy link
Member

pankk commented Mar 8, 2016

One miniscule bug: When having a valid BSN number for a partner, then changing it to an invalid number, save and get the correct warning, finally change it back to the original valid number, you will get a "Another person (Administrator) has the same BSN [...]" warning.

@astirpe
Copy link
Member Author

astirpe commented Mar 8, 2016

Yess it's a bug, I will fix it asap! Or feel free to open a PR. Thank you @pankk !

@hbrunn
Copy link
Member

hbrunn commented Mar 9, 2016

This is not a security measure but only UI. Either actually suppress reading the field as non-privileged user by overriding read or setting the group on the field too, or reword the readme

@hbrunn hbrunn added this to the 8.0 milestone Mar 9, 2016
@astirpe
Copy link
Member Author

astirpe commented Mar 9, 2016

Separate issue #41 opened, to track the bug noticed by @pankk .
Thanks for all the reviews!

@hbrunn
Copy link
Member

hbrunn commented Mar 10, 2016

👍

1 similar comment
@ehahouimohssine
Copy link

👍

hbrunn added a commit that referenced this pull request Mar 21, 2016
@hbrunn hbrunn merged commit 6310159 into OCA:8.0 Mar 21, 2016
@astirpe astirpe deleted the 80_l10n_nl_bsn_fix_hr_security_roles branch March 21, 2016 19:29
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants