Reindex array to remove gaps after applying allowed_domains list #885
Conversation
There was a problem hiding this comment.
Thanks for the fix!
Would you mind also adding a regression test (either to DomainFilteringAdapterTest.php or perhaps an integration test with both adapters) that confirms the fix works as expected? If you're not sure how or don't have the bandwidth I'll gladly add it myself - it might just take a few days for me to find the time.
I could see from testing the fix on the site I'm developing that the gaps in the array were removed but I haven't been able to get my head around how to write a test to prove it. |
|
No problem! I truly don't mind adding this myself :-)
…On Tue, Jun 7, 2022, 10:18 Ben ***@***.***> wrote:
Thanks for the fix!
Would you mind also adding a regression test (either to
DomainFilteringAdapterTest.php
<https://github.com/thephpleague/commonmark/blob/2.3/tests/unit/Extension/Embed/DomainFilteringAdapterTest.php>
or perhaps an integration test with both adapters) that confirms the fix
works as expected? If you're not sure how or don't have the bandwidth I'll
gladly add it myself - it might just take a few days for me to find the
time.
I could see from testing the fix on the site I'm developing that the gaps
in the array were removed but I haven't been able to get my head around how
to write a test to prove it.
—
Reply to this email directly, view it on GitHub
<#885 (comment)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/AABRKMVVA2OVFLXPCGWBSK3VN5K43ANCNFSM5X7HIIEQ>
.
You are receiving this because you commented.Message ID:
***@***.***>
|
|
Test added via effe03b. I modified it to use a mock so we could more easily verify the actual array being passed to the decorated instance. Thanks again for catching and resolving this! I'll release v2.3.3 with this fix momentarily. |
Fix for #884