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

Remove duplicate logic in initListSubmits #12660

Merged
merged 8 commits into from
Sep 2, 2020
Merged
Show file tree
Hide file tree
Changes from 5 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
14 changes: 8 additions & 6 deletions routers/repo/issue.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
}

Expand All @@ -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
}
}

Expand Down
2 changes: 1 addition & 1 deletion templates/repo/issue/view_content/pull.tmpl
Original file line number Diff line number Diff line change
Expand Up @@ -49,7 +49,7 @@
{{end}}

{{if $canChoose }}
<a href="#" class="ui poping up icon re-request-review" data-is-checked="{{if eq .Type 4}}remove{{else}}add{{end}}" data-issue-id="{{$.Issue.ID}}" data-content="{{ if eq .Type 4 }} {{$.i18n.Tr "repo.issues.remove_request_review"}} {{else}} {{$.i18n.Tr "repo.issues.re_request_review"}} {{end}}" data-id="{{.ReviewerID}}" data-update-url="{{$.RepoLink}}/issues/request_review">
<a href="#" class="ui poping up icon re-request-review" data-is-checked="{{if eq .Type 4}}true{{else}}false{{end}}" data-issue-id="{{$.Issue.ID}}" data-content="{{ if eq .Type 4 }} {{$.i18n.Tr "repo.issues.remove_request_review"}} {{else}} {{$.i18n.Tr "repo.issues.re_request_review"}} {{end}}" data-id="{{.ReviewerID}}" data-update-url="{{$.RepoLink}}/issues/request_review">
{{svg "octicon-sync" 16}}
</a>
{{end}}
Expand Down
8 changes: 4 additions & 4 deletions templates/repo/issue/view_content/sidebar.tmpl
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@
{{svg "octicon-gear" 16}}
{{end}}
</span>
<div class="filter menu" data-action="" data-issue-id="{{$.Issue.ID}}" data-update-url="{{$.RepoLink}}/issues/request_review">
<div class="filter menu" data-action="update" data-issue-id="{{$.Issue.ID}}" data-update-url="{{$.RepoLink}}/issues/request_review">
<div class="header" style="text-transform: none;font-size:16px;">{{.i18n.Tr "repo.issues.new.add_reviewer_title"}}</div>
{{if .Reviewers}}
<div class="ui icon search input">
Expand Down Expand Up @@ -44,7 +44,7 @@
{{$canChoose = true}}
{{end}}

<a class="{{if not $canChoose}}ui poping up{{end}} item {{if $checked}} checked {{end}}" href="#" data-id="{{.ID}}" data-id-selector="#review_request_{{.ID}}" data-can-change="{{if not $canChoose}}block{{end}}" {{if not $canChoose}} data-content="{{$.i18n.Tr "repo.issues.remove_request_review_block"}}"{{end}} data-is-checked="{{if $checked}}add{{else}}remove{{end}}">
<a class="{{if not $canChoose}}ui poping up{{end}} item {{if $checked}} checked {{end}} {{if not $canChoose}}ban-change{{end}}" href="#" data-id="{{.ID}}" data-id-selector="#review_request_{{.ID}}" {{if not $canChoose}} data-content="{{$.i18n.Tr "repo.issues.remove_request_review_block"}}"{{end}}>
<span class="octicon-check {{if not $checked}}invisible{{end}}">{{svg "octicon-check" 16}}</span>
<span class="text">
<img class="ui avatar image" src="{{.RelAvatarLink}}"> {{.GetDisplayName}}
Expand Down Expand Up @@ -78,7 +78,7 @@
{{end}}

{{if $canChoose}}
<a href="#" class="ui poping up icon re-request-review" data-is-checked="{{if eq .Type 4}}remove{{else}}add{{end}}" data-content="{{ if eq .Type 4 }} {{$.i18n.Tr "repo.issues.remove_request_review"}} {{else}} {{$.i18n.Tr "repo.issues.re_request_review"}} {{end}}" data-issue-id="{{$.Issue.ID}}" data-id="{{.ReviewerID}}" data-update-url="{{$.RepoLink}}/issues/request_review">
<a href="#" class="ui poping up icon re-request-review" data-is-checked="{{if eq .Type 4}}true{{else}}false{{end}}" data-content="{{ if eq .Type 4 }} {{$.i18n.Tr "repo.issues.remove_request_review"}} {{else}} {{$.i18n.Tr "repo.issues.re_request_review"}} {{end}}" data-issue-id="{{$.Issue.ID}}" data-id="{{.ReviewerID}}" data-update-url="{{$.RepoLink}}/issues/request_review">
{{svg "octicon-sync" 16}}
</a>
{{end}}
Expand Down Expand Up @@ -244,7 +244,7 @@
{{svg "octicon-gear" 16}}
{{end}}
</span>
<div class="filter menu" data-action="" data-issue-id="{{$.Issue.ID}}" data-update-url="{{$.RepoLink}}/issues/assignee">
<div class="filter menu" data-action="update" data-issue-id="{{$.Issue.ID}}" data-update-url="{{$.RepoLink}}/issues/assignee">
<div class="header" style="text-transform: none;font-size:16px;">{{.i18n.Tr "repo.issues.new.add_assignees_title"}}</div>
<div class="ui icon search input">
<i class="search icon"></i>
Expand Down
92 changes: 36 additions & 56 deletions web_src/js/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -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',
Expand All @@ -167,7 +167,6 @@ function updateIssuesMeta(url, action, issueIds, elementId, isAdd) {
action,
issue_ids: issueIds,
id: elementId,
is_add: isAdd
},
success: resolve
});
Expand Down Expand Up @@ -373,89 +372,70 @@ 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);
});
Promise.all(promises).then(reload);
}
});

$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')) {
Expand All @@ -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');
});
Expand Down Expand Up @@ -521,7 +504,6 @@ function initCommentForm() {
'',
$menu.data('issue-id'),
$(this).data('id'),
$(this).data('is-checked')
).then(reload);
}
switch (input_id) {
Expand Down Expand Up @@ -552,7 +534,6 @@ function initCommentForm() {
'',
$menu.data('issue-id'),
$(this).data('id'),
$(this).data('is-checked')
).then(reload);
}

Expand Down Expand Up @@ -672,10 +653,9 @@ function initIssueComments() {
event.preventDefault();
updateIssuesMeta(
url,
'',
isChecked === 'true' ? 'attach' : 'detach',
issueId,
id,
isChecked
).then(reload);
});

Expand Down