-
Notifications
You must be signed in to change notification settings - Fork 0
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 an ability to split contributors gf-462 #477
Conversation
perekotypole
commented
Sep 23, 2024
- added a split option to the contributors table and contributors list in project page
- split option is shown only for contributors with more than one git email, and it don't let to split it on backed
- added split modal
- when the form is submitted, a new contributor is created and the selected email is transferred to this contributor
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Change please the name of the file (delete "copy")
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
oh, thanks for noticing
apps/frontend/src/pages/project/libs/components/contributor-menu/contributor-menu.tsx
Outdated
Show resolved
Hide resolved
|
||
const contributorSplit: z.ZodType<ContributorSplitRequestDto> = z | ||
.object({ | ||
emailId: z |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
let's rename it to gitEmailId
instead to avoid confusion, and the same for error message
emailId: z | |
gitEmailId: z |
@@ -1,6 +1,7 @@ | |||
import { ContributorValidationRule } from "./contributor-validation-rule.enum.js"; | |||
|
|||
const ContributorValidationMessage = { | |||
EMAIL_REQUIRED: "Email is required", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
EMAIL_REQUIRED: "Email is required", | |
GIT_EMAIL_REQUIRED: "Git email is required.", |
|
||
const currentContributor = currentContributorEntity.toObject(); | ||
|
||
if (currentContributor.gitEmails.length <= SINGLE_ITEM) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
let's create a more specific constant for this
if (currentContributor.gitEmails.length <= SINGLE_ITEM) { | |
if (currentContributor.gitEmails.length <= MIN_GIT_EMAILS_LENGTH_FOR_SPLIT) { |
const splittedEmail = currentContributor.gitEmails.find( | ||
({ id }) => id === payload.emailId, | ||
); | ||
const hasSplittedEmail = splittedEmail !== undefined; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
should be split in 3d form
const splittedEmail = currentContributor.gitEmails.find( | |
({ id }) => id === payload.emailId, | |
); | |
const hasSplittedEmail = splittedEmail !== undefined; | |
const splitEmail = currentContributor.gitEmails.find( | |
({ id }) => id === payload.emailId, | |
); | |
const splitEmail = splitEmail !== undefined; |
@@ -1,5 +1,6 @@ | |||
const NotificationMessage = { | |||
CONTRIBUTOR_MERGE_SUCCESS: "Contributor was successfully merged.", | |||
CONTRIBUTOR_MERGE_SUCCESS: "Contributors were successfully merged.", | |||
CONTRIBUTOR_SPLIT_SUCCESS: "Contributors were successfully split.", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
In case you choose plural here, then change CONTRIBUTOR_SPLIT_FAILED
to plural as well:
"Failed to split contributors." instead of "Failed to split contributor."
or leave CONTRIBUTOR_SPLIT_FAILED
without any change, and change this message to "Contributor wes successfully split."
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
The selected contributor will be deleted. This action can be undone | ||
later using the split functionality. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think it sounds a little bit unclear, perhaps something like this is better?
The selected contributor will be deleted. You can undo this action later using the split functionality.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this message was attached to my ticket, I took it from the task