Merge pull request 'feat: combine review requests comments' (#5695) from gusted/forgejo-combine-request-review into forgejo

+43 -35

models/issues/comment.go

··· 222 222 return lang.TrString("repo.issues.role." + string(r) + "_helper") 223 223 } 224 224 225 + type RequestReviewTarget interface { 226 + ID() int64 227 + Name() string 228 + Type() string 229 + } 230 + 225 231 // Comment represents a comment in commit and issue page. 226 232 type Comment struct { 227 - ID int64 `xorm:"pk autoincr"` 228 - Type CommentType `xorm:"INDEX"` 229 - PosterID int64 `xorm:"INDEX"` 230 - Poster *user_model.User `xorm:"-"` 231 - OriginalAuthor string 232 - OriginalAuthorID int64 233 - IssueID int64 `xorm:"INDEX"` 234 - Issue *Issue `xorm:"-"` 235 - LabelID int64 236 - Label *Label `xorm:"-"` 237 - AddedLabels []*Label `xorm:"-"` 238 - RemovedLabels []*Label `xorm:"-"` 239 - OldProjectID int64 240 - ProjectID int64 241 - OldProject *project_model.Project `xorm:"-"` 242 - Project *project_model.Project `xorm:"-"` 243 - OldMilestoneID int64 244 - MilestoneID int64 245 - OldMilestone *Milestone `xorm:"-"` 246 - Milestone *Milestone `xorm:"-"` 247 - TimeID int64 248 - Time *TrackedTime `xorm:"-"` 249 - AssigneeID int64 250 - RemovedAssignee bool 251 - Assignee *user_model.User `xorm:"-"` 252 - AssigneeTeamID int64 `xorm:"NOT NULL DEFAULT 0"` 253 - AssigneeTeam *organization.Team `xorm:"-"` 254 - ResolveDoerID int64 255 - ResolveDoer *user_model.User `xorm:"-"` 256 - OldTitle string 257 - NewTitle string 258 - OldRef string 259 - NewRef string 260 - DependentIssueID int64 `xorm:"index"` // This is used by issue_service.deleteIssue 261 - DependentIssue *Issue `xorm:"-"` 233 + ID int64 `xorm:"pk autoincr"` 234 + Type CommentType `xorm:"INDEX"` 235 + PosterID int64 `xorm:"INDEX"` 236 + Poster *user_model.User `xorm:"-"` 237 + OriginalAuthor string 238 + OriginalAuthorID int64 239 + IssueID int64 `xorm:"INDEX"` 240 + Issue *Issue `xorm:"-"` 241 + LabelID int64 242 + Label *Label `xorm:"-"` 243 + AddedLabels []*Label `xorm:"-"` 244 + RemovedLabels []*Label `xorm:"-"` 245 + AddedRequestReview []RequestReviewTarget `xorm:"-"` 246 + RemovedRequestReview []RequestReviewTarget `xorm:"-"` 247 + OldProjectID int64 248 + ProjectID int64 249 + OldProject *project_model.Project `xorm:"-"` 250 + Project *project_model.Project `xorm:"-"` 251 + OldMilestoneID int64 252 + MilestoneID int64 253 + OldMilestone *Milestone `xorm:"-"` 254 + Milestone *Milestone `xorm:"-"` 255 + TimeID int64 256 + Time *TrackedTime `xorm:"-"` 257 + AssigneeID int64 258 + RemovedAssignee bool 259 + Assignee *user_model.User `xorm:"-"` 260 + AssigneeTeamID int64 `xorm:"NOT NULL DEFAULT 0"` 261 + AssigneeTeam *organization.Team `xorm:"-"` 262 + ResolveDoerID int64 263 + ResolveDoer *user_model.User `xorm:"-"` 264 + OldTitle string 265 + NewTitle string 266 + OldRef string 267 + NewRef string 268 + DependentIssueID int64 `xorm:"index"` // This is used by issue_service.deleteIssue 269 + DependentIssue *Issue `xorm:"-"` 262 270 263 271 CommitID int64 264 272 Line int64 // - previous line / + proposed line

+1

modules/templates/helper.go

