snippet-library
snippet-library copied to clipboard
`gpnf-auto-attach-child-files`: Refactored for greater configuration flexibility.
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.
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 :raised_hands:
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
Just waiting on confirmation from a user that this works well for them before merging!
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!
@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: https://github.com/gravitywiz/snippet-library/pull/535#issuecomment-1265688832
We're just waiting for a user to confirm this works for them
This is confirmed to work as intended. I'm mergin' it!