Skip to content

Conversation

@timyates
Copy link
Contributor

@timyates timyates commented Jun 6, 2024

What?

Adds backend services API for managing certifications.

Should support #2477

I tried to keep as close to the Typescript skeleton as possible.

When running, Certifications swagger API can be seen here and the Earned Certifications swagger API can be seen here

Missing

Questions for future discussion:

  • Permissions and requirements required for these services
  • Who can create, update, delete certifications
  • Who can create, update, delete earned certifications

Certifications

List

GET /services/certification

Create

POST /services/certification

Body should be: com.objectcomputing.checkins.services.certification.CertificationDTO

{
  "name": "string",
  "badgeUrl": "string",
  "active": "boolean"
}

badgeUrl is optional and active is optional and defaults to true

Update

PUT /services/certification/{id}

Body should be: com.objectcomputing.checkins.services.certification.CertificationDTO (as above)

Merge

POST /services/certification/merge

Body should be com.objectcomputing.checkins.services.certification.CertificationMergeDTO

{
  "sourceId": "3fa85f64-5717-4562-b3fc-2c963f66afa6",
  "targetId": "3fa85f64-5717-4562-b3fc-2c963f66afa6"
}

Earned Certifications

List

GET /services/earned-certification{?memberId,certificationId,includeInactive}

Query parameters memberId, certificationId and includeInactive are optional for filtering the response to a certification or member or both, or including earned-certifications for deactivated certifications

Create

POST /services/earned-certification

Body should be com.objectcomputing.checkins.services.certification.EarnedCertificationDTO

{
  "memberId": "3fa85f64-5717-4562-b3fc-2c963f66afa6",
  "certificationId": "3fa85f64-5717-4562-b3fc-2c963f66afa6",
  "description": "string",
  "earnedDate": "2024-06-06",
  "expirationDate": "2024-06-06",
  "certificateImageUrl": "string"
}

expirationDate and certificateImageUrl are optional

Update

PUT /services/earned-certification/{id}

Body should be com.objectcomputing.checkins.services.certification.EarnedCertificationDTO as above

Delete

DELETE /services/earned-certification/{id}

@timyates timyates self-assigned this Jun 6, 2024
@timyates timyates marked this pull request as ready for review June 7, 2024 11:44
@timyates timyates changed the title WIP: Add API for managing certifications and earning them Add API for managing certifications and earning them Jun 7, 2024
@timyates timyates requested review from mkimberlin and mvolkmann June 7, 2024 14:01
@timyates timyates added the server label Jun 7, 2024
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Certification names in the database must be unique, and I added an is_active so we can deactivate certifications when they're no-longer considered relevant. Earned Certifications for deactivated Certifications do not show up in the list by default, but I added a query param to make them appear if required

Copy link
Contributor

@vhscom vhscom left a comment

Choose a reason for hiding this comment

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

LGTM

@timyates
Copy link
Contributor Author

timyates commented Jun 7, 2024

@mvolkmann who should be able to manage certifications and their earning?

I'm guessing we need a permission for administering certifications,

And then the earned certifications can only be managed by the member themselves (plus people with the certification permission above?)

@mkimberlin
Copy link
Member

I was just about to leave a comment to that effect on here, @timyates. I think you are spot on, except I would make a CAN_MANAGE_EARNED_CERTIFICATIONS or similar instead of relying on the permission for managing certification types.

Added `CAN_MANAGE_CERTIFICATIONS` and `CAN_MANAGE_EARNED_CERTIFICATIONS`
@timyates timyates requested a review from mkimberlin June 10, 2024 11:32
@timyates
Copy link
Contributor Author

timyates commented Jun 10, 2024

@mvolkmann @mkimberlin In the latest commit, I added 2 permissions:

CAN_MANAGE_CERTIFICATIONS -- which allows creation and updating of Certifications
CAN_MANAGE_EARNED_CERTIFICATIONS -- which allows creation and updating of EnabledCertifications for ANY user

Without the second permission, users can only create, update and delete EarnedCertifications which they are the owner of (via the memberId field)

I hope this makes sense and is the correct direction 🤔

@timyates
Copy link
Contributor Author

Pushed 0632931 which allows anyone to create a certification

So in the Certificate controller, update and merge require the CAN_MANAGE_CERTIFICATIONS permission

The permissions for earned certifications are handled in the service itself with

private void validatePermission(EarnedCertification earnedCertification, String action) {
// Fail if the user doesn't have permission to modify the earned certification
UUID currentUserId = currentUserServices.getCurrentUser().getId();
boolean hasPermission = rolePermissionServices.findUserPermissions(currentUserId).contains(Permission.CAN_MANAGE_EARNED_CERTIFICATIONS);
validate(!hasPermission && !earnedCertification.getMemberId().equals(currentUserId), "User %s does not have permission to %s Earned Certificate for user %s", currentUserId, action, earnedCertification.getMemberId());
}

@mkimberlin mkimberlin merged commit f001bd0 into develop Jun 11, 2024
@mkimberlin mkimberlin deleted the feature-2446/backend-for-certifications branch June 11, 2024 14:11
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants