Ben Admin Kit #18

bryanbierce · 2023-11-10T22:57:21Z

https://rippling.atlassian.net/browse/BENEEX-84

This diff scaffolds out the kit API for the Ben Admin kit. This kit will be used to integrate with 3rd party benefit administration providers who require payroll and other demographic data about EE's managed in Rippling in order to provide insurance and other benefits

stevevls

Thanks for being one of our early contributors as we're still ironing out the process. 😅 This repo is public-facing, so this is going to be a very detailed review since 1) third party developers will need to be able to read it and make sense of it and 2) once merged, it's hard to change. Thanks for bearing with us!

To start, I have a couple of high level comments:

Please draft a kit specification here: https://rippling.atlassian.net/wiki/spaces/ID/pages/3703538906/Kit+Specifications. This spec should give us a clear breakdown of capabilities, what is provided by the kit, and what we require from a third party.
Can we rename this to be benefits admin to avoid jargon-y sounding names?
Left some comments inline, but in general please prefer existing data types from the flux_core package instead of creating your own. Definitely open to chat if you there's a reason they're not suitable.
Also looks like in some cases you're defining some types that might want to get pushed down into flux_core.
A lint check is failing...you can run ./run_ruff --fix in the top level directory to auto-fix where possible. It will also tell you where there are any lint violations that it wouldn't auto-fix.

flux_sdk/ben_admin/capabilities/process_deduction_data/interface.py

flux_sdk/ben_admin/capabilities/process_deduction_data/data_models.py

flux_sdk/ben_admin/capabilities/report_employee_data/data_models.py

stevevls · 2023-11-14T21:33:10Z

flux_sdk/ben_admin/capabilities/report_employee_data/data_models.py

+class EmploymentType(Enum):
+    CONTRACTOR = 1
+    SALARIED_FT = 2
+    SALARIED_PT = 3
+    HOURLY_FT = 4
+    HOURLY_PT = 5
+    TEMP = 6
+
+
+class MonetaryValue:
+    value: float
+    currency_type: str
+
+
+class PayFrequency(Enum):
+    WEEKLY = 1
+    BI_WEEKLY = 2
+    MONTHLY = 3
+    SEMI_MONTHLY = 4
+
+class PayTimeUnit(Enum):
+    HOUR = 1
+    DAY = 2
+    WEEK = 3
+    MONTH = 4
+    YEAR = 5 
+    PAY_PERIOD = 6
+
+
+class Pay:
+    frequency: PayFrequency
+    frequency_effective_date: datetime
+    time_unit: PayTimeUnit
+    value_per_unit: MonetaryValue
+    value_effective_date: datetime
+
+class EmploymentHours:
+    type: EmploymentType
+    effectiveDate: datetime
+    hours_per_week: Optional[int]
+
+class Employment:
+    hours: EmploymentHours
+    is_rehire: bool
+    termination_date: Optional[datetime]
+    start_date: datetime
+    original_hire_date: datetime
+    w2_start_date: datetime
+    pay: Pay


I imagine that much of this can be moved into flux core. What are your thoughts @Kingpin007 @vguptarippling ?

I agree, alot of this feels easily re-usable. The Employement object conflicts with Employee a bit as they have some overlappigng data.

If there is additional information in Employment I would recommend you extend Employee and add it to flux-core

flux_sdk/ben_admin/capabilities/report_employee_data/interface.py

bryanbierce · 2023-11-14T23:43:56Z

Thanks for being one of our early contributors as we're still ironing out the process. 😅 This repo is public-facing, so this is going to be a very detailed review since 1) third party developers will need to be able to read it and make sense of it and 2) once merged, it's hard to change. Thanks for bearing with us!

To start, I have a couple of high level comments:

Please draft a kit specification here: https://rippling.atlassian.net/wiki/spaces/ID/pages/3703538906/Kit+Specifications. This spec should give us a clear breakdown of capabilities, what is provided by the kit, and what we require from a third party.

Can we rename this to be benefits admin to avoid jargon-y sounding names?

Left some comments inline, but in general please prefer existing data types from the flux_core package instead of creating your own. Definitely open to chat if you there's a reason they're not suitable.

Also looks like in some cases you're defining some types that might want to get pushed down into flux_core.

A lint check is failing...you can run ./run_ruff --fix in the top level directory to auto-fix where possible. It will also tell you where there are any lint violations that it wouldn't auto-fix.

@stevevls Happy to help iron out the kinks :)

yes, https://rippling.atlassian.net/wiki/spaces/ID/pages/3757047835/Benefits+Administration
yes
I'm not in love with the Employee object. Its does not contain all of the Role and Job data which we require which means I will need to extend it. Since it is so flat, the field names will end up being a bit messy. Looking in the flux kits code in rippling-main I don't see that there is any easily reusable constructors for Employee either so I'm just going to end up having to re-implement the builder anyways. I don't really see the point in reusing it. That said, I can re-implement that builder in a way that others can use it in the future and we can just deal with an un-tidy schema if that is a strong preference.
Yes, the data around pay and hours all feels like things that other kits might need in the future and should be maintained centrally if Employee is.
I don't see that ./run-ruf exists in flux_sdk. Is there a setup script that I missed? Looking at the lint output I see that its mostly line length issues. Is there a config in flux-sdk that can be used to setup file formatting in VSCode or PyCharm?

