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

feat: sgID support for pocdex.public_officer_details #666

Merged
merged 1 commit into from
May 24, 2024

Conversation

cflee
Copy link
Contributor

@cflee cflee commented May 21, 2024

Problem

Mockpass currently doesn't support sgID clients requesting for the pocdex.public_officer_details scope, "NA" is always returned.

(It also doesn't support pocdex.number_of_employments, but I don't seem to receive that value from sgID, so I don't want to blindly emulate it without knowing what the real values are.)

Solution

Features:

  • Supporting adding an publicofficerdetails array to personas to serve those details to sgID clients asking for pocdex.public_officer_details

Examples

Example of info to add to persona:

"publicofficerdetails": [
  {
    "work_email": "[email protected]",
    "agency_name": "Work Allocation Singapore",
    "department_name": "Allocation Central",
    "employment_type": "Fixed Term",
    "employment_title": "Senior Software Engineer - LLv1 (Individual Contributor) (WAS)"
  }
]

Example response from sgid client library's userinfo output, it now returns the string for public officer details with an array, instead of the string NA:

{
  sub: 'u=952b0342-0649-a6fe-245b-87cfcc3d38da',
  data: {
    'myinfo.name': 'LIM YONG XIANG',
    'pocdex.public_officer_details': '[{"work_email":"[email protected]","agency_name":"Work Allocation Singapore","department_name":"Allocation Central","employment_type":"Fixed Term","employment_title":"Senior Software Engineer - LLv1 (Individual Contributor) (WAS)"}]',
    'myinfo.nric_number': 'S9812379B'
  }
}

@LoneRifle
Copy link
Collaborator

thanks for this @cflee ! Could we have some reference to that in the README please? myinfo/v3.json is pretty difficult to read, so we should ensure that users are aware of the sole pocdex entry

@cflee
Copy link
Contributor Author

cflee commented May 21, 2024

I tried to update it, though I'm not sure if it could be more succinct.

I just realised that I forgot to highlight/ask in the PR message – I went ahead to add modify one entry persona only, but are we ok to modify an existing entry persona, or would we prefer to get a new entry persona entirely? Though I would think that it's really unlikely anyone is using the existing personas as-is and also relying on them not being a public officer, so it should be fine.

Copy link
Collaborator

@LoneRifle LoneRifle left a comment

Choose a reason for hiding this comment

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

yea that's fine

@LoneRifle LoneRifle merged commit e79d0bc into opengovsg:main May 24, 2024
2 checks passed
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