provide download link for releases

RESOLVED FIXED

Status

Release Engineering
Balrog: Frontend
RESOLVED FIXED
3 years ago
a year ago

People

(Reporter: bhearsum, Assigned: bhearsum)

Tracking

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(2 attachments, 2 obsolete attachments)

(Assignee)

Description

3 years ago
Created attachment 8611197 [details] [diff] [review]
add download link for releases

This is becoming an increasingly common thing to do, and constructing URLs by hand is annoying.

Peter, is this a reasonable way to do this in Angular? It seems to work well - I get a download prompt in the same tab from my browser with a nice filename.
Attachment #8611197 - Flags: review?(peterbe)
(Assignee)

Comment 1

3 years ago
Created attachment 8611198 [details] [diff] [review]
support pretty formatting for releases
Attachment #8611198 - Flags: review?(peterbe)
Comment on attachment 8611197 [details] [diff] [review]
add download link for releases

Review of attachment 8611197 [details] [diff] [review]:
-----------------------------------------------------------------

r+ with nits. 

I wasn't sure about the target attribute so I made this: http://plnkr.co/edit/7BSq3s8pHZe2ebK7X567
It doesn't pop-up with a save dialog. How it works for you I don't know.

::: app/templates/releases.html
@@ +70,5 @@
>      <h3 class="panel-title">
>        <div style="float:right">
>          <i ng-show="release_name && $first">Current</i>
>          <button ng-show="release_name && !$first" class="btn btn-default btn-xs" ng-click="openRevertModal(release)">Revert to this</button>
> +        <a ng-show="!release_name" class="btn btn-default btn-xs" target="_self" download="{{ release.name }}.json" ng-href="/api/releases/{{ release.name }}?pretty=1">Download</a>

If it works, sure! 

You might need to escape the name though. Just to be safe. 
Consider this: http://stackoverflow.com/a/14512986/205832
Attachment #8611197 - Flags: review?(peterbe) → review+
Comment on attachment 8611198 [details] [diff] [review]
support pretty formatting for releases

Review of attachment 8611198 [details] [diff] [review]:
-----------------------------------------------------------------

::: auslib/admin/views/releases.py
@@ +211,5 @@
>          headers.update(get_csrf_headers())
> +        if request.args.get("pretty"):
> +            indent = 4
> +        else:
> +            indent = None

Or `indent = 4 if request.args.get("pretty") else None`
Attachment #8611198 - Flags: review?(peterbe) → review+
(Assignee)

Comment 4

3 years ago
Created attachment 8611358 [details] [diff] [review]
escape release names

Like this?
Attachment #8611197 - Attachment is obsolete: true
Attachment #8611358 - Flags: review?(peterbe)
Comment on attachment 8611358 [details] [diff] [review]
escape release names

Review of attachment 8611358 [details] [diff] [review]:
-----------------------------------------------------------------

You're supposed to put in extra filters into the app/filters/ directory. The naming convention seems to be *_filter.js 
See https://github.com/mozilla/balrog-ui/blob/master/app/js/filters/startfrom_filter.js

Once you've done that you don't need to do anything to "include" it. The lineman code automatically builds it in.
Attachment #8611358 - Flags: review?(peterbe) → review-
(Assignee)

Comment 6

3 years ago
Created attachment 8612246 [details] [diff] [review]
move filter definition

Thanks Peter, I'm glad I came to you for this review!
Attachment #8611358 - Attachment is obsolete: true
Attachment #8612246 - Flags: review?(peterbe)
Duplicate of this bug: 1151041
Comment on attachment 8612246 [details] [diff] [review]
move filter definition

Review of attachment 8612246 [details] [diff] [review]:
-----------------------------------------------------------------

Don't hate me for saying this but; in retrospect that name is poor. It should be "URL encode" or something like "urlencode". 

It's actually just a angular'esque wrapper for 'encodeURIComponent'. I guess technically you could call it "uriencode" but the word "url" is slowly beating "uri" into submission as the standard lingo. 

Take it or leave it :)
Attachment #8612246 - Flags: review?(peterbe) → review+
(Assignee)

Comment 9

3 years ago
(In reply to Peter Bengtsson [:peterbe] from comment #8)
> Comment on attachment 8612246 [details] [diff] [review]
> move filter definition
> 
> Review of attachment 8612246 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> Don't hate me for saying this but; in retrospect that name is poor. It
> should be "URL encode" or something like "urlencode". 
> 
> It's actually just a angular'esque wrapper for 'encodeURIComponent'. I guess
> technically you could call it "uriencode" but the word "url" is slowly
> beating "uri" into submission as the standard lingo. 
> 
> Take it or leave it :)

Haha, no worries. I'll land with it renamed - I think url or uriencode makes it a lot clearer.
(Assignee)

Updated

3 years ago
Attachment #8612246 - Flags: checked-in+

Comment 10

3 years ago
Commit pushed to master at https://github.com/mozilla/balrog

https://github.com/mozilla/balrog/commit/e17b9cdc94e9e20af7da2e0d344c66cb1edbd3b7
bug 1168838: provide download link for releases. r=peterbe
(Assignee)

Updated

3 years ago
Attachment #8611198 - Flags: checked-in+
(Assignee)

Comment 11

3 years ago
I pushed this to prod, and it's working well!
Status: NEW → RESOLVED
Last Resolved: 3 years ago
Resolution: --- → FIXED
\o/
(Assignee)

Updated

3 years ago
Duplicate of this bug: 1110916

Comment 14

a year ago
Commit pushed to master at https://github.com/mozilla/balrog

https://github.com/mozilla/balrog/commit/e05253385312c3756d1a7a99e9c8a29e116c59e3
[balrog-ui] bug 1168838: provide download link for releases. r=peterbe
You need to log in before you can comment on or make changes to this bug.