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 multi delete issues #22344

Closed

Conversation

CodeDoctorDE
Copy link
Contributor

@CodeDoctorDE CodeDoctorDE commented Jan 4, 2023

Screenshot 2023-01-05 at 08-31-26 REPO

Closes #22273

I couldn't test it because make does not like me.

Copy link
Member

@yardenshoham yardenshoham left a comment

Choose a reason for hiding this comment

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

I tested on Gitpod, if you want to try:
Open in Gitpod

LGTM

@GiteaBot GiteaBot added the lgtm/need 1 This PR needs approval from one additional maintainer to be merged. label Jan 5, 2023
@CodeDoctorDE CodeDoctorDE marked this pull request as ready for review January 5, 2023 07:31
@CodeDoctorDE
Copy link
Contributor Author

Thanks I forgot that gitpod can do this too

@@ -171,6 +171,7 @@
{{else}}
<div class="ui red active basic button issue-action" data-action="close" data-url="{{$.RepoLink}}/issues/status" style="margin-left: auto">{{.locale.Tr "repo.issues.action_close"}}</div>
{{end}}
<div class="ui negative active button issue-action" data-action="delete" data-url="{{$.RepoLink}}/issues/status" style="margin-left: auto">{{.locale.Tr "repo.issues.delete"}}</div>
Copy link
Member

Choose a reason for hiding this comment

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

I'm strongly against allowing such a destructive action to occur without presenting a warning dialogue.
At best, extract the existing issue deletion dialogue into a new template and reuse it, or copy it if that's not easily possible.

Copy link
Member

Choose a reason for hiding this comment

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

In addition, why display a text when it's probably a better idea to use the octicon-trash SVG instead?
First of all to minimize the user potential to missclick there, and secondly to improve the UX.

@@ -29,7 +29,7 @@
{{end}}
<div class="ui divider"></div>
<a class="{{if not $.RepoIDs}}ui basic primary button{{end}} repo name item" href="{{$.Link}}?type={{$.ViewType}}&sort={{$.SortType}}&state={{$.State}}&q={{$.Keyword}}">
<span class="text truncate">All</span>
<span class="text truncate">{{.locale.Tr "all"}}</span>
Copy link
Member

@delvh delvh Jan 5, 2023

Choose a reason for hiding this comment

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

This change seems oddly familiar… 😉

Comment on lines +133 to +140
func DeleteIssues(doer *user_model.User, gitRepo *git.Repository, issues []*issues_model.Issue) error {
for _, issue := range issues {
if err := DeleteIssue(doer, gitRepo, issue); err != nil {
return err
}
}
return nil
}
Copy link
Member

Choose a reason for hiding this comment

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

Hmm, we could think about passing a context in as well and executing this whole block inside a transaction so that it is automatically rolled back if the deletion of any issue fails.

@wxiaoguang
Copy link
Contributor

I couldn't test it because make does not like me.

gitpod can do this too

And there are some open review comments, is this PR active?

@wxiaoguang wxiaoguang added the issue/needs-feedback For bugs, we need more details. For features, the feature must be described in more detail label May 5, 2023
@CodeDoctorDE
Copy link
Contributor Author

I'm currently very busy, feel free to close this for now

@wxiaoguang
Copy link
Contributor

Thank you very much for your contribution.

If you don't mind, maybe some some contributors or I could pick it?

lunny pushed a commit that referenced this pull request Jun 13, 2023
Clarify the "link-action" behavior:

>  // A "link-action" can post AJAX request to its "data-url"
> // Then the browser is redirect to: the "redirect" in response, or
"data-redirect" attribute, or current URL by reloading.

And enhance the "link-action" to support showing a modal dialog for
confirm. A similar general approach could also help PRs like
#22344 (comment)

> // If the "link-action" has "data-modal-confirm(-html)" attribute, a
confirm modal dialog will be shown before taking action.


And a lot of duplicate code can be removed now. A good framework design
can help to avoid code copying&pasting.

---------

Co-authored-by: silverwind <me@silverwind.io>
@wxiaoguang
Copy link
Contributor

Replaced by Batch delete issue and improvie tippy opts #25253

Major changes:

  1. Only "repo admin" can delete issues.
  2. Use a separate route handler, re-using "update status" doesn't seem right.
  3. Add the confirm dialog

@wxiaoguang wxiaoguang closed this Jun 14, 2023
@go-gitea go-gitea locked as resolved and limited conversation to collaborators Sep 12, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
issue/needs-feedback For bugs, we need more details. For features, the feature must be described in more detail lgtm/need 1 This PR needs approval from one additional maintainer to be merged.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Delete multiple issues at once
5 participants