Skip to content

Redirect on project after issue created #17211

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
merged 6 commits into from
Oct 5, 2021
Merged

Redirect on project after issue created #17211

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
1aaa769
Redirect on project after issue created
romdum Oct 3, 2021
0af4e42
Code review - Remove redirect_on_project param in URL (#17211)
romdum Oct 3, 2021
34a7201
Code review - Change variable redirect_on_project by redirection_afte…
romdum Oct 3, 2021
6849c8c
Code review - Change variable redirection_after_creation by redirect_…
romdum Oct 3, 2021
43b3ad9
Merge branch 'main' into feature/redirect_on_project
6543 Oct 5, 2021
11696b2
Merge branch 'main' into feature/redirect_on_project
6543 Oct 5, 2021
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
9 changes: 8 additions & 1 deletion routers/web/repo/issue.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -803,6 +803,9 @@ func NewIssue(ctx *context.Context) {
ctx.Data["Project"] = project
}

if len(ctx.Req.URL.Query().Get("project")) > 0 {
Copy link
Member

@lunny lunny Oct 4, 2021

Choose a reason for hiding this comment

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

It seems this is unnecessary? The POST requests will also request the current URL. So that the query strings are still there.

Copy link
Contributor

@wxiaoguang wxiaoguang Oct 4, 2021 •
edited
Loading

Choose a reason for hiding this comment

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

The issue edit form's action is filled by {{.Link}}, without query string. So this parameter is necessary.

ctx.Data["redirect_on_project"] = true
}
}

RetrieveRepoMetas(ctx, ctx.Repo.Repository, false)
Expand Down Expand Up @@ -990,7 +993,11 @@ func NewIssuePost(ctx *context.Context) {
}

log.Trace("Issue created: %d/%d", repo.ID, issue.ID)
ctx.Redirect(ctx.Repo.RepoLink + "/issues/" + fmt.Sprint(issue.Index))
if ctx.FormBool("redirect_on_project") {
Copy link
Contributor

@wxiaoguang wxiaoguang Oct 3, 2021

Choose a reason for hiding this comment

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

Is is necessary to introduce redirect_on_project? Can we just check if form.ProjectID > 0?

Copy link
Contributor Author

@romdum romdum Oct 3, 2021

Choose a reason for hiding this comment

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

I don't think it's possible, when form is submitted we can't know if user click on "new issue" button from project view, maybe he creates issue from issue list view.

Copy link
Contributor

@wxiaoguang wxiaoguang Oct 3, 2021

Choose a reason for hiding this comment

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

On which page besides projects/view.tmpl the new issue link uses project_id=xxx in query string?

Copy link
Contributor Author

@romdum romdum Oct 3, 2021

Choose a reason for hiding this comment

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

No one, but when you submit the new issue form with a project, the project_id is also filled ?

Copy link
Contributor

@wxiaoguang wxiaoguang Oct 3, 2021

Choose a reason for hiding this comment

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

I haven't read the source code carefully, just a feeling and curiosity: if there is no project_id, how can the newly created issue be added into current project?

Copy link
Contributor Author

@romdum romdum Oct 3, 2021 •
edited
Loading

Choose a reason for hiding this comment

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

There is two cases :

  • First, you want to create an issue from button on list issue view, after on the form you can assign issue to a project. In this case, we don't want to redirect on the project but the project_id is filled.
  • Second, you want to create an issue from the button on project view, you are redirected to the new issue form with the project automatically selected. In this case we want to redirect on the project when the form is submitted

If the project is not selected when the form is submitted the user is redirected to issue view

Copy link
Contributor

@wxiaoguang wxiaoguang Oct 3, 2021

Choose a reason for hiding this comment

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

For the first case, the project_id is in POST body. So ctx.Req.URL.Query().Get("project_id") will work like redirect_on_project, Query().Get("project_id") only presents when the issue is created from the page view button.

For me, I prefer to use use existing and simple mechanism as long as it doesn't introduce ambiguity.

Copy link
Contributor Author

@romdum romdum Oct 3, 2021 •
edited
Loading

Choose a reason for hiding this comment

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

Ok I didn't understand what do you mean, sorry. So I removed redirect_on_project in URL and I used ctx.Req.URL.Query().Get("project") to know if we should redirect on project page or not

Copy link
Contributor

@wxiaoguang wxiaoguang Oct 3, 2021

Choose a reason for hiding this comment

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

I wasn't saying clearly. 😂

ctx.Redirect(ctx.Repo.RepoLink + "/projects/" + fmt.Sprint(form.ProjectID))
} else {
ctx.Redirect(ctx.Repo.RepoLink + "/issues/" + fmt.Sprint(issue.Index))
}
}

// commentTag returns the CommentTag for a comment in/with the given repo, poster and issue
Expand Down
1 change: 1 addition & 0 deletions templates/repo/issue/new_form.tmpl
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -236,5 +236,6 @@
{{end}}
</div>
</div>
<input type="hidden" name="redirect_on_project" value="{{.redirect_on_project}}">
Copy link
Contributor

@wxiaoguang wxiaoguang Oct 3, 2021

Choose a reason for hiding this comment

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

Maybe we can call this variable redirection_after_creation (now we only have 2 choices, empty or project), then developers in future will have a chance to introduce more redirection pages.

Copy link
Contributor Author

@romdum romdum Oct 3, 2021

Choose a reason for hiding this comment

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

Yes good idea, it's done 👍

Copy link
Member

@delvh delvh Oct 3, 2021

Choose a reason for hiding this comment

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

Maybe we can call this variable redirection_after_creation (now we only have 2 choices, empty or project), then developers in future will have a chance to introduce more redirection pages.

I think it can be even shorter, what do you think about redirection_after_creation?

</div>
</form>