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

Add reviewers selection to new pull request form #26596

Closed
wants to merge 13 commits into from
Closed

Add reviewers selection to new pull request form #26596

wants to merge 13 commits into from

Conversation

splitt3r
Copy link
Contributor

@splitt3r splitt3r commented Aug 19, 2023

This is my first contribution so please be kind. I'm not that fluent in Go. I hope the code looks god.

image

image

The UI looks just like the assignee selection here because i think the actions from the pull request edit dialog don't make much sense here. The assignee selection on the pr create and edit pages also differ.

This only touches the UI parts not the API.

For some strange reason i had to comment out https://github.com/go-gitea/gitea/blob/main/modules/notification/notification.go#L234 while testing. Otherwise Gitea just hang there. I think there is / was some notifier setup broken in the Gitpod.io setup.

Fixes #26289

@GiteaBot GiteaBot added the lgtm/need 2 This PR needs two approvals by maintainers to be considered for merging. label Aug 19, 2023
@pull-request-size pull-request-size bot added the size/L Denotes a PR that changes 100-499 lines, ignoring generated files. label Aug 19, 2023
@lunny lunny added the type/enhancement An improvement of existing functionality label Aug 21, 2023
@lunny lunny added this to the 1.21.0 milestone Aug 21, 2023
@splitt3r splitt3r marked this pull request as ready for review September 1, 2023 11:03
@lunny lunny requested review from Zettat123 and delvh September 6, 2023 06:10
@github-actions github-actions bot added the topic/ui Change the appearance of the Gitea UI label Sep 9, 2023
@lunny lunny modified the milestones: 1.21.0, 1.22.0 Sep 21, 2023
@github-actions github-actions bot added modifies/translation modifies/api This PR adds API routes or modifies them modifies/frontend labels Oct 20, 2023
@splitt3r splitt3r requested a review from lunny November 1, 2023 10:28
this needs to be a negative integer
@splitt3r
Copy link
Contributor Author

splitt3r commented Dec 3, 2023

Ping @lunny and @steve-dwyer-itdev could you habe another look? Would be awesome to get this into the next release.

@sebastian-sauer
Copy link
Contributor

@denyskon i think you did modify the same template for project selection on PR creation. Maybe you could take a look at this PR, too?

@@ -800,6 +802,47 @@ func CompareDiff(ctx *context.Context) {
if ctx.Written() {
return
}

// Get reviewer info for pr
Copy link
Member

Choose a reason for hiding this comment

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

Maybe this can be moved into RetrieveRepoMetas, where similar code exists for populating ctx.Data["Assignees"]?

if err != nil {
return err
}
_, err = issue_service.ReviewRequest(ctx, issue, issue.Poster, reviewer, true)
Copy link
Member

Choose a reason for hiding this comment

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

This also creates the review request notification. So the user will receive the "review requested" notification before the "pull request created" notification. If you look at the assignee code, they are first added without sending a notification. The notification is sent later on:

notify_service.IssueChangeAssignee(ctx, issue.Poster, issue, assignee, false, assigneeCommentMap[assigneeID])

Due to this, it also seems like the UI notification cannot be created. I'm seeing this in the logs:

2023/12/30 13:20:58 ...tification/notify.go:56:handler() [E] Was unable to create issue notification: issue does not exist [id: 10, repo_id: 0, index: 0]

I suspect this is because the notifier runs in a different transaction and does not "see" the newly created PR yet?

@GiteaBot GiteaBot added lgtm/blocked A maintainer has reservations with the PR and thus it cannot be merged and removed lgtm/need 2 This PR needs two approvals by maintainers to be considered for merging. labels Dec 30, 2023
@lunny lunny modified the milestones: 1.22.0, 1.23.0 Mar 29, 2024
@lunny
Copy link
Member

lunny commented Nov 2, 2024

Thank to @splitt3r for the contributions. Let's continue the discussion in #32403 .

@splitt3r splitt3r closed this Nov 3, 2024
@GiteaBot GiteaBot removed this from the 1.23.0 milestone Nov 4, 2024
@wxiaoguang
Copy link
Contributor

wxiaoguang commented Nov 9, 2024

Thank you all.

"Add reviewers selection to new pull request #32403" has been merged.

@go-gitea go-gitea locked as resolved and limited conversation to collaborators Feb 1, 2025
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
lgtm/blocked A maintainer has reservations with the PR and thus it cannot be merged modifies/api This PR adds API routes or modifies them modifies/translation size/L Denotes a PR that changes 100-499 lines, ignoring generated files. topic/ui Change the appearance of the Gitea UI type/enhancement An improvement of existing functionality
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Assign reviewers on pull request creation
8 participants