Prepend a real summary in mail digest

[mattiaverga]

Fixes #277

This change prepends a short summary made only by short titles when the user choose to receive mail digest with verbose messages.
Full messages are appended to the summary, so the user can see at a glance what are the digest contents.

I modified the relevant test accordingly with the new digest format. Note that I was unable to run real unittests because Vagrant environment doesn't install (it errors out during provision).

[codecov]

Codecov Report

Merging #278 into develop will increase coverage by 0.1%.
The diff coverage is 58.53%.

@@            Coverage Diff             @@
##           develop     #278     +/-   ##
==========================================
+ Coverage    56.57%   56.68%   +0.1%     
==========================================
  Files           70       70             
  Lines         3694     3712     +18     
  Branches       475      476      +1     
==========================================
+ Hits          2090     2104     +14     
- Misses        1536     1541      +5     
+ Partials        68       67      -1

Impacted Files	Coverage Δ
fmn/formatters.py	79.1% <58.53%> (-0.1%)	⬇️

---

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update c6f873b...6cf3020. Read the comment docs.

[mattiaverga]

I'm unsure about adding a test case where digest content is too big:

    def test_batch_too_big(self):
        """Test batch content when it is too big."""
        big_batch = self.messages * 32000
        expected_start = (
            'Precedence: Bulk\n'
            'Auto-Submitted: auto-generated\n'
            'From: notifications@fedoraproject.org\n'
            'To: jeremy@jcline.org\n'
            'X-Fedmsg-Topic: org.fedoraproject.dev.fmn.filter.update\n'
            'X-Fedmsg-Category: fmn\n'
            'X-Fedmsg-Username: jcline\n'
            'X-Fedmsg-Username: bowlofeggs\n'
            'X-Fedmsg-Num-Packages: 0\n'
            'Subject: Fedora Notifications Digest (2 updates)\n'
            'MIME-Version: 1.0\n'
            'Content-Type: text/plain; charset="utf-8"\n'
            'Content-Transfer-Encoding: base64\n\n'
            'VGhpcyBtZXNzYWdlIGRpZ2VzdCB3YXMgdG9vIGxhcmdlIHRvIGJlIHNlbnQhClRoZSBmb2xsb3dp\n'
            'bmcgbWVzc2FnZXMgd2VyZSBiYXRjaGVkOgoK\n'
        )

        actual = formatters.email_batch(big_batch, self.verbose_recipient)
        self.assertIn(expected, actual)

This would be a very time consuming test. Or maybe we can reduce the max digest content length (now it's 20M chars, I think a value of 2M chars would be more appropriate...).

[jeremycline]

I guess one could mock the return from len so it is 20M+, but since there wasn't coverage for that before I'm not too bothered if that's still not covered. I agree 20M is crazy high. I'd be fine with 2M or even less, but I'd leave that for a different PR.

[jeremycline]

Looks good. Could you add a change log entry?

Thanks!

[mattiaverga]

Sure, should I add the new entry as "Feature" for a new 2.1.0 version or as a "Bug fix" for v.2.0.3?

By the way, while running the 'test_batch_too_big' locally, I found that the encoding of the digest content is corrupted: line 502 should be digest_content += msg['msg_id'] + u'\n' (now the '\n' misses the unicode declaration and the resultant b64 content goes corrupted). Should I push the fix in this same PR?

[jeremycline]

Just add a new section "Master" and put it under "Bug fixes" since it's fixing a regression. That way it can be either 2.0.3 if no features go in before the next release, or 2.1.0.

By the way, while running the 'test_batch_too_big' locally, I found that the encoding of the digest content is corrupted: line 502 should be digest_content += msg['msg_id'] + u'\n' (now the '\n' misses the unicode declaration and the resultant b64 content goes corrupted). Should I push the fix in this same PR?

Ah, nice catch. I'm fine with it being included in this PR, although it'd be nice if it was its own commit.

[mattiaverga]

I'm fine with it being included in this PR, although it'd be nice if it was its own commit.

I will made a separate PR for that, adding a test unit and lowering the batch characters limit to 500k (that would be an email of nearly 2 MBytes of text - it seems a reasonable limit).

I also would like to think some way to stop processing the batch if that limit is trespassed, now we process the full batch and then we eventually discard it just to process again all the message IDs.

[jeremycline]

Great, thanks! I squashed this, made one small change (a file got marked as executable), and merged it, but it looks like I changed enough that GitHub doesn't know it's merged. It's 681cf86.