Skip to content
Open
Show file tree
Hide file tree
Changes from 2 commits
Commits
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
5 changes: 5 additions & 0 deletions Bugzilla/Config/Attachment.pm
Original file line number Diff line number Diff line change
Expand Up @@ -54,6 +54,11 @@ sub get_param_list {
},
{name => 'github_pr_linking_enabled', type => 'b', default => 0},
{name => 'github_pr_signature_secret', type => 't', default => ''},
{
name => 'attachment_hide_content_types',
type => 't',
default => 'text/x-phabricator-request text/x-github-pull-request',
},
Comment thread
dklawren marked this conversation as resolved.
);
return @param_list;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -23,12 +23,14 @@
attachment_rendered = 0;
Hook.process("row");
NEXT IF attachment_rendered;
is_external = attachment_hide_types.${attachment.contenttype} ? 1 : 0;
%]
<tr data-attachment-id="[% attachment.id FILTER none %]" class="
[%~ " bz_private" IF attachment.isprivate %]
[%~ " attach-obsolete" IF attachment.isobsolete %]
[%~ " attach-external" IF is_external %]
[%~ " attach-patch" IF attachment.can_review %]
" [% IF attachment.isobsolete %]style="display:none"[% END %]>
" [% IF attachment.isobsolete || is_external %]style="display:none"[% END %]>
<td class="attach-desc-td">
<div class="attach-desc">
<a href="[% basepath FILTER none %]attachment.cgi?id=[% attachment.id FILTER none %]"
Expand Down Expand Up @@ -103,6 +105,9 @@
<button type="button" role="none" id="attachments-add-btn" class="secondary">Attach New File</button>
</a>
[% END %]
[% IF external_attachments %]
<button type="button" id="attachments-external-btn" class="secondary">Show External</button>
[% END %]
[% IF obsolete_attachments %]
<button type="button" id="attachments-obsolete-btn" class="secondary">Show Obsolete</button>
[% END %]
Expand Down
22 changes: 20 additions & 2 deletions extensions/BugModal/template/en/default/bug_modal/edit.html.tmpl
Original file line number Diff line number Diff line change
Expand Up @@ -51,12 +51,24 @@
# count attachments
active_attachments = 0;
obsolete_attachments = 0;
external_attachments = 0;

attachment_hide_types = {};
IF Param('attachment_hide_content_types');
FOREACH _type IN Param('attachment_hide_content_types').split('[,\s]+');
attachment_hide_types.$_type = 1 IF _type;
END;
END;

FOREACH attachment IN bug.attachments;
NEXT IF attachment.isprivate && !(user.is_insider || attachment.attacher.id == user.id || (attachment.is_bounty_attachment && bug.reporter.id == user.id)) ;
IF attachment.isobsolete;
obsolete_attachments = obsolete_attachments + 1;
ELSE;
active_attachments = active_attachments + 1;
IF attachment_hide_types.${attachment.contenttype};
external_attachments = external_attachments + 1;
END;
END;

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

external_attachments here only counts external-type attachments that are not obsolete.
attachments.html.tmpl:108-110 gates the "Show External" button on that count:

[% IF external_attachments %]
  <button type="button" id="attachments-external-btn" class="secondary">Show External</button>
[% END %]

Following bug 2043229 requirements, a row which is both obsolete and external is only meant to show once both "Show External" and "Show Obsolete" are toggled on (bug_modal.js:279-292: function updateAttachmentRows).

So a bug with only obsolete external attachments never displays the "Show External" button, also we need that button to reveal them.
=> It could results in permanently hidden and then unreachable attachments

Quick-fix: I suggest to count obsolete and external attachments too when deciding whether to render the button, but to not include them in the subtitle number of external files

END;

Expand Down Expand Up @@ -1391,11 +1403,15 @@
[% IF active_attachments || obsolete_attachments || user.id %]
[%
sub = [];
visible_attachments = active_attachments - external_attachments;
IF active_attachments + obsolete_attachments == 0;
sub.push("No files");
END;
IF active_attachments;
sub.push(active_attachments _ " file" _ (active_attachments == 1 ? "" : "s"));
IF visible_attachments;
sub.push(visible_attachments _ " file" _ (visible_attachments == 1 ? "" : "s"));
END;
IF external_attachments;
sub.push(external_attachments _ " external file" _ (external_attachments == 1 ? "" : "s"));
END;
IF obsolete_attachments;
sub.push(obsolete_attachments _ " obsolete file" _ (obsolete_attachments == 1 ? "" : "s"));
Expand All @@ -1408,6 +1424,8 @@
[% INCLUDE bug_modal/attachments.html.tmpl
active_attachments = active_attachments
obsolete_attachments = obsolete_attachments
external_attachments = external_attachments
attachment_hide_types = attachment_hide_types
Comment thread
dklawren marked this conversation as resolved.
%]
[% END %]
[% END %]
Expand Down
29 changes: 22 additions & 7 deletions extensions/BugModal/web/bug_modal.js
Original file line number Diff line number Diff line change
Expand Up @@ -272,13 +272,28 @@ $(function() {
}
});

// toggle obsolete attachments
$('#attachments-obsolete-btn')
.click(function(event) {
event.preventDefault();
$(event.target).text(($('#attachments tr:hidden').length ? 'Hide Obsolete' : 'Show Obsolete'));
$('#attachments tr.attach-obsolete').toggle();
});
// toggle obsolete/external attachments
let attachObsoleteShowing = false;
let attachExternalShowing = false;

function updateAttachmentRows() {
$('#attachments tr.attach-obsolete').toggle(attachObsoleteShowing);
$('#attachments tr.attach-external:not(.attach-obsolete)').toggle(attachExternalShowing);
$('#attachments-obsolete-btn').text(attachObsoleteShowing ? 'Hide Obsolete' : 'Show Obsolete');
$('#attachments-external-btn').text(attachExternalShowing ? 'Hide External' : 'Show External');
}
Comment thread
dklawren marked this conversation as resolved.
Comment thread
dklawren marked this conversation as resolved.

$('#attachments-obsolete-btn').click(function(event) {
event.preventDefault();
attachObsoleteShowing = !attachObsoleteShowing;
updateAttachmentRows();
});

$('#attachments-external-btn').click(function(event) {
event.preventDefault();
attachExternalShowing = !attachExternalShowing;
updateAttachmentRows();
});

// URL --> unsafe warning
$('.bug-url')
Expand Down
8 changes: 8 additions & 0 deletions template/en/default/admin/params/attachment.html.tmpl
Original file line number Diff line number Diff line change
Expand Up @@ -77,5 +77,13 @@

google_storage_service_account =>
"Google Cloud Storage Service Account",

attachment_hide_content_types =>
"A comma or whitespace separated list of MIME types that should be hidden"
_ " by default in the bug modal attachments list. Attachments matching these"
Comment thread
dklawren marked this conversation as resolved.
_ " types will not appear in the attachments table unless the user clicks the"
_ " <em>Show External</em> button. This is useful for content types such as"
_ " <tt>text/x-phabricator-request</tt> and <tt>text/x-github-pull-request</tt>"
_ " that are already displayed in a dedicated section above the attachments list.",
}
%]
Loading