gpnf-auto-attach-child-files: Refactored for greater configuration flexibility.

[veryspry]

Context

⛑️ Ticket(s): https://secure.helpscout.net/conversation/2011825626/38759/

Summary

• Refactors gpnf-auto-attach-child-files into a class.
• Moves configuration logic to constructor args so that users can easily create multiple, unique configurations.
• Adds child_form_upload_field_ids config option to limit which child form upload fields are included in a notification.

[claygriffiths]

Awesome work!

I'm going to give this a functional test here shortly. In the meantime, here are a couple of minor suggestions for the code.

[claygriffiths]

Just noticed a typo in a method name and now PHPCS is barking about a couple of things.

I did a functional test and it looks good! I did notice that it slowed down submission a bit, but I'm not sure if that's an issue with the refactor itself.

I went ahead and approved this pending the two small things mentioned above.

[veryspry]

I did a functional test and it looks good! I did notice that it slowed down submission a bit, but I'm not sure if that's an issue with the refactor itself.

Thanks for doing that! All was looking good during my tests but it's always nice to get an extra set of eyes on it 🙌

As far as the slowdown goes, my guess is that it isn't due to the refactor as the logic is essentially the same as before. The main difference is the way that configuration options are set

If I had to guess why, database reads are probably the main culprit. For instance:

• getting the form object which queries the form table and the form meta table: https://github.com/gravitywiz/snippet-library/pull/535/files#diff-6dc898ef07f19e6f8f6a1c641d2c979e25e78c5327ea4622eccf6b3eb35fb4c7R65
• getting all entries for each child form: https://github.com/gravitywiz/snippet-library/pull/535/files#diff-6dc898ef07f19e6f8f6a1c641d2c979e25e78c5327ea4622eccf6b3eb35fb4c7R66

We also make a call to preg_replace() for each attachment we find which might impact speed noticeably?

That said, PCRE is supposed to cache compiled regex's which should be helpful since we pass the same regex string to each call of preg_replace()

https://stackoverflow.com/a/210086

[veryspry]

Just waiting on confirmation from a user that this works well for them before merging!

[veryspry]

Feature request:

Support for multiple child forms (in one parent form) whose upload fields have different id's.

For example: Parent Form has two child forms, A and B. A's file upload field has id of 1 and B's has a file upload field with id of 2.

I'm going to go ahead and add that functionality to this snippet before merging!

[veryspry]

@claygriffiths just a heads up that I changed the API here a bit so that this works with more complex child form configurations.

See here for more details: #535 (comment)

[veryspry]

We're just waiting for a user to confirm this works for them

[veryspry]

This is confirmed to work as intended. I'm mergin' it!