-
-
Notifications
You must be signed in to change notification settings - Fork 5.9k
Added URL mapping for Release attachments like on github.com #1700
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
Changes from 1 commit
2e6c39b
1a539a6
aefa570
0a9a812
5ca8651
84d22d8
52b4dea
98cb0d2
7aa17f7
4119816
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -58,6 +58,16 @@ func (a *Attachment) IncreaseDownloadCount() error { | |
return nil | ||
} | ||
|
||
// FileSize is returning the file datasize | ||
func (a *Attachment) FileSize() (string, error) { | ||
stats, err := os.Stat(AttachmentLocalPath(a.UUID)) | ||
if err != nil { | ||
return "error", fmt.Errorf("AttachmentFileSize: %v", err) | ||
} | ||
result := float64(stats.Size()) / float64(1048576) | ||
return fmt.Sprintf("%.1f", result) + " MB", nil | ||
} | ||
|
||
// AttachmentLocalPath returns where attachment is stored in local file | ||
// system based on given UUID. | ||
func AttachmentLocalPath(uuid string) string { | ||
|
@@ -126,6 +136,11 @@ func GetAttachmentByUUID(uuid string) (*Attachment, error) { | |
return getAttachmentByUUID(x, uuid) | ||
} | ||
|
||
// GetAttachmentByReleaseIDFileName returns attachment by given releaseId and fileName. | ||
func GetAttachmentByReleaseIDFileName(releaseID int64, fileName string) (*Attachment, error) { | ||
return getAttachmentByReleaseIDFileName(x, releaseID, fileName) | ||
} | ||
|
||
func getAttachmentsByIssueID(e Engine, issueID int64) ([]*Attachment, error) { | ||
attachments := make([]*Attachment, 0, 10) | ||
return attachments, e.Where("issue_id = ? AND comment_id = 0", issueID).Find(&attachments) | ||
|
@@ -142,6 +157,22 @@ func GetAttachmentsByCommentID(commentID int64) ([]*Attachment, error) { | |
return attachments, x.Where("comment_id=?", commentID).Find(&attachments) | ||
} | ||
|
||
/* Author : github.com/gdeverlant | ||
getAttachmentByReleaseIDFileName return a file based on the the following infos: | ||
- releaseID | ||
- fileName | ||
*/ | ||
func getAttachmentByReleaseIDFileName(e Engine, releaseID int64, fileName string) (*Attachment, error) { | ||
attach := &Attachment{ReleaseID: releaseID, Name: fileName} | ||
has, err := e.Get(attach) | ||
if err != nil { | ||
return nil, err | ||
} else if !has { | ||
return nil, ErrAttachmentNotExist{0, attach.UUID} | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Change this to There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. why ????????????????????? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. So you can later understand and check if file was not found or if there was some internal error that you have to return error 500 There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Instead of telling me how to change it why don't you post the full code so that I get what you try to mean. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. i hope you are happy There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. you changed it in wrong function and wrong line. You need to change: There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. No you expressed it wrong that's why I could not understand! |
||
} | ||
return attach, nil | ||
} | ||
|
||
// DeleteAttachment deletes the given attachment and optionally the associated file. | ||
func DeleteAttachment(a *Attachment, remove bool) error { | ||
_, err := DeleteAttachments([]*Attachment{a}, remove) | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -268,6 +268,41 @@ func Action(ctx *context.Context) { | |
ctx.Redirect(redirectTo) | ||
} | ||
|
||
// RedirectDownload return a file based on the the following infos: | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. "// RedirectDownload returns a file based on tag and filename" :) |
||
func RedirectDownload(ctx *context.Context) { | ||
var ( | ||
vTag = ctx.Params("vTag") | ||
fileName = ctx.Params("fileName") | ||
) | ||
if len(vTag) > 0 { | ||
fmt.Println("Value of vTag %v", vTag) | ||
} | ||
if len(fileName) > 0 { | ||
fmt.Println("Value of fileName %v", fileName) | ||
} | ||
|
||
var tagNames []string | ||
tagNames = append(tagNames, vTag) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. just do |
||
|
||
curRepo := ctx.Repo.Repository | ||
|
||
releases, err := models.GetReleasesByRepoIDAndNames(curRepo.ID, tagNames) | ||
if err != nil { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think there should be also added or statement to check There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. No need, it'll fall through :) |
||
ctx.Handle(500, "RedirectDownload -> Release not found", err) | ||
return | ||
} | ||
|
||
if len(releases) == 1 { | ||
release := releases[0] | ||
att, err := models.GetAttachmentByReleaseIDFileName(release.ID, fileName) | ||
if err != nil { | ||
ctx.Handle(500, "RedirectDownload -> Attachment not found", err) | ||
return | ||
} | ||
ctx.Redirect(setting.AppSubURL + "/attachments/" + att.UUID) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. place this into There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Done |
||
} | ||
} | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. End the function with |
||
|
||
// Download download an archive of a repository | ||
func Download(ctx *context.Context) { | ||
var ( | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -486,6 +486,10 @@ func RegisterRoutes(m *macaron.Macaron) { | |
m.Get("/:id/:action", repo.ChangeMilestonStatus) | ||
m.Post("/delete", repo.DeleteMilestone) | ||
}, reqRepoWriter, context.RepoRef()) | ||
// New redirection from fake url | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. No need for this comment |
||
m.Group("/releases", func() { | ||
m.Get("/download/:vTag/:fileName", repo.RedirectDownload) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Move this to the Group just below (since they are identical) There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Done |
||
}, repo.MustBeNotBare, reqRepoWriter, context.RepoRef()) | ||
m.Group("/releases", func() { | ||
m.Get("/new", repo.NewRelease) | ||
m.Post("/new", bindIgnErr(auth.NewReleaseForm{}), repo.NewReleasePost) | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -53,21 +53,25 @@ | |
<div class="download"> | ||
<h2>{{$.i18n.Tr "repo.release.downloads"}}</h2> | ||
<ul class="list"> | ||
{{$tagname := .TagName}} | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Since you're using a named range, you don't need this assignment There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. this assignment is needed because tag range is not named There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. correct |
||
{{if .Attachments}} | ||
{{range $attachment := .Attachments}} | ||
<li> | ||
<!-- {{AppSubUrl}}/attachments/{{$attachment.UUID}} --> | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Remove commented out code There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I will not remove this out because I want to add for each release an option to display like github url or leave it the old way. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. than it has to be done before it gets merged in master There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Ok it's removed now I will do that later There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Remove old comments :) |
||
<a target="_blank" rel="noopener" href="{{$.RepoLink}}/releases/download/{{$tagname}}/{{$attachment.Name}}"> | ||
<span class="ui image octicon octicon-desktop-download" title='{{$attachment.Name}}'></span> {{$attachment.Name}} | ||
<a target="_blank" href="{{$.RepoLink}}/releases/download/{{$tagname}}/{{$attachment.Name}}" class="ui right"><span class="ui image octicon octicon-file-binary" title='{{$attachment.Name}}'></span>{{$attachment.FileSize}}</a> | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. There should not be tag inside tag. File size shoud be moved out of link, seperated with space and placed in brackets: There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Well it is not forbidden in HTML and there is a reason so I leave it. The font displayed is different from the front of the row with the filename so that is the reason I put this inside of another . There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. but why would you need that? Why first link is not enough? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Because the font is displayed differently from the other file description. The MB is some kind of Time New Roman. It is convenient for the user to give him a choice of either clicking on the file size, the icon or the filename to download. It is UI experience. |
||
</a> | ||
</li> | ||
{{end}} | ||
{{end}} | ||
<li> | ||
<a href="{{$.RepoLink}}/archive/{{.TagName}}.zip" rel="nofollow"><i class="octicon octicon-file-zip"></i> {{$.i18n.Tr "repo.release.source_code"}} (ZIP)</a> | ||
</li> | ||
<li> | ||
<a href="{{$.RepoLink}}/archive/{{.TagName}}.tar.gz"><i class="octicon octicon-file-zip"></i> {{$.i18n.Tr "repo.release.source_code"}} (TAR.GZ)</a> | ||
</li> | ||
{{if .Attachments}} | ||
{{range .Attachments}} | ||
<li> | ||
<a target="_blank" rel="noopener" href="{{AppSubUrl}}/attachments/{{.UUID}}"> | ||
<span class="ui image octicon octicon-desktop-download" title='{{.Name}}'></span> {{.Name}} | ||
</a> | ||
</li> | ||
{{end}} | ||
{{end}} | ||
|
||
</ul> | ||
</div> | ||
{{else}} | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
No need to add Author in comments, it will be visible in git history anyway
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done