dev/core#1270 - fix email processor dropping attachments - ALTERNATE

[demeritcowboy]

Overview

This is an alternate to #15344

https://lab.civicrm.org/dev/core/issues/1270

The email processor will drop attachments if there's more in the incoming email than the limit set at system settings - misc. The setting is intended for activity creation in the UI, but shouldn't cause the email processor to drop attachments.

Before

Some attachments can be lost.

After

It files and keeps all the attachments.

Technical Details

Adds a new admin setting for non-UI processes parallel to the existing max_attachments setting which is then only used for UI processes.

Comments

Unfortunately the existing max_attachments setting seems to be in limboland where it currently has a hack, so to add the new setting I copied that. I'm not sure that's desired.

@pfigel

[civibot]

(Standard links)

• If this is your first pull-request for CiviCRM, please browse CONTRIBUTING.md for information about the development and testing processes.
• If you are reviewing this pull-request, you may wish to consult the test sites and the Review Standards (long template, short template).

[demeritcowboy]

Hmm, regarding test failure looks like I might have used something php 7.2-ish. Remembering now that tests run on 7.0. Fixing...

[pfigel]

Thanks @demeritcowboy, this LGTM! I'm not sure if I can make an educated judgment on the setting hack, but I'd be okay with it, so leaving that to the merger 😁.

For more details on the rationale behind adding a new setting, see the discussion starting here.

• General standards
  • (r-explain) PASS: Explains the rationale behind why a new setting is needed/preferred.
  • (r-user) PASS: User would expect attachments not to be dropped.
  • (r-doc) PASS: New setting is sufficiently described, old setting has been amended to make the difference clear.
  • (r-run) PASS: I've verified that:
    • The bug is fixed (i.e. up to 100 attachments are created)
    • The setting form works
    • Attachment limits aren't affected in the UI
• Developer standards
  • (r-tech) PASS:
  • (r-code) PASS-ISH: I'm uncertain about the setting hack, but wouldn't be opposed to merging as I don't see a trivial alternative and it doesn't really make things worse.
  • (r-maint) PASS: Adds tests
  • (r-test) PASS

[seamuslee001]

Makes sense to me merging

[demeritcowboy]

Thanks! @pfigel @seamuslee001