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: Add absenceManagersId #147

Merged
merged 4 commits into from
Nov 29, 2023
Merged

feat: Add absenceManagersId #147

merged 4 commits into from
Nov 29, 2023

Conversation

Que-tin
Copy link
Collaborator

@Que-tin Que-tin commented Nov 24, 2023

No description provided.

Copy link
Member

@jhnns jhnns left a comment

Choose a reason for hiding this comment

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

@Que-tin Personally I think it should be called absenceManagerIds since it's an array of multiple ids. Maybe we want to clarify that with the backend team. Besides that: Looks good 👍

@Que-tin
Copy link
Collaborator Author

Que-tin commented Nov 24, 2023

But its the an array of ids of multiple managers.
absenceManagerIds sounds like a single manager could have multiple ids.

  • absenceManagersId -> Array of 1:1
  • absenceManagerIds -> Array of 1:n
  • absenceManagersIds -> Array of n:n

So I'd say first is correct even tho I agree it sounds a bit odd.

P.s. I don't have rights to merge stuff.

@jhnns
Copy link
Member

jhnns commented Nov 24, 2023

My array naming algorithm is as follows:

  1. How do you call one item in the array? It's an absenceManagerId
  2. Now pluralize it, hence absenceManagerIds

😁

I think both absenceManagerIds and absenceManagersIds make sense since it's a n:n relationship. But not absenceManagersId because it's missing the fact that it's an array of ids :)

@Que-tin Que-tin closed this Nov 27, 2023
@Que-tin Que-tin reopened this Nov 27, 2023
@Que-tin
Copy link
Collaborator Author

Que-tin commented Nov 27, 2023

Naming wont be changes as of now.

@Que-tin Que-tin changed the title Add absenceManagersId feat: Add absenceManagersId Nov 29, 2023
@Que-tin Que-tin merged commit a2fc3f0 into peerigon:main Nov 29, 2023
0 of 6 checks passed
github-actions bot pushed a commit that referenced this pull request Nov 29, 2023
# [23.2.0](v23.1.0...v23.2.0) (2023-11-29)

### Features

* Add absenceManagersId ([#147](#147)) ([a2fc3f0](a2fc3f0))
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.

3 participants