··· 182 182 "RenderMarkdownToHtml": RenderMarkdownToHtml, 183 183 "RenderLabel": RenderLabel, 184 184 "RenderLabels": RenderLabels, 185 + "RenderReviewRequest": RenderReviewRequest, 185 186 186 187 // ----------------------------------------------------------------- 187 188 // misc

+12

modules/templates/util_render.go

··· 262 262 htmlCode += "</span>" 263 263 return template.HTML(htmlCode) 264 264 } 265 + 266 + func RenderReviewRequest(users []issues_model.RequestReviewTarget) template.HTML { 267 + usernames := make([]string, 0, len(users)) 268 + for _, user := range users { 269 + usernames = append(usernames, template.HTMLEscapeString(user.Name())) 270 + } 271 + 272 + htmlCode := `<span class="review-request-list">` 273 + htmlCode += strings.Join(usernames, ", ") 274 + htmlCode += "</span>" 275 + return template.HTML(htmlCode) 276 + }

+6 -3

options/locale/locale_en-US.ini

··· 1791 1791 issues.review.content.empty = You need to leave a comment indicating the requested change(s). 1792 1792 issues.review.reject = requested changes %s 1793 1793 issues.review.wait = was requested for review %s 1794 - issues.review.add_review_request = requested review from %s %s 1795 - issues.review.remove_review_request = removed review request for %s %s 1796 - issues.review.remove_review_request_self = refused to review %s 1794 + issues.review.add_review_request = requested review from %[1]s %[2]s 1795 + issues.review.add_review_requests = requested reviews from %[1]s %[2]s 1796 + issues.review.remove_review_request = removed review request for %[1]s %[2]s 1797 + issues.review.remove_review_requests = removed review requests for %[1]s %[2]s 1798 + issues.review.remove_review_request_self = refused to review %[1]s %[2]s 1799 + issues.review.add_remove_review_requests = requested reviews from %[1]s and removed review requests for %[2]s %[3]s 1797 1800 issues.review.pending = Pending 1798 1801 issues.review.pending.tooltip = This comment is not currently visible to other users. To submit your pending comments, select "%s" -> "%s/%s/%s" at the top of the page. 1799 1802 issues.review.review = Review

+122

routers/web/repo/issue.go