flux_sdk/benefits_administration/capabilities/process_employees_deductions/interface.py

...its_administration/capabilities/report_employees_personal_and_employment_data/data_models.py

stevevls · 2023-11-15T18:57:23Z

...its_administration/capabilities/report_employees_personal_and_employment_data/data_models.py

+
+
+class BenefitsEligibility(Enum):
+    # this will expand to cover cobra later


Thx for this comment...I was looking at this and wondering why it wasn't a bool. 😁

...its_administration/capabilities/report_employees_personal_and_employment_data/data_models.py

...efits_administration/capabilities/report_employees_personal_and_employment_data/interface.py

stevevls · 2023-11-15T20:34:25Z

I'm not in love with the Employee object. Its does not contain all of the Role and Job data which we require which means I will need to extend it. Since it is so flat, the field names will end up being a bit messy. Looking in the flux kits code in rippling-main I don't see that there is any easily reusable constructors for Employee either so I'm just going to end up having to re-implement the builder anyways. I don't really see the point in reusing it. That said, I can re-implement that builder in a way that others can use it in the future and we can just deal with an un-tidy schema if that is a strong preference.

This is a fair point. If you think it makes sense to create a specialized set of classes for your kit, that's totally fine. They would just need to be named differently than the core types so that the name indicates their purpose. I feel like re-using the name Employee in different contexts would be confusing.

I don't see that ./run-ruf exists in flux_sdk. Is there a setup script that I missed? Looking at the lint output I see that its mostly line length issues. Is there a config in flux-sdk that can be used to setup file formatting in VSCode or PyCharm?

I think it was merged to main after you branch, so if you merge or rebase you should get it. We don't have configs for IDEs (yet), but that's a good idea.

bryanbierce · 2023-11-15T20:37:51Z

This is a fair point. If you think it makes sense to create a specialized set of classes for your kit, that's totally fine. They would just need to be named differently than the core types so that the name indicates their purpose. I feel like re-using the name Employee in different contexts would be confusing.

I've gone ahead and updated the name on mine and used the model from core for part of it so we achieve some consistency. I am duplicating a few of the fields from the Employee object within Employment and status since I think it will be more natural for our consumers to read these types and find that data in those places

stevevls · 2023-12-12T22:11:02Z

@bryanbierce I just merged in latest main because we've added lint and test actions in GH that are now required statuses to merge. Looks like we have some linting failures to take care of. The mypy status check isn't required yet b/c some errors got merged down to main, but please make sure that we don't introduce any new issues as we plan to make it required ASAP. Thanks!

flux_sdk/benefits_administration/capabilities/process_employees_deductions/data_models.py

...its_administration/capabilities/report_employees_personal_and_employment_data/data_models.py

* Add deduction models for Ben Admin * fixes

pshanmukrao · 2024-04-08T09:36:25Z

flux_sdk/benefits_administration/capabilities/report_employees_hr_data/data_models.py

+class EmploymentHours:
+    type: EmploymentType
+    type_effective_date: datetime
+    hours_per_week: int | None
+    hours_effective_date: datetime
+
+
+class Employment:
+    hours: EmploymentHours
+    pay: Pay
+    is_rehire: bool
+    termination_date: datetime | None
+    termination_type: TerminationType | None
+    start_date: datetime
+    original_hire_date: datetime
+    w2_start_date: datetime


@bryanbierce What are the fields do you need that are not part of Employee ?

I see that hours_per_week and hours_effective_date are the only fields ? Can they go inside Employee data type ? cc: @stevevls @Kingpin007 @ujjwalshukla

type_effective_date, hours_per_week, hours_effective_date , termination_type and the pay object are not on Employee currently. I had asked the team if I could extend Employee with these originally and the answer from @Kingpin007 and @ujjwalshukla was no. Happy to move them around if you all would rather co-locate these now. This Employment object only existed because the team originally did not want any of the new data in flux_core so I took the opportunity to duplicate some fields and make the objects a bit more sensible to work with.

Reviewer approved changes

bryanbierce marked this pull request as ready for review November 13, 2023 20:23

bryanbierce requested a review from a team as a code owner November 13, 2023 20:23

bryanbierce requested review from Kingpin007 and vguptarippling November 13, 2023 20:32

stevevls requested a review from a team as a code owner November 14, 2023 19:38

stevevls reviewed Nov 14, 2023

View reviewed changes

bryanbierce force-pushed the bb-ce-ben-admin-kit branch 2 times, most recently from 3f60b25 to d925f71 Compare November 14, 2023 23:47

