Skip to content

Remove duplicate logic in initListSubmits #12660

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.

Sign up for GitHub

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

Merged
techknowlogick merged 8 commits into from
Sep 2, 2020
Merged

Remove duplicate logic in initListSubmits #12660

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
8 commits
Select commit Hold shift + click to select a range
f114356
Remove duplicate logic in initListSubmits
a1012112796 Aug 31, 2020
1553de6
fix choose block
a1012112796 Aug 31, 2020
08b284a
fix nit
a1012112796 Aug 31, 2020
27a9878
try fix bug
a1012112796 Sep 2, 2020
500215b
simple code
a1012112796 Sep 2, 2020
dddf3f9
Merge branch 'master' into fix_sidebar
a1012112796 Sep 2, 2020
c4c90ce
Merge branch 'master' into fix_sidebar
techknowlogick Sep 2, 2020
ea5f803
Merge branch 'master' into fix_sidebar
a1012112796 Sep 2, 2020
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
View file
Open in desktop
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
View file
Open in desktop
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
View file
Open in desktop
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
View file
Open in desktop
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