··· 1825 1825 1826 1826 // Combine multiple label assignments into a single comment 1827 1827 combineLabelComments(issue) 1828 + combineRequestReviewComments(issue) 1828 1829 1829 1830 getBranchData(ctx, issue) 1830 1831 if issue.IsPull { ··· 3663 3664 return "" 3664 3665 } 3665 3666 return attachHTML 3667 + } 3668 + 3669 + type RequestReviewTarget struct { 3670 + user *user_model.User 3671 + team *organization.Team 3672 + } 3673 + 3674 + func (t *RequestReviewTarget) ID() int64 { 3675 + if t.user != nil { 3676 + return t.user.ID 3677 + } 3678 + return t.team.ID 3679 + } 3680 + 3681 + func (t *RequestReviewTarget) Name() string { 3682 + if t.user != nil { 3683 + return t.user.GetDisplayName() 3684 + } 3685 + return t.team.Name 3686 + } 3687 + 3688 + func (t *RequestReviewTarget) Type() string { 3689 + if t.user != nil { 3690 + return "user" 3691 + } 3692 + return "team" 3693 + } 3694 + 3695 + // combineRequestReviewComments combine the nearby request review comments as one. 3696 + func combineRequestReviewComments(issue *issues_model.Issue) { 3697 + var prev, cur *issues_model.Comment 3698 + for i := 0; i < len(issue.Comments); i++ { 3699 + cur = issue.Comments[i] 3700 + if i > 0 { 3701 + prev = issue.Comments[i-1] 3702 + } 3703 + if i == 0 || cur.Type != issues_model.CommentTypeReviewRequest || 3704 + (prev != nil && prev.PosterID != cur.PosterID) || 3705 + (prev != nil && cur.CreatedUnix-prev.CreatedUnix >= 60) { 3706 + if cur.Type == issues_model.CommentTypeReviewRequest && (cur.Assignee != nil || cur.AssigneeTeam != nil) { 3707 + if cur.RemovedAssignee { 3708 + if cur.AssigneeTeam != nil { 3709 + cur.RemovedRequestReview = append(cur.RemovedRequestReview, &RequestReviewTarget{team: cur.AssigneeTeam}) 3710 + } else { 3711 + cur.RemovedRequestReview = append(cur.RemovedRequestReview, &RequestReviewTarget{user: cur.Assignee}) 3712 + } 3713 + } else { 3714 + if cur.AssigneeTeam != nil { 3715 + cur.AddedRequestReview = append(cur.AddedRequestReview, &RequestReviewTarget{team: cur.AssigneeTeam}) 3716 + } else { 3717 + cur.AddedRequestReview = append(cur.AddedRequestReview, &RequestReviewTarget{user: cur.Assignee}) 3718 + } 3719 + } 3720 + } 3721 + continue 3722 + } 3723 + 3724 + // Previous comment is not a review request, so cannot group. Start a new group. 3725 + if prev.Type != issues_model.CommentTypeReviewRequest { 3726 + if cur.RemovedAssignee { 3727 + if cur.AssigneeTeam != nil { 3728 + cur.RemovedRequestReview = append(cur.RemovedRequestReview, &RequestReviewTarget{team: cur.AssigneeTeam}) 3729 + } else { 3730 + cur.RemovedRequestReview = append(cur.RemovedRequestReview, &RequestReviewTarget{user: cur.Assignee}) 3731 + } 3732 + } else { 3733 + if cur.AssigneeTeam != nil { 3734 + cur.AddedRequestReview = append(cur.AddedRequestReview, &RequestReviewTarget{team: cur.AssigneeTeam}) 3735 + } else { 3736 + cur.AddedRequestReview = append(cur.AddedRequestReview, &RequestReviewTarget{user: cur.Assignee}) 3737 + } 3738 + } 3739 + continue 3740 + } 3741 + 3742 + // Start grouping. 3743 + if cur.RemovedAssignee { 3744 + addedIndex := slices.IndexFunc(prev.AddedRequestReview, func(t issues_model.RequestReviewTarget) bool { 3745 + if cur.AssigneeTeam != nil { 3746 + return cur.AssigneeTeam.ID == t.ID() && t.Type() == "team" 3747 + } 3748 + return cur.Assignee.ID == t.ID() && t.Type() == "user" 3749 + }) 3750 + 3751 + // If for this target a AddedRequestReview, then we remove that entry. If it's not found, then add it to the RemovedRequestReview. 3752 + if addedIndex == -1 { 3753 + if cur.AssigneeTeam != nil { 3754 + prev.RemovedRequestReview = append(prev.RemovedRequestReview, &RequestReviewTarget{team: cur.AssigneeTeam}) 3755 + } else { 3756 + prev.RemovedRequestReview = append(prev.RemovedRequestReview, &RequestReviewTarget{user: cur.Assignee}) 3757 + } 3758 + } else { 3759 + prev.AddedRequestReview = slices.Delete(prev.AddedRequestReview, addedIndex, addedIndex+1) 3760 + } 3761 + } else { 3762 + removedIndex := slices.IndexFunc(prev.RemovedRequestReview, func(t issues_model.RequestReviewTarget) bool { 3763 + if cur.AssigneeTeam != nil { 3764 + return cur.AssigneeTeam.ID == t.ID() && t.Type() == "team" 3765 + } 3766 + return cur.Assignee.ID == t.ID() && t.Type() == "user" 3767 + }) 3768 + 3769 + // If for this target a RemovedRequestReview, then we remove that entry. If it's not found, then add it to the AddedRequestReview. 3770 + if removedIndex == -1 { 3771 + if cur.AssigneeTeam != nil { 3772 + prev.AddedRequestReview = append(prev.AddedRequestReview, &RequestReviewTarget{team: cur.AssigneeTeam}) 3773 + } else { 3774 + prev.AddedRequestReview = append(prev.AddedRequestReview, &RequestReviewTarget{user: cur.Assignee}) 3775 + } 3776 + } else { 3777 + prev.RemovedRequestReview = slices.Delete(prev.RemovedRequestReview, removedIndex, removedIndex+1) 3778 + } 3779 + } 3780 + 3781 + // Propoagate creation time. 3782 + prev.CreatedUnix = cur.CreatedUnix 3783 + 3784 + // Remove the current comment since it has been combined to prev comment 3785 + issue.Comments = append(issue.Comments[:i], issue.Comments[i+1:]...) 3786 + i-- 3787 + } 3666 3788 } 3667 3789 3668 3790 // combineLabelComments combine the nearby label comments as one.

