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: ooo.new integration with DEEL #18423

Draft
wants to merge 17 commits into
base: main
Choose a base branch
from

Conversation

ShashankGupta10
Copy link
Contributor

@ShashankGupta10 ShashankGupta10 commented Dec 30, 2024

What does this PR do?

image

Mandatory Tasks (DO NOT REMOVE)

  • I have self-reviewed the code (A decent size PR without self-review might be rejected).
  • I have updated the developer docs in /docs if this PR makes changes that would require a documentation change. If N/A, write N/A here and check the checkbox.
  • I confirm automated tests are in place that prove my fix is effective or that my feature works.

How should this be tested?

  • Are there environment variables that should be set? No.
  • What are the minimal test data to have? None.
  • What is expected (happy path) to have (input and output)? DEEL has OOO Entry
  • Any other important info that could help to test that PR

Copy link

vercel bot commented Dec 30, 2024

@ShashankGupta10 is attempting to deploy a commit to the cal Team on Vercel.

A member of the Team first needs to authorize it.

@graphite-app graphite-app bot added the community Created by Linear-GitHub Sync label Dec 30, 2024
@graphite-app graphite-app bot requested a review from a team December 30, 2024 21:11
@github-actions github-actions bot added High priority Created by Linear-GitHub Sync ✨ feature New feature or request labels Dec 30, 2024
@dosubot dosubot bot added the app-store area: app store, apps, calendar integrations, google calendar, outlook, lark, apple calendar label Dec 30, 2024
@ShashankGupta10 ShashankGupta10 marked this pull request as draft December 30, 2024 21:11
Copy link

graphite-app bot commented Dec 30, 2024

Graphite Automations

"Add consumer team as reviewer" took an action on this PR • (12/30/24)

1 reviewer was added to this PR based on Keith Williams's automation.

"Add community label" took an action on this PR • (12/30/24)

1 label was added to this PR based on Keith Williams's automation.

@ShashankGupta10
Copy link
Contributor Author

ShashankGupta10 commented Dec 30, 2024

Since this is my first PR here at cal, Would love some feedback if I did make some mistakes.

  • Not sure that editing config.json by hand for deel app in the app-store is correct. What is the correct way?
  • Also i am not able to obtain the HRIS profile id from deel since i dont have a business account. Any help here would be appreciated

Drafted the PR for these reasons.

Edit - will fix the code for type checks and tests that are failing first

Copy link
Contributor

@Praashh Praashh left a comment

Choose a reason for hiding this comment

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

type checks are failing, @ShashankGupta10 can you please fix it?

Copy link
Contributor

@Praashh Praashh Dec 31, 2024

Choose a reason for hiding this comment

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

we should have proper handling using tryCtach

Comment on lines 15 to 16
await throwIfNotHaveAdminAccessToTeam({ teamId: Number(teamId) ?? null, userId: req.session.user.id });
const installForObject = teamId ? { teamId: Number(teamId) } : { userId: req.session.user.id };
Copy link
Contributor

Choose a reason for hiding this comment

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

there should be null check for session.user.id.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I think the check at lines 9-11 is for that purpose

Copy link
Contributor

Choose a reason for hiding this comment

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

can you please attach a loom video as well @ShashankGupta10

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yeah sure. But I had a doubt. I dont have a DEEL business account and hence cannot get the hris_profile_id. Any other way to test it maybe? @Praashh or @PeerRich

Copy link
Contributor

Choose a reason for hiding this comment

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

not sure but try if you can get some test (dummy) key id for that.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Tried but could not find anything actually. Will try again and get back to you tomorrow

@ShashankGupta10 ShashankGupta10 force-pushed the shashank/ooo_new_deel_integration branch from 268c2fc to 497e0b0 Compare December 31, 2024 10:40
@ShashankGupta10 ShashankGupta10 marked this pull request as ready for review December 31, 2024 20:10
@ShashankGupta10 ShashankGupta10 marked this pull request as draft January 2, 2025 19:45
@ShashankGupta10
Copy link
Contributor Author

This is what I found.

  • The DEEL API also expects a time_off_type_id which is a UID of the timeoff type you can create in DEEL itself.
  • You can get the valid time_off_type_id's for that user by making a request to the DEEL API providing the HRIS profile ID of the user wanting to create a time off request.
  • Our current ooo.new platform has a list of reasons you can have for the timeoff. This will not work since you should only be able to select the time off types that are allowed for you in DEEL.

This is "IF YOUR ORGANIZTION IS CONNECTED TO DEEL" from the cal app-store

  • What we can do is we can keep everything same for the users not connected to DEEL,
  • But for the people who are connected to deel, we change the dropdown to the allowed time off types.
  • BUT we will have to change the table in the DB accordingly coz right now it stores only some reasons like Vacation, sick leave etc.

Any ideas on how we should proceed? I will make the changes in this PR accordingly

References:
https://developer.deel.com/docs/time-off#update-a-time-off-request-for-a-worker
https://developer.deel.com/reference/getpoliciesforprofile
https://developer.deel.com/reference/createtimeoff

@Praashh
Copy link
Contributor

Praashh commented Jan 5, 2025

This is what I found.

  • The DEEL API also expects a time_off_type_id which is a UID of the timeoff type you can create in DEEL itself.
  • You can get the valid time_off_type_id's for that user by making a request to the DEEL API providing the HRIS profile ID of the user wanting to create a time off request.
  • Our current ooo.new platform has a list of reasons you can have for the timeoff. This will not work since you should only be able to select the time off types that are allowed for you in DEEL.

This is "IF YOUR ORGANIZTION IS CONNECTED TO DEEL" from the cal app-store

  • What we can do is we can keep everything same for the users not connected to DEEL,
  • But for the people who are connected to deel, we change the dropdown to the allowed time off types.
  • BUT we will have to change the table in the DB accordingly coz right now it stores only some reasons like Vacation, sick leave etc.

Any ideas on how we should proceed? I will make the changes in this PR accordingly

References: https://developer.deel.com/docs/time-off#update-a-time-off-request-for-a-worker https://developer.deel.com/reference/getpoliciesforprofile https://developer.deel.com/reference/createtimeoff

not sure tbh

@ShashankGupta10
Copy link
Contributor Author

@PeerRich sorry for tagging again but help needed!

@PeerRich
Copy link
Member

PeerRich commented Jan 6, 2025

i dont know personally. there is a chance one of our team members has to take over this PR and get the IDs from deel

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
app-store area: app store, apps, calendar integrations, google calendar, outlook, lark, apple calendar community Created by Linear-GitHub Sync ✨ feature New feature or request High priority Created by Linear-GitHub Sync
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[CAL-4945] ooo.new integration with HR tools
3 participants