bryanbierce commented Nov 15, 2023

View reviewed changes

flux_sdk/benefits_administration/capabilities/process_employees_deductions/interface.py Outdated Show resolved Hide resolved

Kingpin007 reviewed Nov 15, 2023

View reviewed changes

...its_administration/capabilities/report_employees_personal_and_employment_data/data_models.py Outdated Show resolved Hide resolved

bryanbierce force-pushed the bb-ce-ben-admin-kit branch from abb8777 to 8763977 Compare November 15, 2023 17:06

stevevls reviewed Nov 15, 2023

View reviewed changes

bryanbierce force-pushed the bb-ce-ben-admin-kit branch 3 times, most recently from dee5378 to 241cf8c Compare December 7, 2023 21:50

bryanbierce force-pushed the bb-ce-ben-admin-kit branch 5 times, most recently from ea24554 to f5a349e Compare December 19, 2023 22:25

bryanbierce force-pushed the bb-ce-ben-admin-kit branch from f5a349e to 6ea26a6 Compare January 31, 2024 19:44

bryanbierce force-pushed the bb-ce-ben-admin-kit branch from 172fd74 to a713cb5 Compare March 19, 2024 21:03

bryanbierce requested review from Kingpin007 and stevevls March 19, 2024 21:10

Kingpin007 reviewed Mar 21, 2024

View reviewed changes

flux_sdk/benefits_administration/capabilities/process_employees_deductions/data_models.py Outdated Show resolved Hide resolved

Kingpin007 reviewed Mar 21, 2024

View reviewed changes

...its_administration/capabilities/report_employees_personal_and_employment_data/data_models.py Outdated Show resolved Hide resolved

bryanbierce and others added 22 commits April 5, 2024 10:07

Update BUILD

91248f0

Add Ben Admin deduction data models (#36)

f74bd7d

* Add deduction models for Ben Admin * fixes

update data models

7249db4

reset poetry

5894a83

lint

1cdb40f

bump version

f0657d6

lint

3580ff3

lint

b982548

lint

2a7489b

lint

8bb6d05

try again

b0d41a0

dont change version

28e45e0

remove options for long hand union. ¯\_(ツ)_/¯

600648b

change capability name

b8db335

update types

3f7e790

remove get_file_name

a6bc315

move types to core

f61d5d3

import format

423a348

update field name casing. fix doc strings

4a4b29d

import order again

090230b

add types to core

40e2abb

fix import sort

0fd6d99

bryanbierce force-pushed the bb-ce-ben-admin-kit branch from c9cb2d5 to 0fd6d99 Compare April 5, 2024 17:09

bryanbierce added 2 commits April 5, 2024 15:12

fix rebase

16887cd

field casing

da100d9

pshanmukrao reviewed Apr 8, 2024

View reviewed changes

sz2376 approved these changes Apr 8, 2024

View reviewed changes

bryanbierce merged commit 74613c5 into main Apr 9, 2024
3 checks passed

bryanbierce deleted the bb-ce-ben-admin-kit branch April 9, 2024 21:58

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Ben Admin Kit #18

Ben Admin Kit #18

bryanbierce commented Nov 10, 2023 •

edited

Loading

stevevls left a comment •

edited

Loading

stevevls Nov 14, 2023 •

edited

Loading

bryanbierce Nov 14, 2023

Kingpin007 Apr 2, 2024

bryanbierce commented Nov 14, 2023 •

edited

Loading

stevevls Nov 15, 2023

stevevls commented Nov 15, 2023 •

edited

Loading

bryanbierce commented Nov 15, 2023

stevevls commented Dec 12, 2023

pshanmukrao Apr 8, 2024

bryanbierce Apr 8, 2024



		class BenefitsEligibility(Enum):
		# this will expand to cover cobra later

Ben Admin Kit #18

Ben Admin Kit #18

Conversation

bryanbierce commented Nov 10, 2023 • edited Loading

stevevls left a comment • edited Loading

Choose a reason for hiding this comment

stevevls Nov 14, 2023 • edited Loading

Choose a reason for hiding this comment

bryanbierce Nov 14, 2023

Choose a reason for hiding this comment

Kingpin007 Apr 2, 2024

Choose a reason for hiding this comment

bryanbierce commented Nov 14, 2023 • edited Loading

stevevls Nov 15, 2023

Choose a reason for hiding this comment

stevevls commented Nov 15, 2023 • edited Loading

bryanbierce commented Nov 15, 2023

stevevls commented Dec 12, 2023

pshanmukrao Apr 8, 2024

Choose a reason for hiding this comment

bryanbierce Apr 8, 2024

Choose a reason for hiding this comment

bryanbierce commented Nov 10, 2023 •

edited

Loading

stevevls left a comment •

edited

Loading

stevevls Nov 14, 2023 •

edited

Loading

bryanbierce commented Nov 14, 2023 •

edited

Loading

stevevls commented Nov 15, 2023 •

edited

Loading