+431

routers/web/repo/issue_test.go

··· 7 7 "testing" 8 8 9 9 issues_model "code.gitea.io/gitea/models/issues" 10 + org_model "code.gitea.io/gitea/models/organization" 11 + user_model "code.gitea.io/gitea/models/user" 10 12 11 13 "github.com/stretchr/testify/assert" 12 14 ) ··· 373 375 }) 374 376 } 375 377 } 378 + 379 + func TestCombineReviewRequests(t *testing.T) { 380 + testCases := []struct { 381 + name string 382 + beforeCombined []*issues_model.Comment 383 + afterCombined []*issues_model.Comment 384 + }{ 385 + { 386 + name: "case 1", 387 + beforeCombined: []*issues_model.Comment{ 388 + { 389 + Type: issues_model.CommentTypeReviewRequest, 390 + PosterID: 1, 391 + Assignee: &user_model.User{ 392 + ID: 1, 393 + Name: "Ghost", 394 + }, 395 + CreatedUnix: 0, 396 + }, 397 + { 398 + Type: issues_model.CommentTypeReviewRequest, 399 + PosterID: 1, 400 + RemovedAssignee: true, 401 + Assignee: &user_model.User{ 402 + ID: 1, 403 + Name: "Ghost", 404 + }, 405 + CreatedUnix: 0, 406 + }, 407 + { 408 + Type: issues_model.CommentTypeComment, 409 + PosterID: 1, 410 + Content: "test", 411 + CreatedUnix: 0, 412 + }, 413 + }, 414 + afterCombined: []*issues_model.Comment{ 415 + { 416 + Type: issues_model.CommentTypeReviewRequest, 417 + PosterID: 1, 418 + CreatedUnix: 0, 419 + AddedRequestReview: []issues_model.RequestReviewTarget{}, 420 + Assignee: &user_model.User{ 421 + ID: 1, 422 + Name: "Ghost", 423 + }, 424 + }, 425 + { 426 + Type: issues_model.CommentTypeComment, 427 + PosterID: 1, 428 + Content: "test", 429 + CreatedUnix: 0, 430 + }, 431 + }, 432 + }, 433 + { 434 + name: "case 2", 435 + beforeCombined: []*issues_model.Comment{ 436 + { 437 + Type: issues_model.CommentTypeReviewRequest, 438 + PosterID: 1, 439 + Assignee: &user_model.User{ 440 + ID: 1, 441 + Name: "Ghost", 442 + }, 443 + CreatedUnix: 0, 444 + }, 445 + { 446 + Type: issues_model.CommentTypeReviewRequest, 447 + PosterID: 1, 448 + Assignee: &user_model.User{ 449 + ID: 2, 450 + Name: "Ghost 2", 451 + }, 452 + CreatedUnix: 0, 453 + }, 454 + }, 455 + afterCombined: []*issues_model.Comment{ 456 + { 457 + Type: issues_model.CommentTypeReviewRequest, 458 + PosterID: 1, 459 + CreatedUnix: 0, 460 + AddedRequestReview: []issues_model.RequestReviewTarget{ 461 + &RequestReviewTarget{ 462 + user: &user_model.User{ 463 + ID: 1, 464 + Name: "Ghost", 465 + }, 466 + }, 467 + &RequestReviewTarget{ 468 + user: &user_model.User{ 469 + ID: 2, 470 + Name: "Ghost 2", 471 + }, 472 + }, 473 + }, 474 + Assignee: &user_model.User{ 475 + ID: 1, 476 + Name: "Ghost", 477 + }, 478 + }, 479 + }, 480 + }, 481 + { 482 + name: "case 3", 483 + beforeCombined: []*issues_model.Comment{ 484 + { 485 + Type: issues_model.CommentTypeReviewRequest, 486 + PosterID: 1, 487 + Assignee: &user_model.User{ 488 + ID: 1, 489 + Name: "Ghost", 490 + }, 491 + CreatedUnix: 0, 492 + }, 493 + { 494 + Type: issues_model.CommentTypeReviewRequest, 495 + PosterID: 1, 496 + RemovedAssignee: true, 497 + AssigneeTeam: &org_model.Team{ 498 + ID: 1, 499 + Name: "Team 1", 500 + }, 501 + CreatedUnix: 0, 502 + }, 503 + }, 504 + afterCombined: []*issues_model.Comment{ 505 + { 506 + Type: issues_model.CommentTypeReviewRequest, 507 + PosterID: 1, 508 + CreatedUnix: 0, 509 + AddedRequestReview: []issues_model.RequestReviewTarget{ 510 + &RequestReviewTarget{ 511 + user: &user_model.User{ 512 + ID: 1, 513 + Name: "Ghost", 514 + }, 515 + }, 516 + }, 517 + RemovedRequestReview: []issues_model.RequestReviewTarget{ 518 + &RequestReviewTarget{ 519 + team: &org_model.Team{ 520 + ID: 1, 521 + Name: "Team 1", 522 + }, 523 + }, 524 + }, 525 + Assignee: &user_model.User{ 526 + ID: 1, 527 + Name: "Ghost", 528 + }, 529 + }, 530 + }, 531 + }, 532 + { 533 + name: "case 4", 534 + beforeCombined: []*issues_model.Comment{ 535 + { 536 + Type: issues_model.CommentTypeReviewRequest, 537 + PosterID: 1, 538 + Assignee: &user_model.User{ 539 + ID: 1, 540 + Name: "Ghost", 541 + }, 542 + CreatedUnix: 0, 543 + }, 544 + { 545 + Type: issues_model.CommentTypeReviewRequest, 546 + PosterID: 1, 547 + RemovedAssignee: true, 548 + AssigneeTeam: &org_model.Team{ 549 + ID: 1, 550 + Name: "Team 1", 551 + }, 552 + CreatedUnix: 0, 553 + }, 554 + { 555 + Type: issues_model.CommentTypeReviewRequest, 556 + PosterID: 1, 557 + AssigneeTeam: &org_model.Team{ 558 + ID: 1, 559 + Name: "Team 1", 560 + }, 561 + CreatedUnix: 0, 562 + }, 563 + }, 564 + afterCombined: []*issues_model.Comment{ 565 + { 566 + Type: issues_model.CommentTypeReviewRequest, 567 + PosterID: 1, 568 + CreatedUnix: 0, 569 + AddedRequestReview: []issues_model.RequestReviewTarget{ 570 + &RequestReviewTarget{ 571 + user: &user_model.User{ 572 + ID: 1, 573 + Name: "Ghost", 574 + }, 575 + }, 576 + }, 577 + RemovedRequestReview: []issues_model.RequestReviewTarget{}, 578 + Assignee: &user_model.User{ 579 + ID: 1, 580 + Name: "Ghost", 581 + }, 582 + }, 583 + }, 584 + }, 585 + { 586 + name: "case 5", 587 + beforeCombined: []*issues_model.Comment{ 588 + { 589 + Type: issues_model.CommentTypeReviewRequest, 590 + PosterID: 1, 591 + Assignee: &user_model.User{ 592 + ID: 1, 593 + Name: "Ghost", 594 + }, 595 + CreatedUnix: 0, 596 + }, 597 + { 598 + Type: issues_model.CommentTypeReviewRequest, 599 + PosterID: 1, 600 + RemovedAssignee: true, 601 + AssigneeTeam: &org_model.Team{ 602 + ID: 1, 603 + Name: "Team 1", 604 + }, 605 + CreatedUnix: 0, 606 + }, 607 + { 608 + Type: issues_model.CommentTypeReviewRequest, 609 + PosterID: 1, 610 + AssigneeTeam: &org_model.Team{ 611 + ID: 1, 612 + Name: "Team 1", 613 + }, 614 + CreatedUnix: 0, 615 + }, 616 + { 617 + Type: issues_model.CommentTypeReviewRequest, 618 + PosterID: 1, 619 + RemovedAssignee: true, 620 + Assignee: &user_model.User{ 621 + ID: 1, 622 + Name: "Ghost", 623 + }, 624 + CreatedUnix: 0, 625 + }, 626 + }, 627 + afterCombined: []*issues_model.Comment{ 628 + { 629 + Type: issues_model.CommentTypeReviewRequest, 630 + PosterID: 1, 631 + CreatedUnix: 0, 632 + AddedRequestReview: []issues_model.RequestReviewTarget{}, 633 + RemovedRequestReview: []issues_model.RequestReviewTarget{}, 634 + Assignee: &user_model.User{ 635 + ID: 1, 636 + Name: "Ghost", 637 + }, 638 + }, 639 + }, 640 + }, 641 + { 642 + name: "case 6", 643 + beforeCombined: []*issues_model.Comment{ 644 + { 645 + Type: issues_model.CommentTypeReviewRequest, 646 + PosterID: 1, 647 + Assignee: &user_model.User{ 648 + ID: 1, 649 + Name: "Ghost", 650 + }, 651 + CreatedUnix: 0, 652 + }, 653 + { 654 + Type: issues_model.CommentTypeReviewRequest, 655 + PosterID: 1, 656 + RemovedAssignee: true, 657 + AssigneeTeam: &org_model.Team{ 658 + ID: 1, 659 + Name: "Team 1", 660 + }, 661 + CreatedUnix: 0, 662 + }, 663 + { 664 + Type: issues_model.CommentTypeComment, 665 + PosterID: 1, 666 + Content: "test", 667 + CreatedUnix: 0, 668 + }, 669 + { 670 + Type: issues_model.CommentTypeReviewRequest, 671 + PosterID: 1, 672 + AssigneeTeam: &org_model.Team{ 673 + ID: 1, 674 + Name: "Team 1", 675 + }, 676 + CreatedUnix: 0, 677 + }, 678 + { 679 + Type: issues_model.CommentTypeReviewRequest, 680 + PosterID: 1, 681 + RemovedAssignee: true, 682 + Assignee: &user_model.User{ 683 + ID: 1, 684 + Name: "Ghost", 685 + }, 686 + CreatedUnix: 0, 687 + }, 688 + }, 689 + afterCombined: []*issues_model.Comment{ 690 + { 691 + Type: issues_model.CommentTypeReviewRequest, 692 + PosterID: 1, 693 + CreatedUnix: 0, 694 + RemovedRequestReview: []issues_model.RequestReviewTarget{&RequestReviewTarget{ 695 + team: &org_model.Team{ 696 + ID: 1, 697 + Name: "Team 1", 698 + }, 699 + }}, 700 + AddedRequestReview: []issues_model.RequestReviewTarget{&RequestReviewTarget{ 701 + user: &user_model.User{ 702 + ID: 1, 703 + Name: "Ghost", 704 + }, 705 + }}, 706 + Assignee: &user_model.User{ 707 + ID: 1, 708 + Name: "Ghost", 709 + }, 710 + }, 711 + { 712 + Type: issues_model.CommentTypeComment, 713 + PosterID: 1, 714 + Content: "test", 715 + CreatedUnix: 0, 716 + }, 717 + { 718 + Type: issues_model.CommentTypeReviewRequest, 719 + PosterID: 1, 720 + CreatedUnix: 0, 721 + AddedRequestReview: []issues_model.RequestReviewTarget{&RequestReviewTarget{ 722 + team: &org_model.Team{ 723 + ID: 1, 724 + Name: "Team 1", 725 + }, 726 + }}, 727 + RemovedRequestReview: []issues_model.RequestReviewTarget{&RequestReviewTarget{ 728 + user: &user_model.User{ 729 + ID: 1, 730 + Name: "Ghost", 731 + }, 732 + }}, 733 + AssigneeTeam: &org_model.Team{ 734 + ID: 1, 735 + Name: "Team 1", 736 + }, 737 + }, 738 + }, 739 + }, 740 + { 741 + name: "case 7", 742 + beforeCombined: []*issues_model.Comment{ 743 + { 744 + Type: issues_model.CommentTypeReviewRequest, 745 + PosterID: 1, 746 + Assignee: &user_model.User{ 747 + ID: 1, 748 + Name: "Ghost", 749 + }, 750 + CreatedUnix: 0, 751 + }, 752 + { 753 + Type: issues_model.CommentTypeReviewRequest, 754 + PosterID: 1, 755 + AssigneeTeam: &org_model.Team{ 756 + ID: 1, 757 + Name: "Team 1", 758 + }, 759 + CreatedUnix: 61, 760 + }, 761 + }, 762 + afterCombined: []*issues_model.Comment{ 763 + { 764 + Type: issues_model.CommentTypeReviewRequest, 765 + PosterID: 1, 766 + CreatedUnix: 0, 767 + AddedRequestReview: []issues_model.RequestReviewTarget{&RequestReviewTarget{ 768 + user: &user_model.User{ 769 + ID: 1, 770 + Name: "Ghost", 771 + }, 772 + }}, 773 + Assignee: &user_model.User{ 774 + ID: 1, 775 + Name: "Ghost", 776 + }, 777 + }, 778 + { 779 + Type: issues_model.CommentTypeReviewRequest, 780 + PosterID: 1, 781 + CreatedUnix: 0, 782 + RemovedRequestReview: []issues_model.RequestReviewTarget{&RequestReviewTarget{ 783 + team: &org_model.Team{ 784 + ID: 1, 785 + Name: "Team 1", 786 + }, 787 + }}, 788 + AssigneeTeam: &org_model.Team{ 789 + ID: 1, 790 + Name: "Team 1", 791 + }, 792 + }, 793 + }, 794 + }, 795 + } 796 + 797 + for _, testCase := range testCases { 798 + t.Run(testCase.name, func(t *testing.T) { 799 + issue := issues_model.Issue{ 800 + Comments: testCase.beforeCombined, 801 + } 802 + combineRequestReviewComments(&issue) 803 + assert.EqualValues(t, testCase.afterCombined[0], issue.Comments[0]) 804 + }) 805 + } 806 + }

