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] Backend for employee profile #12334

Merged
merged 17 commits into from
Dec 31, 2024
Merged

Conversation

esizer
Copy link
Member

@esizer esizer commented Dec 18, 2024

🤖 Resolves #12001

👋 Introduction

Adds the EmployeeProfile and all supporting code to the backend.

🧪 Testing

  1. Confirm matches requirements outlined in the ticket
  2. Refresh DB to get an employee profile on applicant@test.com by running make seed-fresh
  3. Open graphiql /graphiql
  4. Make sure you are authenticated as applicant@test.com (from env or bearer token)
  5. Query for your own users employee profile
  6. Confirm it returns as expected
  7. Query for another users employee profile
  8. Confirm unauthorized
  9. Repeat steps 5-8 but with the update mutation

@esizer esizer marked this pull request as ready for review December 19, 2024 14:15
@mnigh mnigh self-requested a review December 23, 2024 15:30
@brindasasi brindasasi self-requested a review December 23, 2024 16:37
mnigh

This comment was marked as outdated.

@esizer
Copy link
Member Author

esizer commented Dec 27, 2024

Is there a step I am missing?

Nope, I made a copy past error. Fixed in fix graphql input for employee profile

@mnigh mnigh self-requested a review December 27, 2024 16:33
@mnigh
Copy link
Contributor

mnigh commented Dec 27, 2024

Having trouble with returning any values (this after having run make refresh-api; make seed-fresh; pnpm i; pnpm build:fresh).

  1. Make sure you are authenticated as applicant@test.com (from env or bearer token)
  2. Query for your own users employee profile
  3. Confirm it returns as expected

Screen recording

Screen.Recording.2024-12-27.at.12.45.49.mov

dreamRoleWorkStream: WorkStream @belongsTo
dreamRoleDepartments: [Department!] @belongsToMany

userPublicProfile: UserPublicProfile
Copy link
Contributor

Choose a reason for hiding this comment

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

The issue states: userPublicProfile: UserPublicProfile @self.

Copy link
Member Author

Choose a reason for hiding this comment

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

Does it work without self? Cause it should, @self is when you want to reference the root model which this does not 🤔

@@ -602,6 +623,7 @@ type User {
locationExemptions: String
acceptedOperationalRequirements: [LocalizedOperationalRequirement]
positionDuration: [PositionDuration]
employeeProfile: EmployeeProfile
Copy link
Contributor

Choose a reason for hiding this comment

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

The issue states: type employeeProfile: EmployeeProfile @self @canResolved(ability: "view").

Copy link
Member Author

Choose a reason for hiding this comment

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

Same as the last one, this is a different model than User so @self does not make sense here. Unless I just do not understand the @self directive 😅

Copy link
Contributor

Choose a reason for hiding this comment

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

I agree this field is about relationship between EmployeeProfile and UserPublicProfile, which are also different types. @self is not needed in here

Copy link
Contributor

@mnigh mnigh left a comment

Choose a reason for hiding this comment

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

It would be nice to have another reviewer on this PR before merging.

Copy link
Contributor

@brindasasi brindasasi left a comment

Choose a reason for hiding this comment

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

Just those questions , others looking great to me

api/app/Enums/Mentorship.php Show resolved Hide resolved
api/app/Enums/Mentorship.php Show resolved Hide resolved

case COACHING;
case LEARNING;

Copy link
Contributor

Choose a reason for hiding this comment

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

Exec coaching status

  • Not participating
  • Have a coach
  • Is a coach
  • Both coaching/learning

same question as Mentorship status below
Are we not storing Not participating as sep enum value?

Copy link
Member Author

Choose a reason for hiding this comment

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

I envisioned it being an array.

  • [] = not participating
  • null = not provided

Copy link
Contributor

Choose a reason for hiding this comment

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

that works as well!

Copy link
Contributor

@brindasasi brindasasi left a comment

Choose a reason for hiding this comment

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

LGTM!

@esizer esizer added this pull request to the merge queue Dec 31, 2024
Merged via the queue into main with commit 7202f97 Dec 31, 2024
11 checks passed
@esizer esizer deleted the 12001-employee-profile-backend branch December 31, 2024 16:17
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.

✨ Backend data for Government Career Planning Employee Profile
3 participants