From 3981f1b1278f558721e4b40b6fdbba9365b9cb1b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E8=B5=B5=E6=99=BA=E8=B6=85?= <1012112796@qq.com> Date: Thu, 3 Sep 2020 00:55:13 +0800 Subject: [PATCH] Remove duplicate logic in initListSubmits (#12660) * Remove duplicate logic in initListSubmits Using the same logic to handle Choosing reviewers and assignees as choosing label. It's the first step of #10926. Signed-off-by: a1012112796 <1012112796@qq.com> * fix choose block * fix nit * try fix bug * simple code Co-authored-by: techknowlogick --- routers/repo/issue.go | 14 +-- templates/repo/issue/view_content/pull.tmpl | 2 +- .../repo/issue/view_content/sidebar.tmpl | 8 +- web_src/js/index.js | 92 ++++++++----------- 4 files changed, 49 insertions(+), 67 deletions(-) diff --git a/routers/repo/issue.go b/routers/repo/issue.go index dabe0f6b0..4bbc35502 100644 --- a/routers/repo/issue.go +++ b/routers/repo/issue.go @@ -1516,10 +1516,11 @@ func updatePullReviewRequest(ctx *context.Context) { } reviewID := ctx.QueryInt64("id") - event := ctx.Query("is_add") + action := ctx.Query("action") - if event != "add" && event != "remove" { - ctx.ServerError("updatePullReviewRequest", fmt.Errorf("is_add should not be \"%s\"", event)) + // TODO: Not support 'clear' now + if action != "attach" && action != "detach" { + ctx.Status(403) return } @@ -1532,19 +1533,20 @@ func updatePullReviewRequest(ctx *context.Context) { return } - err = isLegalReviewRequest(reviewer, ctx.User, event == "add", issue) + err = isLegalReviewRequest(reviewer, ctx.User, action == "attach", issue) if err != nil { ctx.ServerError("isLegalRequestReview", err) return } - err = issue_service.ReviewRequest(issue, ctx.User, reviewer, event == "add") + err = issue_service.ReviewRequest(issue, ctx.User, reviewer, action == "attach") if err != nil { ctx.ServerError("ReviewRequest", err) return } } else { - ctx.ServerError("updatePullReviewRequest", fmt.Errorf("%d in %d is not Pull Request", issue.ID, issue.Repo.ID)) + ctx.Status(403) + return } } diff --git a/templates/repo/issue/view_content/pull.tmpl b/templates/repo/issue/view_content/pull.tmpl index 94edc8b12..b9254adea 100644 --- a/templates/repo/issue/view_content/pull.tmpl +++ b/templates/repo/issue/view_content/pull.tmpl @@ -49,7 +49,7 @@ {{end}} {{if $canChoose }} - + {{svg "octicon-sync" 16}} {{end}} diff --git a/templates/repo/issue/view_content/sidebar.tmpl b/templates/repo/issue/view_content/sidebar.tmpl index 71e84de2f..d02dedfb8 100644 --- a/templates/repo/issue/view_content/sidebar.tmpl +++ b/templates/repo/issue/view_content/sidebar.tmpl @@ -12,7 +12,7 @@ {{svg "octicon-gear" 16}} {{end}} -
+
{{.i18n.Tr "repo.issues.new.add_reviewer_title"}}
{{if .Reviewers}}
@@ -44,7 +44,7 @@ {{$canChoose = true}} {{end}} - + {{svg "octicon-check" 16}} {{.GetDisplayName}} @@ -78,7 +78,7 @@ {{end}} {{if $canChoose}} - + {{svg "octicon-sync" 16}} {{end}} @@ -244,7 +244,7 @@ {{svg "octicon-gear" 16}} {{end}} -
+
{{.i18n.Tr "repo.issues.new.add_assignees_title"}}
diff --git a/web_src/js/index.js b/web_src/js/index.js index a8965a437..1c23d0f73 100644 --- a/web_src/js/index.js +++ b/web_src/js/index.js @@ -157,7 +157,7 @@ function initLabelEdit() { }); } -function updateIssuesMeta(url, action, issueIds, elementId, isAdd) { +function updateIssuesMeta(url, action, issueIds, elementId) { return new Promise(((resolve) => { $.ajax({ type: 'POST', @@ -167,7 +167,6 @@ function updateIssuesMeta(url, action, issueIds, elementId, isAdd) { action, issue_ids: issueIds, id: elementId, - is_add: isAdd }, success: resolve }); @@ -373,21 +372,20 @@ function initCommentForm() { const $list = $(`.ui.${outerSelector}.list`); const $noSelect = $list.find('.no-select'); const $listMenu = $(`.${selector} .menu`); - let hasLabelUpdateAction = $listMenu.data('action') === 'update'; - const labels = {}; + let hasUpdateAction = $listMenu.data('action') === 'update'; + const items = {}; $(`.${selector}`).dropdown('setting', 'onHide', () => { - hasLabelUpdateAction = $listMenu.data('action') === 'update'; // Update the var - if (hasLabelUpdateAction) { + hasUpdateAction = $listMenu.data('action') === 'update'; // Update the var + if (hasUpdateAction) { const promises = []; - Object.keys(labels).forEach((elementId) => { - const label = labels[elementId]; + Object.keys(items).forEach((elementId) => { + const item = items[elementId]; const promise = updateIssuesMeta( - label['update-url'], - label.action, - label['issue-id'], + item['update-url'], + item.action, + item['issue-id'], elementId, - label['is-checked'] ); promises.push(promise); }); @@ -395,67 +393,49 @@ function initCommentForm() { } }); - $listMenu.find('.item:not(.no-select)').on('click', function () { - // we don't need the action attribute when updating assignees - if (selector === 'select-assignees-modify' || selector === 'select-reviewers-modify') { - // UI magic. We need to do this here, otherwise it would destroy the functionality of - // adding/removing labels - - if ($(this).data('can-change') === 'block') { - return false; - } - - if ($(this).hasClass('checked')) { - $(this).removeClass('checked'); - $(this).find('.octicon-check').addClass('invisible'); - $(this).data('is-checked', 'remove'); - } else { - $(this).addClass('checked'); - $(this).find('.octicon-check').removeClass('invisible'); - $(this).data('is-checked', 'add'); - } - - updateIssuesMeta( - $listMenu.data('update-url'), - '', - $listMenu.data('issue-id'), - $(this).data('id'), - $(this).data('is-checked') - ); - $listMenu.data('action', 'update'); // Update to reload the page when we updated items + $listMenu.find('.item:not(.no-select)').on('click', function (e) { + e.preventDefault(); + if ($(this).hasClass('ban-change')) { return false; } + hasUpdateAction = $listMenu.data('action') === 'update'; // Update the var if ($(this).hasClass('checked')) { $(this).removeClass('checked'); $(this).find('.octicon-check').addClass('invisible'); - if (hasLabelUpdateAction) { - if (!($(this).data('id') in labels)) { - labels[$(this).data('id')] = { + if (hasUpdateAction) { + if (!($(this).data('id') in items)) { + items[$(this).data('id')] = { 'update-url': $listMenu.data('update-url'), action: 'detach', 'issue-id': $listMenu.data('issue-id'), }; } else { - delete labels[$(this).data('id')]; + delete items[$(this).data('id')]; } } } else { $(this).addClass('checked'); $(this).find('.octicon-check').removeClass('invisible'); - if (hasLabelUpdateAction) { - if (!($(this).data('id') in labels)) { - labels[$(this).data('id')] = { + if (hasUpdateAction) { + if (!($(this).data('id') in items)) { + items[$(this).data('id')] = { 'update-url': $listMenu.data('update-url'), action: 'attach', 'issue-id': $listMenu.data('issue-id'), }; } else { - delete labels[$(this).data('id')]; + delete items[$(this).data('id')]; } } } + // TODO: Which thing should be done for choosing review requests + // to make choosed items be shown on time here? + if (selector === 'select-reviewers-modify' || selector === 'select-assignees-modify') { + return false; + } + const listIds = []; $(this).parent().find('.item').each(function () { if ($(this).hasClass('checked')) { @@ -473,23 +453,26 @@ function initCommentForm() { $($(this).parent().data('id')).val(listIds.join(',')); return false; }); - $listMenu.find('.no-select.item').on('click', function () { - if (hasLabelUpdateAction || selector === 'select-assignees-modify') { + $listMenu.find('.no-select.item').on('click', function (e) { + e.preventDefault(); + if (hasUpdateAction) { updateIssuesMeta( $listMenu.data('update-url'), 'clear', $listMenu.data('issue-id'), '', - '' ).then(reload); } $(this).parent().find('.item').each(function () { $(this).removeClass('checked'); $(this).find('.octicon').addClass('invisible'); - $(this).data('is-checked', 'remove'); }); + if (selector === 'select-reviewers-modify' || selector === 'select-assignees-modify') { + return false; + } + $list.find('.item').each(function () { $(this).addClass('hide'); }); @@ -521,7 +504,6 @@ function initCommentForm() { '', $menu.data('issue-id'), $(this).data('id'), - $(this).data('is-checked') ).then(reload); } switch (input_id) { @@ -552,7 +534,6 @@ function initCommentForm() { '', $menu.data('issue-id'), $(this).data('id'), - $(this).data('is-checked') ).then(reload); } @@ -672,10 +653,9 @@ function initIssueComments() { event.preventDefault(); updateIssuesMeta( url, - '', + isChecked === 'true' ? 'attach' : 'detach', issueId, id, - isChecked ).then(reload); });