+15 -27

templates/repo/issue/view_content/comments.tmpl

··· 522 522 </div> 523 523 </div> 524 524 {{else if eq .Type 27}} 525 - <div class="timeline-item event" id="{{.HashTag}}"> 526 - <span class="badge">{{svg "octicon-eye"}}</span> 527 - {{template "shared/user/avatarlink" dict "user" .Poster}} 528 - <span class="text grey muted-links"> 529 - {{template "shared/user/authorlink" .Poster}} 530 - {{if (gt .AssigneeID 0)}} 531 - {{if .RemovedAssignee}} 532 - {{if eq .PosterID .AssigneeID}} 533 - {{ctx.Locale.Tr "repo.issues.review.remove_review_request_self" $createdStr}} 534 - {{else}} 535 - {{ctx.Locale.Tr "repo.issues.review.remove_review_request" .Assignee.GetDisplayName $createdStr}} 536 - {{end}} 537 - {{else}} 538 - {{ctx.Locale.Tr "repo.issues.review.add_review_request" .Assignee.GetDisplayName $createdStr}} 539 - {{end}} 540 - {{else}} 541 -  542 - {{$teamName := "Ghost Team"}} 543 - {{if .AssigneeTeam}} 544 - {{$teamName = .AssigneeTeam.Name}} 545 - {{end}} 546 - {{if .RemovedAssignee}} 547 - {{ctx.Locale.Tr "repo.issues.review.remove_review_request" $teamName $createdStr}} 525 + {{if or .AddedRequestReview .RemovedRequestReview}} 526 + <div class="timeline-item event" id="{{.HashTag}}"> 527 + <span class="badge">{{svg "octicon-tag"}}</span> 528 + {{template "shared/user/avatarlink" dict "user" .Poster}} 529 + <span class="text grey muted-links"> 530 + {{if and (eq (len .RemovedRequestReview) 1) (eq (len .AddedRequestReview) 0) (eq ((index .RemovedRequestReview 0).ID) .PosterID) (eq ((index .RemovedRequestReview 0).Type) "user")}} 531 + <span class="review-request-list">{{ctx.Locale.Tr "repo.issues.review.remove_review_request_self" $createdStr}}</span> 532 + {{else if and .AddedRequestReview (not .RemovedRequestReview)}} 533 + {{ctx.Locale.TrN (len .AddedRequestReview) "repo.issues.review.add_review_request" "repo.issues.review.add_review_requests" (RenderReviewRequest .AddedRequestReview) $createdStr}} 534 + {{else if and (not .AddedRequestReview) .RemovedRequestReview}} 535 + {{ctx.Locale.TrN (len .RemovedRequestReview) "repo.issues.review.remove_review_request" "repo.issues.review.remove_review_requests" (RenderReviewRequest .RemovedRequestReview) $createdStr}} 548 536 {{else}} 549 - {{ctx.Locale.Tr "repo.issues.review.add_review_request" $teamName $createdStr}} 537 + {{ctx.Locale.Tr "repo.issues.review.add_remove_review_requests" (RenderReviewRequest .AddedRequestReview) (RenderReviewRequest .RemovedRequestReview) $createdStr}} 550 538 {{end}} 551 - {{end}} 552 - </span> 553 - </div> 539 + </span> 540 + </div> 541 + {{end}} 554 542 {{else if and (eq .Type 29) (or (gt .CommitsNum 0) .IsForcePush)}} 555 543  556 544 {{if and .Issue.IsClosed (gt .ID $.LatestCloseCommentID)}}

