fedora-infra / bodhi

Bodhi is a web-system that facilitates the process of publishing updates for a Fedora-based software distribution.
https://bodhi.fedoraproject.org
GNU General Public License v2.0
152 stars 195 forks source link

Strip subjects in email #882

Closed bowlofeggs closed 7 years ago

bowlofeggs commented 8 years ago

This issue was originally reported by Harald Reindl. I've quoted what he wrote below:

that stuff below as subject without trimming is really careless, besides that depending on the mail client you can't see much of the message in the preview window, it's all listed in the content anyways AND in many cases nmail filters would reject mails with ordinary header lenghts (happened often here until i raised the limits *only* for that fedora mails)

Subject: [Fedora Update] [comment] breeze-icon-theme-5.25.0-1.fc24
 extra-cmake-modules-5.25.0-1.fc24 kf5-5.25.0-2.fc24
 kf5-attica-5.25.0-1.fc24 kf5-baloo-5.25.0-1.fc24 kf5-bluez-qt-5.25.0-1.fc24
 kf5-frameworkintegration-5.25.0-1.fc24 kf5-kactivities-5.25.0-1.fc24
 kf5-kactivities-stats-5.25.0-1.fc24 kf5-kapidox-5.25.0-1.fc24
 kf5-karchive-5.25.0-1.fc24 kf5-kauth-5.25.0-1.fc24
 kf5-kbookmarks-5.25.0-1.fc24 kf5-kcmutils-5.25.0-1.fc24
 kf5-kcodecs-5.25.0-1.fc24 kf5-kcompletion-5.25.0-1.fc24
 kf5-kconfig-5.25.0-1.fc24 kf5-kconfigwidgets-5.25.0-1.fc24
 kf5-kcoreaddons-5.25.0-1.fc24 kf5-kcrash-5.25.0-1.fc24
 kf5-kdbusaddons-5.25.0-1.fc24 kf5-kdeclarative-5.25.0-1.fc24
 kf5-kded-5.25.0-1.fc24 kf5-kdelibs4support-5.25.0-1.fc24
 kf5-kdesignerplugin-5.25.0-1.fc24 kf5-kdesu-5.25.0-1.fc24
 kf5-kdewebkit-5.25.0-1.fc24 kf5-kdnssd-5.25.0-1.fc24
 kf5-kdoctools-5.25.0-1.fc24 kf5-kemoticons-5.25.0-1.fc24
 kf5-kfilemetadata-5.25.0-1.fc24 kf5-kglobalaccel-5.25.0-1.fc24
 kf5-kguiaddons-5.25.0-1.fc24 kf5-khtml-5.25.0- 1.fc24
 kf5-ki18n-5.25.0-1.fc24 kf5-kiconthemes-5.25.0-1.fc24
 kf5-kidletime-5.25.0-1.fc24 kf5-kimageformats-5.25.0-1.fc24
 kf5-kinit-5.25.0-1.fc24 kf5-kio-5.25.0-1.fc24 kf5-kitemmodels-5.25.0-1.fc24
 kf5-kitemviews-5.25.0-1.fc24 kf5-kjobwidgets-5.25.0-1.fc24
 kf5-kjs-5.25.0-1.fc24 kf5-kjsembed-5.25.0-1.fc24
 kf5-kmediaplayer-5.25.0-1.fc24 kf5-knewstuff-5.25.0-1.fc24
 kf5-knotifications-5.25.0-1.fc24 kf5-knotifyconfig-5.25.0-1.fc24
 kf5-kpackage-5.25.0-1.fc24 kf5-kparts-5.25.0-1.fc24
 kf5-kpeople-5.25.0-1.fc24 kf5-kplotting-5.25.0-1.fc24
 kf5-kpty-5.25.0-1.fc24 kf5-kross-5.25.0-1.fc24 kf5-krunner-5.25.0-1.fc24
 kf5-kservice-5.25.0-1.fc24 kf5-ktexteditor-5.25.0-1.fc24
 kf5-ktextwidgets-5.25.0-1.fc24 kf5-kunitconversion-5.25.0-1.fc24
 kf5-kwallet-5.25.0-1.fc24 kf5-kwayland-5.25.0-1.fc24
 kf5-kwidgetsaddons-5.25.0-1.fc24 kf5-kwindowsystem-5.25.0-1.fc24
 kf5-kxmlgui-5.25.0-1.fc24 kf5-kxmlrpcclient-5.25.0-1.fc24
 kf5-modemmanager-qt-5.25.0-1.fc24 kf5-networkmanager-qt-5.25.0-1.fc24
 kf5-plasma-5.25.0-2.f c24 kf5-solid-5.25.0-1.fc24 kf5-sonnet-5.25.0-1.fc24
 kf5-threadweaver-5.25.0-1.fc24 oxygen-icon-theme-5.25.0-1.fc24
nbianca commented 7 years ago

Hello Randy,

What is the expected outcome? Simply keep first 30 characters or so and add ellipsis?

bowlofeggs commented 7 years ago

Hello Bianca!

Actually @ryanlerch just wrote some template code to do something very similar on this update page:

<%def name="generateupdatetitlestring()">

  % if len(update.builds) == 1:
    % for build in update.builds:
      ${self.util.packagename_from_nvr(build.nvr)}
    % endfor
  % elif len(update.builds) == 2:
    % for build in update.builds:
      % if loop.last:
      and
      % endif
      ${self.util.packagename_from_nvr(build.nvr)}
    % endfor
  % elif len(update.builds) > 2:
    % for build in update.builds:
      % if loop.index == 0:
      ${self.util.packagename_from_nvr(build.nvr)},
      % elif loop.index == 1:
      ${self.util.packagename_from_nvr(build.nvr)}
      &amp; ${len(update.builds)-2} more
      % endif
    % endfor
  % endif
</%def>

I bet we could take that idea and move it into a Python method on the bodhi.server.models.Update class and basically do the same thing there (but probably substitute the &amp with "and", or we could make it an optional boolean on the method whether to use &amp or and). If we did that, then we could just have the template I cited use the common method for some nice code sharing.

How does that sound to you?

bowlofeggs commented 7 years ago

This was fixed by #1561.