Skip to content

HTTPS clone URL

Subversion checkout URL

You can clone with HTTPS or Subversion.

Download ZIP

Loading…

paginate commbadge notes on in reviewer tools review history (bug 977325) #1814

Merged
merged 1 commit into from

2 participants

@ngokevin
Owner

Breaking #1607 up. Screencast

Part 1: In a waffle, use a separate template for reviewers app history that is populated by the Commbadge API rather than passing in ActivityLog objects through the view.

Part 2: Display Commbadge attachments along with the notes.

Part 3: Create Commbadge notes for all review actions.

Part 4: Create Commbadge attachments from reviewer tools.

Part 5: Paginate Commbadge notes if a lot exist for a version.

  • JS pagination of notes pulling in from the Commbadge API on reviewer tools.
@ngokevin
Owner

thanks, commbadge is nearly done. just need to guide it in. comment on the PR next time :fist:

media/css/devreg/reviewers.styl
((12 lines not shown))
+
+ }
+ .next.active + .prev:after {
+ background-color: $medium-gray;
+ border-radius: 3px;
+ bottom: 2px;
+ content: "";
+ display: inline-block;
+ height: 3px;
+ left: 2px;
+ margin: 0 2px;
+ position: relative;
+ width: 3px;
+ }
+}
+.comm-note {
@cvan Owner
cvan added a note

add a newline between these blocks -> more :eyeglasses: readable

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
media/js/devreg/reviewers/reviewers_commbadge.js
@@ -9,7 +9,8 @@ define('reviewersCommbadge', [], function() {
function _userArg(url) {
// Persona user token.
- return urlparams(url, {'_user': localStorage.getItem('0::user')});
+ return urlparams(url, {'_user': localStorage.getItem('0::user'),
@cvan Owner
cvan added a note

add a TODO. also, won't this contain _user=undefined if the user hasn't logged in to commbadge?

@ngokevin Owner
ngokevin added a note

Yeah, it doesn't work unless we fix the login issue which was why I got around to trying out those patches.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
requirements/prod.txt
@@ -96,7 +96,7 @@ statsd==2.0.3
suds==0.4
## Not on pypi.
--e git+https://github.com/mozilla/amo-validator.git@cde411df44cb3d4fcd763b58d9e5924dfc34df8b#egg=amo-validator
+-e git+https://github.com/mozilla/amo-validator.git@631efe1790c54eabbc3032a86916624a89da0a63#egg=amo-validator
@cvan Owner
cvan added a note

oops?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
@cvan
Owner

r+wc good work, mate :tanabata_tree:

@ngokevin
Owner

@ngokevin ngokevin merged commit e3c0570 into from
@ngokevin ngokevin deleted the branch
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Commits on Mar 4, 2014
  1. @ngokevin
This page is out of date. Refresh to see the latest.
View
41 media/css/devreg/reviewers.styl
@@ -772,13 +772,48 @@ button.search, .log-filter-outside button {
top: -20px;
width: auto;
}
+#review-files .activity .comm-notes-paginator {
+ border-bottom: none;
+ display: none;
+
+ a {
+ display: none;
+ float: right;
+ margin-right: 5px;
+
+ }
+ .next.active + .prev:after {
+ background-color: $medium-gray;
+ border-radius: 3px;
+ bottom: 2px;
+ content: "";
+ display: inline-block;
+ height: 3px;
+ left: 2px;
+ margin: 0 2px;
+ position: relative;
+ width: 3px;
+ }
+}
+
+.comm-note {
+
+ pre {
+ padding-left: 0;
+ }
+ strong {
+ color: lighten($medium-gray, 15%);
+ }
+}
+
#review-files table.activity {
th {
width: 25%;
padding-right: 1em;
}
tr {
- border-top: 1px dotted #ccc;
+ border-bottom: 1px dotted #ccc;
+ border-top: none;
td {
padding: 15px 0;
@@ -790,8 +825,8 @@ button.search, .log-filter-outside button {
padding-bottom: 0;
}
}
- tr:first-child {
- border-top: 0 none;
+ tr:last-child {
+ border-bottom: 0;
}
}
#review-files .listing-header span.light {
View
76 media/js/devreg/reviewers/reviewers_commbadge.js
@@ -9,7 +9,9 @@ define('reviewersCommbadge', [], function() {
function _userArg(url) {
// Persona user token.
- return urlparams(url, {'_user': localStorage.getItem('0::user')});
+ // TODO: store schema version in localStorage, sync Zamboni + Commonplace login.
+ return urlparams(url, {'_user': localStorage.getItem('0::user'),
+ 'ordering': '-created'});
}
// Fetch all of the app's threads.
@@ -32,36 +34,60 @@ define('reviewersCommbadge', [], function() {
}
});
- function appendNotesToTable(notes, $table) {
- // Given a list of notes, passes each note into the template
- // and appends to review history table.
- notes = notes.objects;
- for (var i = 0; i < notes.length; i++) {
- var note = notes[i];
- var author = note.author_meta.name;
- var created = moment(note.created).format('MMMM Do YYYY, h:mm:ss a');
-
- // Append notes to table.
- $table.append(noteTemplate({
- attachments: note.attachments,
- body: note.body,
- // L10n: {0} is author of note, {1} is a datetime. (e.g. "by Kevin on 2014-01-16").
- metadata: format(gettext('By {0} on {1}'),
- [author, created]),
- noteType: noteTypes[note.note_type],
- }));
- }
- }
-
// Fetch all of the notes for each thread.
for (var i = 0; i < threads.length; i++) {
var thread = threads[i];
var $table = $('table.activity[data-version=' + thread.version + ']');
var commNoteUrl = $itemHistory.data('comm-note-url').replace(threadIdPlaceholder, thread.id);
- $.get(_userArg(commNoteUrl), function(notes) {
- appendNotesToTable(notes, $table);
- });
+ $.get(_userArg(commNoteUrl), getNoteHandler($table));
}
});
+
+ function appendNotesToTable(notes, $table) {
+ // Given a list of notes, passes each note into the template
+ // and appends to review history table.
+ $table.find('.comm-note').remove();
+ notes = notes.objects;
+
+ for (var i = 0; i < notes.length; i++) {
+ var note = notes[i];
+ var author = note.author_meta.name;
+ var created = moment(note.created).format('MMMM Do YYYY, h:mm:ss a');
+
+ // Append notes to table.
+ $('tbody', $table).append(noteTemplate({
+ attachments: note.attachments,
+ body: note.body,
+ // L10n: {0} is author of note, {1} is a datetime. (e.g., "by Kevin on Feburary 18th 2014 12:12 pm").
+ metadata: format(gettext('By {0} on {1}'),
+ [author, created]),
+ noteType: noteTypes[note.note_type],
+ }));
+ }
+ }
+
+ function getNoteHandler($table) {
+ return function(notes) {
+ appendNotesToTable(notes, $table);
+
+ $table.attr('data-prev-url', notes.meta.previous);
+ $table.attr('data-next-url', notes.meta.next);
+
+ // Show/hide pagination links.
+ var $paginator = $('.comm-notes-paginator', $table);
+ $paginator.toggle(notes.meta.previous !== null || notes.meta.next !== null);
+ $('.prev', $paginator).toggle(notes.meta.previous !== null);
+ $('.next', $paginator).toggle(notes.meta.next !== null)
+ .toggleClass('active', notes.meta.next !== null);
+ };
+ }
+
+ $('.prev, .next').click(_pd(function() {
+ // Fetch previous or next offset on click of paginator links..
+ var $this = $(this);
+ var noteUrl = $this.hasClass('next') ? 'data-next-url' : 'data-prev-url';
+ var $table = $this.closest('table.activity');
+ $.get($table.attr(noteUrl), getNoteHandler($table));
+ }));
});
View
4 mkt/reviewers/templates/reviewers/includes/commbadge_note_template.html
@@ -1,8 +1,8 @@
<script type="text/template" id="commbadge-note">
- <tr>
+ <tr class="comm-note">
<th><%= noteType %></th>
<td>
- <div><%= metadata %></div>
+ <strong><%= metadata %></strong>
<pre class="light history-comment"><%= body %></pre>
<% if (attachments.length) { %>
<div class="log-group log-attachments">
View
50 mkt/reviewers/templates/reviewers/includes/review_history_commbadge.html
@@ -56,28 +56,38 @@
<td>
{# By version. #}
<table class="activity" data-version="{{ version.id }}">
- {% if version.releasenotes %}
- <tr>
- <th>{{ _('Version Notes') }}</th>
- <td class="activity_version">
- <div class="history-notes">
- {{ version.releasenotes|nl2br }}
- </div>
+ <thead>
+ {% if version.releasenotes %}
+ <tr>
+ <th>{{ _('Version Notes') }}</th>
+ <td class="activity_version">
+ <div class="history-notes">
+ {{ version.releasenotes|nl2br }}
+ </div>
+ </td>
+ </tr>
+ {% endif %}
+ {% if version.approvalnotes %}
+ <tr>
+ <th>{{ _('Notes for Reviewers') }}</th>
+ <td class="activity_approval">
+ <div class="history-notes">
+ {{ version.approvalnotes|urlize(100)|nl2br }}
+ </div>
+ </td>
+ </tr>
+ {% endif %}
+ </thead>
+ <tbody>
+ {# Results populated here by reviewers_commbadge.js (Commbadge API) #}
+ <tr class="comm-notes-paginator">
+ <th>&nbsp;</th>
+ <td>
+ <a class="next" href="#">{{ _('Next Page') }}</a>
+ <a class="prev" href="#">{{ _('Previous Page') }}</a>
</td>
</tr>
- {% endif %}
- {% if version.approvalnotes %}
- <tr>
- <th>{{ _('Notes for Reviewers') }}</th>
- <td class="activity_approval">
- <div class="history-notes">
- {{ version.approvalnotes|urlize(100)|nl2br }}
- </div>
- </td>
- </tr>
- {% endif %}
-
- {# Results populated here by reviewers_commbadge.js (Commbadge API) #}
+ </tbody>
</table>
</td>
Something went wrong with that request. Please try again.