+21

tests/integration/fixtures/TestPullCombinedReviewRequest/review.yml

··· 1 + - 2 + id: 1001 3 + type: 4 4 + reviewer_id: 2 5 + issue_id: 3 6 + official: true 7 + stale: false 8 + original_author_id: 0 9 + updated_unix: 1000000000 10 + created_unix: 1000000000 11 + 12 + - 13 + id: 1002 14 + type: 4 15 + reviewer_id: 11 16 + issue_id: 3 17 + official: true 18 + stale: false 19 + original_author_id: 0 20 + updated_unix: 1000000000 21 + created_unix: 1000000000

+34

tests/integration/pull_test.go

··· 65 65 assert.NotContains(t, mergeInstructions, warningMessage) 66 66 }) 67 67 } 68 + 69 + func TestPullCombinedReviewRequest(t *testing.T) { 70 + defer tests.AddFixtures("tests/integration/fixtures/TestPullCombinedReviewRequest/")() 71 + defer tests.PrepareTestEnv(t)() 72 + 73 + session := loginUser(t, "user2") 74 + 75 + helper := func(t *testing.T, action, userID, expectedText string) { 76 + t.Helper() 77 + 78 + req := NewRequestWithValues(t, "POST", "/user2/repo1/pulls/request_review", map[string]string{ 79 + "_csrf": GetCSRF(t, session, "/user2/repo1/pulls/3"), 80 + "issue_ids": "3", 81 + "action": action, 82 + "id": userID, 83 + }) 84 + session.MakeRequest(t, req, http.StatusOK) 85 + 86 + req = NewRequest(t, "GET", "/user2/repo1/pulls/3") 87 + resp := session.MakeRequest(t, req, http.StatusOK) 88 + htmlDoc := NewHTMLParser(t, resp.Body) 89 + 90 + assert.Contains(t, htmlDoc.Find(".timeline-item:has(.review-request-list)").Last().Text(), expectedText) 91 + } 92 + 93 + helper(t, "detach", "2", "refused to review") 94 + helper(t, "attach", "4", "requested reviews from user4 and removed review requests for user2") 95 + helper(t, "attach", "9", "requested reviews from user4, user9 and removed review requests for user2") 96 + helper(t, "attach", "2", "requested reviews from user4, user9") 97 + helper(t, "detach", "4", "requested review from user9") 98 + helper(t, "detach", "11", "requested reviews from user9 and removed review requests for user11") 99 + helper(t, "detach", "9", "removed review request for user11") 100 + helper(t, "detach", "2", "removed review requests for user11, user2") 101 + }

Configure Feed

Configure Feed